Re: [PATCH v2] gianfar: Add I/O barriers when touching buffer descriptor ownership.

On May 2, 2007, at 3:40 PM, Scott Wood wrote:

Kumar Gala wrote:

quoted

On May 2, 2007, at 3:12 PM, Scott Wood wrote:

quoted

wmb() is a sync, smp_wmb() is an eieio.  Andy told me he would not  accept a sync in those spots.

Sorry, was looking at the iobarrier code.

quoted

And the driver is already ppc-specific; it uses in/out_be32.

True, but its hidden behind the gfar_read/write accessors.
Your change is a bit more blatant.

Well, Segher doesn't want me to use iobarrier (because it's not I/O). Andy doesn't want me to use wmb() (because it's sync).  I don't think something like gfar_wmb() would be appropriate.  So the remaining options are either eieio(), or a new non-arch-specific, non-driver-specific mem_wmb() (or whatever).

While I like the latter option, I don't think this bugfix should have to wait for it.

Ok, I've resigned to the eieio's.  If we end up respinning the patch again for any reason I'd like to see something in the commit comment to the fact that we are adding ppc specific sync operations.

Why doesn't marking the bdp pointer volatile resolve the issue in gfar_clean_rx_ring() to ensure load ordering?

- k