Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-935-ge4ccd4c47b-fm-20220914.001-ge4ccd4c4
Mime-Version: 1.0
Message-Id: <a8ee97f5-b92f-47a6-9b50-197974738ff7@www.fastmail.com>
In-Reply-To: <PH0PR12MB5481192DB7B5C6E19683D514DC499@PH0PR12MB5481.namprd12.prod.outlook.com>
References: <20220915050106.650813-1-parav@nvidia.com>
 <96457b14-e196-4f29-be9a-7fa25ac805d9@www.fastmail.com>
 <PH0PR12MB5481192DB7B5C6E19683D514DC499@PH0PR12MB5481.namprd12.prod.outlook.com>
Date:   Thu, 15 Sep 2022 17:16:24 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Parav Pandit" <parav@nvidia.com>,
        "stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
        "parri.andrea@gmail.com" <parri.andrea@gmail.com>,
        "Will Deacon" <will@kernel.org>,
        "Peter Zijlstra" <peterz@infradead.org>,
        "boqun.feng@gmail.com" <boqun.feng@gmail.com>,
        "Nicholas Piggin" <npiggin@gmail.com>,
        "dhowells@redhat.com" <dhowells@redhat.com>,
        "j.alglave@ucl.ac.uk" <j.alglave@ucl.ac.uk>,
        "luc.maranget@inria.fr" <luc.maranget@inria.fr>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        "akiyks@gmail.com" <akiyks@gmail.com>,
        "Dan Lustig" <dlustig@nvidia.com>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>,
        "Jonathan Corbet" <corbet@lwn.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Linux-Arch <linux-arch@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [PATCH] locking/memory-barriers.txt: Improve documentation for writel()
 usage
Content-Type: text/plain
Precedence: bulk

On Thu, Sep 15, 2022, at 4:18 PM, Parav Pandit wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> Sent: Thursday, September 15, 2022 8:38 AM
>> 
>> On Thu, Sep 15, 2022, at 7:01 AM, Parav Pandit wrote:
>> > The cited commit [1] describes that when using writel(), explcit wmb()
>> > is not needed. However, it should have said that dma_wmb() is not
>> > needed.
>> 
>> Are you sure? As I understand it, the dma_wmb() only serializes a set of
>> memory accesses, but does not serialized against an MMIO access, which
>> depending on the CPU architecture may require a different type of barrier.
>> 
>> E.g. on arm, writel() uses __iowmb(), which like wmb() is defined as "dsb(x);
>> arm_heavy_mb();", while dma_wmb() is a "dmb(oshst)".
>
> You are right, on arm heavy barrier dsb() is needed, while on arm64, 
> dmb(oshst) is sufficient.
>
> So more accurate documentation is to say that 
> 'when using writel() a prior IO barrier is not needed ...'
>
> How about that?

That's probably fine, not sure if it's worth changing.

> It started with my cleanup efforts to two drivers [1] and [2] that had 
> difficulty in using writel() on 32-bit system, and it ended up open 
> coding writel() as wmb() + mlx5_write64().
>
> I am cleaning up the repetitive pattern of, 
> wmb();
> mlx5_write64()
>
> Before I fix drivers, I thought to improve the documentation that I can 
> follow. :)

Right, that is definitely a good idea.

I see that there is more going on with that function, at least
the loop in post_send_nop() probably just wants to use
__iowrite64_copy(), but that also has no barrier in it, while
changing mlx5_write64() to use iowrite64be() or similar would
of course add excessive barriers inside of the loop.

       Arnd