Re: [PATCH] tg3: Fix tx race condition

["Michael Chan" <mchan@broadcom.com>]

On Tue, 2006-08-08 at 12:51 +1000, Herbert Xu wrote:

> > 2. Eliminate the private tx_lock altogether and rely solely on
> > netif_tx_lock.  This eliminates one spinlock in tg3_start_xmit()
> > when the ring is full.
> 
> Why not get rid of the lock altogether? By making sure memory
> barriers are present on the stop/wake paths the locks are not
> needed anymore.

Without the lock, I think you can still wake the queue with an empty
ring in some cases.  Although we handle it properly by returning
NETDEV_TX_BUSY, it is considered a BUG condition in tg3.

> 
> > 4. Make tx_cons and tx_prod volatile to guarantee that
> > tg3_start_xmit() and tg3_tx() will see the latest ring indices.
> 
> Such uses of volatile are bad inside the kernel because they
> don't actually provide hardware memory barriers and their effect
> on the compiler are given by constructs such as mb() anyway.
> 

Just trying to prevent the compiler from optimizing the code.  In cases
where we need hardware barriers we use mb.

> So we should simply add memory barriers where needed.
> 

I can do that.  It will be more mb() scattered in the driver.  Actually,
I can probably hide it in the TX_BUFFS_AVAIL() macro.