The cited commit describes that when using writel(), explcit wmb()
is not needed. wmb() is an expensive barrier. writel() uses the needed
I/O barrier instead of expensive wmb().
Hence update the example to be more accurate that matches the current
implementation.
commit 5846581e3563 ("locking/memory-barriers.txt: Fix broken DMA vs. MMIO ordering example")
Signed-off-by: Parav Pandit <[email protected]>
---
changelog:
v0->v1:
- Corrected to mention I/O barrier instead of dma_wmb().
- removed numbered references in commit log
- corrected typo 'explcit' to 'explicit' in commit log
---
Documentation/memory-barriers.txt | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 832b5d36e279..2d77a7411e52 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1927,10 +1927,11 @@ There are some more advanced barrier functions:
before we read the data from the descriptor, and the dma_wmb() allows
us to guarantee the data is written to the descriptor before the device
can see it now has ownership. The dma_mb() implies both a dma_rmb() and
- a dma_wmb(). Note that, when using writel(), a prior wmb() is not needed
- to guarantee that the cache coherent memory writes have completed before
- writing to the MMIO region. The cheaper writel_relaxed() does not provide
- this guarantee and must not be used here.
+ a dma_wmb(). Note that, when using writel(), a prior I/O barrier is not
+ needed to guarantee that the cache coherent memory writes have completed
+ before writing to the MMIO region. The cheaper writel_relaxed() does not
+ provide this guarantee and must not be used here. Hence, writeX() is always
+ preferred.
See the subsection "Kernel I/O barrier effects" for more information on
relaxed I/O accessors and the Documentation/core-api/dma-api.rst file for
--
2.26.2
Hi,
> On Sep 29, 2022, at 10:04 PM, Parav Pandit <[email protected]> wrote:
>
> The cited commit describes that when using writel(), explcit wmb()
> is not needed. wmb() is an expensive barrier. writel() uses the needed
> I/O barrier instead of expensive wmb().
Because you mentioned it in the commit message, Why not mention in the documentation text as well that writel() has the needed I/O barrier in it?
>
> Hence update the example to be more accurate that matches the current
> implementation.
That would make it more accurate, since accuracy is your goal.
thanks,
- Joel
>
> commit 5846581e3563 ("locking/memory-barriers.txt: Fix broken DMA vs. MMIO ordering example")
> Signed-off-by: Parav Pandit <[email protected]>
>
> ---
> changelog:
> v0->v1:
> - Corrected to mention I/O barrier instead of dma_wmb().
> - removed numbered references in commit log
> - corrected typo 'explcit' to 'explicit' in commit log
> ---
> Documentation/memory-barriers.txt | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 832b5d36e279..2d77a7411e52 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1927,10 +1927,11 @@ There are some more advanced barrier functions:
> before we read the data from the descriptor, and the dma_wmb() allows
> us to guarantee the data is written to the descriptor before the device
> can see it now has ownership. The dma_mb() implies both a dma_rmb() and
> - a dma_wmb(). Note that, when using writel(), a prior wmb() is not needed
> - to guarantee that the cache coherent memory writes have completed before
> - writing to the MMIO region. The cheaper writel_relaxed() does not provide
> - this guarantee and must not be used here.
> + a dma_wmb(). Note that, when using writel(), a prior I/O barrier is not
> + needed to guarantee that the cache coherent memory writes have completed
> + before writing to the MMIO region. The cheaper writel_relaxed() does not
> + provide this guarantee and must not be used here. Hence, writeX() is always
> + preferred.
>
> See the subsection "Kernel I/O barrier effects" for more information on
> relaxed I/O accessors and the Documentation/core-api/dma-api.rst file for
> --
> 2.26.2
>
Hi,
On Fri, 30 Sep 2022 05:03:55 +0300, Parav Pandit wrote:
> The cited commit describes that when using writel(), explcit wmb()
> is not needed. wmb() is an expensive barrier. writel() uses the needed
> I/O barrier instead of expensive wmb().
>
> Hence update the example to be more accurate that matches the current
> implementation.
>
> commit 5846581e3563 ("locking/memory-barriers.txt: Fix broken DMA vs. MMIO ordering example")
> Signed-off-by: Parav Pandit <[email protected]>
>
> ---
> changelog:
> v0->v1:
> - Corrected to mention I/O barrier instead of dma_wmb().
I don't think dma_wmb() and wmb() belong to "I/O barrier" as far as
memory-barriers.txt is concerned. They are listed in the "CPU MEMORY
BARRIERS" section. dma_wmb() belongs to "advanced barrier functions".
You see, writel() is one of the functions listed in the "KERNEL I/O
BARRIER EFFECTS" section.
Please be consistent with the word choice of the doc you are modifying,
so that any further confusion can be avoided in this infamously
hard-to-follow document. :-)
Regards,
Akira
> - removed numbered references in commit log
> - corrected typo 'explcit' to 'explicit' in commit log
> ---
> Documentation/memory-barriers.txt | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
[...]
> From: Akira Yokosawa <[email protected]>
> Sent: Friday, September 30, 2022 8:41 AM
>
> Hi,
>
> On Fri, 30 Sep 2022 05:03:55 +0300, Parav Pandit wrote:
> > The cited commit describes that when using writel(), explcit wmb() is
> > not needed. wmb() is an expensive barrier. writel() uses the needed
> > I/O barrier instead of expensive wmb().
> >
> > Hence update the example to be more accurate that matches the current
> > implementation.
> >
> > commit 5846581e3563 ("locking/memory-barriers.txt: Fix broken DMA vs.
> > MMIO ordering example")
> > Signed-off-by: Parav Pandit <[email protected]>
> >
> > ---
> > changelog:
> > v0->v1:
> > - Corrected to mention I/O barrier instead of dma_wmb().
> I don't think dma_wmb() and wmb() belong to "I/O barrier" as far as
> memory-barriers.txt is concerned.
Well, in kernel code for ARM in [1] writel() issues _iowmb() I/O write memory barrier that maps to wmb().
But I agree that in describing the example of interest in this document, it is better to stay away from the detail description and let writeX() explain it.
[1] https://elixir.bootlin.com/linux/latest/source/arch/arm/include/asm/io.h#L290
> They are listed in the "CPU MEMORY
> BARRIERS" section. dma_wmb() belongs to "advanced barrier functions".
>
> You see, writel() is one of the functions listed in the "KERNEL I/O BARRIER
> EFFECTS" section.
>
> Please be consistent with the word choice of the doc you are modifying, so
> that any further confusion can be avoided in this infamously hard-to-follow
> document. :-)
>
I understood.
I will rephrase it as "Note that, when using writel(), a prior barrier is not needed"...
Hi Joel,
> From: Joel Fernandes <[email protected]>
> Sent: Friday, September 30, 2022 7:41 AM
>
> Hi,
>
> > On Sep 29, 2022, at 10:04 PM, Parav Pandit <[email protected]> wrote:
> >
> > The cited commit describes that when using writel(), explcit wmb() is
> > not needed. wmb() is an expensive barrier. writel() uses the needed
> > I/O barrier instead of expensive wmb().
>
> Because you mentioned it in the commit message, Why not mention in the
> documentation text as well that writel() has the needed I/O barrier in it?
>
It is already documented in [1].
Though [1] is doesn’t describe about using I/O barrier.
To keep [1] and above example in sync, and to address Akira's comment [2], it makes sense to drop I/O barrier from the above example and reword the line as below
"Note that, when using writel(), a prior barrier is not needed".
[1] https://elixir.bootlin.com/linux/latest/source/Documentation/memory-barriers.txt#L2559
[2] https://lore.kernel.org/lkml/[email protected]/
> >
> > Hence update the example to be more accurate that matches the current
> > implementation.
>
> That would make it more accurate, since accuracy is your goal.
>
Yes. Above modified text looks more aligned to writeX() doc.
Will send v2.