Re: LLTX and netif_stop_queue

[Eric Lemoine <eric.lemoine@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On 19 Dec 2004 14:31:15 -0500, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2004-12-18 at 00:44, David S. Miller wrote:
> 
> > Perhaps one way to fix this is to add a pointer to a spinlock to
> > the netdev struct, and have hold that the upper level grab that
> > when NETIF_F_LLTX when doing queue state checks.  Actually, that
> > could end up being racy too.
> 
> How about releasing the qlock only when the LLTX transmit lock is
> grabbed? That should bring it to par with what it was originally.

I dont like the idea of releasing inside the driver a lock taken
outside. That might be just me...

Instead, I would suggest to have LLTX drivers check whether queue is
stopped after they grab their private tx lock and before they check tx
ring fullness. That way we close the race window but keep the driver
bug check around.

See attached sungem patch.

-- 
Eric

[-- Attachment #2: sungem-lltx.patch --]
[-- Type: application/octet-stream, Size: 511 bytes --]

===== drivers/net/sungem.c 1.71 vs edited =====
--- 1.71/drivers/net/sungem.c	2004-11-14 10:45:36 +01:00
+++ edited/drivers/net/sungem.c	2004-12-22 19:34:07 +01:00
@@ -976,6 +976,12 @@
 		return NETDEV_TX_LOCKED;
 	}
 
+	/* This handles a LLTX-related race condition */
+	if (netif_queue_stopped(dev)) {
+		spin_unlock_irqrestore(&gp->tx_lock, flags);
+		return NETDEV_TX_BUSY;
+	}
+
 	/* This is a hard error, log it. */
 	if (TX_BUFFS_AVAIL(gp) <= (skb_shinfo(skb)->nr_frags + 1)) {
 		netif_stop_queue(dev);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]