linux-kernel - Re: [PATCH v4] locking/memory-barriers.txt: Improve documentation for writel() example

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Oct 2022 10:37:15 +0900
From:   Akira Yokosawa <akiyks@...il.com>
To:     Arnd Bergmann <arnd@...db.de>, Parav Pandit <parav@...dia.com>
Cc:     bagasdotme@...il.com, Alan Stern <stern@...land.harvard.edu>,
        parri.andrea@...il.com, Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>, boqun.feng@...il.com,
        Nicholas Piggin <npiggin@...il.com>, dhowells@...hat.com,
        j.alglave@....ac.uk, luc.maranget@...ia.fr,
        "Paul E. McKenney" <paulmck@...nel.org>, dlustig@...dia.com,
        Joel Fernandes <joel@...lfernandes.org>,
        Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
        Linux-Arch <linux-arch@...r.kernel.org>,
        linux-doc@...r.kernel.org, Akira Yokosawa <akiyks@...il.com>
Subject: Re: [PATCH v4] locking/memory-barriers.txt: Improve documentation for
 writel() example

On 2022/10/18 5:55, Arnd Bergmann wrote:
> On Mon, Oct 10, 2022, at 12:13 PM, 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
>> platform specific 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 <parav@...dia.com>
> 
> I have no objections, though I still don't see a real need to change
> the wording here.

Parav, I think you need a full rewrite of the Changelog as the change
has become a simple substitution of s/wmb()/barrier/.

In second thought, I'm not sure such a substitution is really safe to
make.

"a barrier" can mean "any barrier", which can include a full barrier
in theory.

So I'd rather make the substituted text read something like:

  Note that, when using writel(), a prior wmb() or weaker is not
  needed to guarantee that the cache coherent memory writes have
  completed before writing to the MMIO region.

In my opinion, "or weaker" is redundant for careful readers who are
well aware of context of this example, but won't do no harm.

Thoughts?

        Thanks, Akira

> 
> Acked-by: Arnd Bergmann <arnd@...db.de>