Subject: Re: [PATCH] sky2: safer transmit ring cleaning (v2)

From: Stephen Hemminger <[email-masked]>
Date: Tue, 12 Jan 2010 17:23:11 -0800

On Tue, 12 Jan 2010 20:50:04 +0100
Jarek Poplawski <[email-masked]> wrote:

> On Tue, Jan 12, 2010 at 11:23:14AM -0800, Stephen Hemminger wrote:
> > On Tue, 12 Jan 2010 20:16:11 +0100
> > Jarek Poplawski <[email-masked]> wrote:
> > 
> > > > 
> > > > What is supposed to happen:
> > > >   * restart sky2_restart calls napi_disable while cleaning  
> > > 
> > > Yes, but it's after the detach; similarly to sky2_suspend().
> > > (I'm not sure how safe vs such re-enabling is sky2_set_ringparam().
> > 
> > set_ringparam happens under rtnl_lock() so reset and ringparams can't
> > conflict.
> 
> I didn't mean reset. I meant tx (dev_queue_xmit()) during ringparams.

sky2_detach does
   tx_lock
   netif_detach() -- stops the queue
   tx_unlock

So if another CPU was about to send, it will have to wait
behind the tx_global_lock, and then it will see the queue as frozen.

BUT, you prod me to look harder and there is a race with
softirq (bottom half) here that needs to be fixed.