Re: RFC on writel and writel_relaxed

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Mon, 2018-03-26 at 16:27 -0600, Jason Gunthorpe wrote:
> > Otherwise almost all drivers out there are broken which I very much
> > doubt :-)
> 
> But.. Sinan is right, you look anywhere in the driver tree and you
> find stuff like this:
> 
> drivers/net/ethernet/intel/i40e/i40e_txrx.c
> 
>         /* Force memory writes to complete before letting h/w
>          * know there are new descriptors to fetch.
>          */
>         wmb();
> 
> 
> It is *systemic*

Yes, because they all copied e1000e :-) If you look at the comment in
there, it does say it's only for weakly ordered archs such as ia64, and
even then, probably predates Linus strong statement on the matter.

> I even see patches adding wmb() based on actual observed memory
> corruption during testing on Intel:
> 
> https://patchwork.kernel.org/patch/10177207/
> 
> So you think all of this is unnecessary and writel is totally strongly
> ordered, even on multi-socket Intel?

I don't kow, it used to be the case, at least that's what drove us to
define things the way we did.

Maybe things changed, but if that's the case, nobody knows for sure,
and we probably want to get Linus POV on the matter.

I know I still write drivers that do not add a wmb in that case because
I expect things to work without it.

If that has changed, we probably can relax some of the barriers in our
implementations of writel on a number of architectures, but not before
auditing a bunch more drivers to make sure they have the write wmb()'s

Cheers,
Ben.