netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roland Dreier <roland@topspin.com>
To: netdev@oss.sgi.com, davem@redhat.com
Cc: openib-general@openib.org
Subject: LLTX and netif_stop_queue
Date: Fri, 17 Dec 2004 13:57:40 -0800	[thread overview]
Message-ID: <52llbwoaej.fsf@topspin.com> (raw)

While testing my IP-over-InfiniBand driver, I discovered that if a net
device sets NETIF_F_LLTX, it seems the device's hard_start_xmit method
can be called even after a netif_stop_queue().

This is because in the LLTX case, qdisc_restart() holds no locks while
calling hard_start_xmit, so something like the following can happen:

    CPU 1                                CPU 2

    qdisc_restart:
      drop queue lock
      call hard_start_xmit()
        net driver:
          acquire TX lock
          queue packet to HW

                                        acquire queue lock...
                                        qdisc_restart:
                                          drop queue lock
                                          call hard_start_xmit:

          queue full, call netif_stop_queue()
          release TX lock

                                            net driver:
                                              acquire TX lock
                                              queue is already full!

Is my understanding correct?  If so it seems the patch below would
make sense.  (e1000 seems to handle this properly already)

Thanks,
  Roland


Since tg3 and sungem now use lockless TX (NETIF_F_LLTX), it's possible
for their hard_start_xmit method to be called even after they call
netif_stop_queue.  Therefore a full queue no longer indicates a bug --
this patch fixes the comment and removes the KERN_ERR printk.

Signed-off-by: Roland Dreier <roland@topspin.com>

Index: linux-bk/drivers/net/sungem.c
===================================================================
--- linux-bk.orig/drivers/net/sungem.c	2004-12-16 15:56:19.000000000 -0800
+++ linux-bk/drivers/net/sungem.c	2004-12-17 13:46:43.307064457 -0800
@@ -976,12 +976,10 @@
 		return NETDEV_TX_LOCKED;
 	}
 
-	/* This is a hard error, log it. */
+	/* This may happen, since we have NETIF_F_LLTX set */
 	if (TX_BUFFS_AVAIL(gp) <= (skb_shinfo(skb)->nr_frags + 1)) {
 		netif_stop_queue(dev);
 		spin_unlock_irqrestore(&gp->tx_lock, flags);
-		printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
-		       dev->name);
 		return NETDEV_TX_BUSY;
 	}
 
Index: linux-bk/drivers/net/tg3.c
===================================================================
--- linux-bk.orig/drivers/net/tg3.c	2004-12-16 15:56:06.000000000 -0800
+++ linux-bk/drivers/net/tg3.c	2004-12-17 13:46:25.952622672 -0800
@@ -3076,12 +3076,10 @@
 		return NETDEV_TX_LOCKED; 
 	} 
 
-	/* This is a hard error, log it. */
+	/* This may happen, since we have NETIF_F_LLTX set */
 	if (unlikely(TX_BUFFS_AVAIL(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
 		netif_stop_queue(dev);
 		spin_unlock_irqrestore(&tp->tx_lock, flags);
-		printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
-		       dev->name);
 		return NETDEV_TX_BUSY;
 	}
 

             reply	other threads:[~2004-12-17 21:57 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-12-17 21:57 Roland Dreier [this message]
2004-12-18  5:44 ` LLTX and netif_stop_queue David S. Miller
2004-12-18 15:35   ` Roland Dreier
2004-12-18 17:58     ` Roland Dreier
2004-12-18 18:26       ` Roland Dreier
2004-12-19 19:33     ` jamal
2004-12-19 19:31   ` jamal
2004-12-19 19:54     ` jamal
2004-12-19 20:02       ` Jamal Hadi Salim
2004-12-19 22:35     ` Roland Dreier
2004-12-19 23:06       ` jamal
2004-12-19 23:16         ` Roland Dreier
2004-12-22 18:49     ` Eric Lemoine
2004-12-23  4:29       ` David S. Miller
2004-12-23  4:38         ` Roland Dreier
2004-12-23  9:10         ` Eric Lemoine
2004-12-23 16:37           ` Patrick McHardy
2004-12-23 18:11             ` Eric Lemoine
2004-12-24 16:10             ` Eric Lemoine
2004-12-28 13:31               ` jamal
2005-01-02 23:30                 ` Eric Lemoine
2005-01-03  7:41                   ` Eric Lemoine
2005-01-03 15:04                   ` jamal
2005-01-03 15:48                     ` Eric Lemoine
2005-01-03 15:57                     ` Roland Dreier
2005-01-03 16:41                       ` Eric Lemoine
2005-01-03 16:54                         ` Roland Dreier
2005-01-03 17:07                           ` Eric Lemoine
2005-01-03 17:12                             ` Grant Grundler
2005-01-04  4:18                               ` jamal
2005-01-19 22:47                                 ` David S. Miller
2005-01-19 23:18                                   ` Stephen Hemminger
2005-01-19 23:41                                     ` David S. Miller
2005-01-20  0:02                                       ` [openib-general] " Jeff Garzik
2005-01-20  0:46                                         ` Stephen Hemminger
2005-01-20  0:47                                           ` David S. Miller
2005-01-20  0:47                                       ` Francois Romieu
2005-01-20  0:52                                         ` David S. Miller
2005-01-20  1:17                                           ` Francois Romieu
2005-01-20  0:46                                     ` [PATCH]: was " David S. Miller
2005-01-20  3:14                                       ` Andi Kleen
2005-01-20  7:05                                         ` David S. Miller
2005-01-20  3:43                                       ` Roland Dreier
2005-01-20  7:05                                         ` David S. Miller
2005-01-20 13:51                                           ` Tommy Christensen
2005-01-20 21:34                                             ` David S. Miller
2005-01-20 21:56                                               ` Grant Grundler
2005-01-21  1:01                                                 ` David S. Miller
2005-01-22  3:17                                                   ` Roland Dreier
2005-01-22  3:53                                                     ` David S. Miller
2005-01-20 21:41                                             ` David S. Miller
2005-01-20 16:56                                           ` Stephen Hemminger
2005-01-21 10:54                                             ` Lennert Buytenhek
2005-01-26  6:27                                               ` David S. Miller
2005-01-26 13:25                                                 ` Lennert Buytenhek
2005-01-27  6:32                                                   ` David S. Miller
2005-01-27  7:16                                                     ` Andi Kleen
2005-01-27  7:22                                                       ` David S. Miller
2005-01-27  8:26                                                         ` Andi Kleen
2005-01-20 19:16                                           ` Jeff Garzik
2005-01-20  4:01                                       ` jamal
2005-01-20  5:18                                         ` David S. Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52llbwoaej.fsf@topspin.com \
    --to=roland@topspin.com \
    --cc=davem@redhat.com \
    --cc=netdev@oss.sgi.com \
    --cc=openib-general@openib.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).