From: Catalin Marinas <catalin.marinas@arm.com>
To: Xiongfeng Wang <wangxiongfeng2@huawei.com>
Cc: will@kernel.org, mark.rutland@arm.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, moyufeng@huawei.com,
linux-arch@vger.kernel.org
Subject: Re: [PATCH] asm-generic: introduce io_stop_wc() and add implementation for ARM64
Date: Fri, 17 Dec 2021 11:59:16 +0000 [thread overview]
Message-ID: <Ybx7lEF4srH4vBmh@arm.com> (raw)
In-Reply-To: <20211217085611.111999-1-wangxiongfeng2@huawei.com>
On Fri, Dec 17, 2021 at 04:56:11PM +0800, Xiongfeng Wang wrote:
> For memory accesses with Normal-Non Cacheable attributes, the CPU may do
You may want to mention "arm64 Normal Non-Cacheable" as other
architectures have a different meaning of NC.
> write combining. But in some situation, this is bad for the performance
> because the prior access may wait too long just to be merged.
>
> We introduce io_stop_wc() to prevent the Normal-NC memory accesses before
> this instruction to be merged with memory accesses after this
> instruction.
>
> We add implementation for ARM64 using DGH instruction and provide NOP
> implementation for other architectures.
>
> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>
> ---
> Documentation/memory-barriers.txt | 9 +++++++++
> arch/arm64/include/asm/barrier.h | 9 +++++++++
> include/asm-generic/barrier.h | 11 +++++++++++
> 3 files changed, 29 insertions(+)
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 7367ada13208..b868b51b1801 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1950,6 +1950,15 @@ There are some more advanced barrier functions:
> For load from persistent memory, existing read memory barriers are sufficient
> to ensure read ordering.
>
> + (*) io_stop_wc();
> +
> + For memory accesses with Normal-Non Cacheable attributes (e.g. those
> + returned by ioremap_wc()), the CPU may do write combining. But in some
> + situation, this is bad for the performance because the prior access may
> + wait too long just to be merged. io_stop_wc() can be used to prevent
> + merging memory accesses with Normal-Non Cacheable attributes before this
> + instruction with any memory accesses appearing after this instruction.
I'm fine with the patch in general but the comment here and in
asm-generic/barrier.h should avoid Normal Non-Cacheable as that's an
arm-specific term. Looking at Documentation/driver-api/device-io.rst, we
could simply say "write-combining". Something like:
For memory accesses with write-combining attributes (e.g. those
returned by ioremap_wc()), the CPU may wait for prior accesses to
be merged with subsequent ones. io_stop_wc() can be used to prevent
the merging of write-combining memory accesses before this macro
with those after it when such wait has performance implications.
(feel free to rephrase it but avoid Normal NC here)
> +
> ===============================
> IMPLICIT KERNEL MEMORY BARRIERS
> ===============================
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 1c5a00598458..62217be36217 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -26,6 +26,14 @@
> #define __tsb_csync() asm volatile("hint #18" : : : "memory")
> #define csdb() asm volatile("hint #20" : : : "memory")
>
> +/*
> + * Data Gathering Hint:
> + * This instruction prevents merging memory accesses with Normal-NC or
> + * Device-GRE attributes before the hint instruction with any memory accesses
> + * appearing after the hint instruction.
> + */
> +#define dgh() asm volatile("hint #6" : : : "memory")
This is fine, arm-specific code.
> +
> #ifdef CONFIG_ARM64_PSEUDO_NMI
> #define pmr_sync() \
> do { \
> @@ -46,6 +54,7 @@
> #define dma_rmb() dmb(oshld)
> #define dma_wmb() dmb(oshst)
>
> +#define io_stop_wc() dgh()
>
> #define tsb_csync() \
> do { \
> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index 640f09479bdf..083be6d34cb9 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
> @@ -251,5 +251,16 @@ do { \
> #define pmem_wmb() wmb()
> #endif
>
> +/*
> + * ioremap_wc() maps I/O memory as memory with Normal-Non Cacheable attributes.
> + * The CPU may do write combining for this kind of memory access. io_stop_wc()
> + * prevents merging memory accesses with Normal-Non Cacheable attributes
> + * before this instruction with any memory accesses appearing after this
> + * instruction.
Please change this as well along the lines of my comment above.
> + */
> +#ifndef io_stop_wc
> +#define io_stop_wc do { } while (0)
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
> #endif /* __ASM_GENERIC_BARRIER_H */
Thanks.
--
Catalin
next prev parent reply other threads:[~2021-12-17 11:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-17 8:56 [PATCH] asm-generic: introduce io_stop_wc() and add implementation for ARM64 Xiongfeng Wang
2021-12-17 11:59 ` Catalin Marinas [this message]
2021-12-20 12:23 ` Xiongfeng Wang
2021-12-22 9:08 ` Christoph Hellwig
2021-12-22 10:43 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Ybx7lEF4srH4vBmh@arm.com \
--to=catalin.marinas@arm.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=moyufeng@huawei.com \
--cc=wangxiongfeng2@huawei.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).