On Fri, 2015-08-28 at 11:09 +0300, Eugene Shatokhin wrote:

quoted

Exactly what problem will that result in?  The tasklet_kill() will

wait

quoted

for the processing of the single element done queue, and everything

will

quoted

be fine.  Or?

Given enough time, what prevents defer_bh() from calling 
tasklet_schedule(&dev->bh) *after* usbnet_stop() calls tasklet_kill()?

Consider the following situation (assuming '&&' are changed to '||'
in 
that while loop in usbnet_terminate_urbs() as they should be):

CPU0                            CPU1
usbnet_stop()                   defer_bh() with list == dev->rxq
   usbnet_terminate_urbs()
                                 __skb_unlink() removes the last
                                 skb from dev->rxq.
                                 dev->rxq, dev->txq and dev->done
                                 are now empty.
   while (!skb_queue_empty()...)
     The loop ends because all 3
     queues are now empty.

   usbnet_terminate_urbs() ends.

usbnet_stop() continues:
   usbnet_status_stop(dev);
   ...
   del_timer_sync (&dev->delay);
   tasklet_kill (&dev->bh);
                                 __skb_queue_tail(&dev->done, skb);
                                 if (dev->done.qlen == 1)
                                   tasklet_schedule(&dev->bh);

The BH is scheduled at this point, which is not what was intended.
The 
race window is small, but still.

This looks possible. I cannot come up with a better fix, although
it isn't nice. Thanks for finding this.

	Regards
		Oliver