Re: [PATCH] locking/memory-barriers.txt: Improve documentation for writel() usage

["Arnd Bergmann" <arnd@arndb.de>]

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