On Tue, Mar 27, 2018 at 10:59:40AM +1100, Benjamin Herrenschmidt wrote:

On Mon, 2018-03-26 at 16:50 -0600, Jason Gunthorpe wrote:

quoted

On Tue, Mar 27, 2018 at 09:36:11AM +1100, Benjamin Herrenschmidt wrote:

quoted

On Mon, 2018-03-26 at 16:27 -0600, Jason Gunthorpe wrote:

quoted

quoted

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.

Hahah, sure I'll buy that..

But still, if this really is the case, a *strong* statement in
barriers.txt to that effect (and not an example demanding the wmb()!)
would be very helpful for those of us that have to review driver code!

I agree, and that Mellanox bug you pointed me to seems to indicate that
this may not even be true on x86 anymore ...

However, with bugs like that it is hard to know what is going on.. It
could be a CPU bug instead.

I think we might need to revisit this properly...

I would love to hear a definitive statement from Intel on what wmb();
writel(); does on x86..

Sinan's patches are backwards if writel is ordered, instead of
using writel_relaxed, they should be eliminating the wmb().

But there is no way patches like that could go ahead until
barriers.txt is updated..

Jason