On Tue, Mar 27, 2018 at 09:01:57AM +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2018-03-26 at 17:46 -0400, Sinan Kaya wrote:
> > On 3/26/2018 5:30 PM, Arnd Bergmann wrote:
> > > > But that was never a requirement of writel(),
> > > > Documentation/memory-barriers.txt gives an explicit example demanding
> > > > the wmb() before writel() for ordering system memory against writel.
> > >
> > > Indeed, but it's in an example for when to use dma_wmb(), not wmb().
> > > Adding Alexander Duyck to Cc, he added that section as part of
> > > 1077fa36f23e ("arch: Add lightweight memory barriers dma_rmb() and
> > > dma_wmb()"). Also adding the other people that were involved with that.
> > >
> >
> > ARM developers can get away with not including wmb() in their code and use
> > writel() to observe memory writes due to implicit barriers.
> >
> > However, same code will not work on Intel.
>
> Wrong. It will.
>
> You do NOT need wmb between writes to memory and writel.
>
> > writel() has a compiler barrier in it for x86.
> > wmb() has a sync operation in it for x86.
> >
> > Unless wmb() is called, PCIe device won't observe memory updates from the CPU.
>
> This is completely wrong. They will. Intel provides the necessary
> ordering guarantees without an explicit wmb.
>
> 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*
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?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
