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

[Kumar Gala <galak@kernel.crashing.org>]

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

> Kumar Gala wrote:
>> On May 2, 2007, at 3:12 PM, Scott Wood wrote:
>>> 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.
>>> 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