From: Stephen Hemminger <[EMAIL PROTECTED]>
Date: Mon, 5 Dec 2005 16:37:53 -0800
> I assert that with lockless transmit (ie NETIF_F_LLTX) it is
> possible for two cpu's to race with the transmit flow control
> (netif_stop_queue/netif_wake_queue). I see it with sky2, and
> probably could reproduce with tg3 as well.
>
> A. CPU is in middle of building up a transmit,
> B. CPU gets packet and passes
> netif_queue_stopped test
> A. CPU completes transmit and since tx queue is full sets stopped
> B. tests for tx queue full and....
>
> This will cause the driver to return DEVICE_BUSY
> Tg3, skge, sky2 and rionet all print message, that this is a BUG!
> but it is just losing a corner case race, and should be silent.
I totally agree, but would recommend something like the patch below,
using tg3 as an example.
We should still warn if the transmit ring is full _and_ netif_stop_queue()
is false. That is an illegal state to observe while holding the tx_lock.
If netif_stop_queue() is true, then it's just the race you describe above,
and should just silently return NETDEV_TX_BUSY, as you recommend.
Agreed?
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 1828a6b..d9f65f4 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3565,12 +3565,20 @@ static int tg3_start_xmit(struct sk_buff
if (!spin_trylock(&tp->tx_lock))
return NETDEV_TX_LOCKED;
- /* This is a hard error, log it. */
if (unlikely(TX_BUFFS_AVAIL(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
- netif_stop_queue(dev);
+ int was_stopped = netif_queue_stopped(dev);
+
+ if (!was_stopped)
+ netif_stop_queue(dev);
+
spin_unlock(&tp->tx_lock);
- printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
- dev->name);
+
+ if (!was_stopped) {
+ /* This is a hard error, log it. */
+ printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
+ "queue awake!\n", dev->name);
+ }
+
return NETDEV_TX_BUSY;
}
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
