Re: [PATCH net-next] r8169: Support for byte queue limits

[Eric Dumazet <eric.dumazet@gmail.com>]

Le vendredi 02 décembre 2011 à 15:54 +0100, Igor Maravić a écrit :
> > Hmm... What about rtl8169_tx_clear_range() or rtl8169_tx_clear() ?
> >
> 
> You mean we should put netdev_reset_queue() in those functions?
> 
> I think that we shouldn't put any thing in these functions.
> 
> When ever they are called, they are precede/followed by function
> rtl8169_hw_reset() which calls rtl_hw_reset() which calls
> rtl8169_init_ring_indexes() in which I've put netdev_reset_queue()
> 
> Only time when rtl8169_tx_clear_range() isn't preceded/followed by
> rtl8169_hw_reset() is in function rtl8169_xmit_frags(). In that
> function it's
> called only in case there's an error. In case there was an error in that
> function, netdev_sent_queue() will never be called, so we are going to have
> accurate length of queue.

Quite frankly, any queued skb is eventually freed, so
netdev_reset_queue() could be avoided.

r8169 driver has some problems (see current threads on netdev about it),
and its not very clear that BQL locking requirements are going to be
respected. This can lead to inaccurate BQL counts and a txqueue freeze.

(In particular, a chip reset can be triggered by an IRQ, while another
cpu is currently sending an skb through ndo_start_xmit())

So I advise not using netdev_reset_queue() in this driver, only
netdev_completed_queue(), and make sure that their calls are serialized
under a given lock (see include/linux/dynamic_queue_limits.h comments)