diff mbox

[take,2] pkt_sched: Fix qdisc_watchdog() vs. dev_deactivate() race

Message ID 20080923080240.GA4692@ff.dom.local
State Accepted, archived
Headers show

Commit Message

Jarek Poplawski Sept. 23, 2008, 8:02 a.m. UTC 
On Mon, Sep 22, 2008 at 10:16:58PM -0700, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Sun, 21 Sep 2008 13:15:51 +0200
> 
> > That's why I think you should reconsider this simple solution for now,
> > until somebody proves this is wrong or something else is better.
> 
> Ok, that sounds reasonable.  I've added those three patches to net-next-2.6
> and will push those out after some build tests.

OK, then we have to say B and try this all. BTW, I guess, after this
change we could have similar effect as reported by Alexander Duyck
while testing his solution for this problem, namely the higher drop
rate in some cases, which I can only explain as: less time in
requeuing more time for new enqueuing. Of course, if I'm right, this
"bug" should be rather "fixed" with longer queues or some other
throttle mechanism.

Thanks,
Jarek P.

-------------------->

pkt_sched: Remove the tx queue state check in qdisc_run()

The current check wrongly uses the state of one (currently the first)
tx queue for all tx queues in case of non-default qdiscs. This check
mainly prevented requeuing loop with __netif_schedule(), but now it's
controlled inside __qdisc_run(), while dequeuing. The wrongness of
this check was first noticed by Herbert Xu.

Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

---

 include/net/pkt_sched.h |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Sept. 23, 2008, 8:06 a.m. UTC | #1 
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Tue, 23 Sep 2008 08:02:40 +0000

> OK, then we have to say B and try this all. BTW, I guess, after this
> change we could have similar effect as reported by Alexander Duyck
> while testing his solution for this problem, namely the higher drop
> rate in some cases, which I can only explain as: less time in
> requeuing more time for new enqueuing. Of course, if I'm right, this
> "bug" should be rather "fixed" with longer queues or some other
> throttle mechanism.
 ...
> pkt_sched: Remove the tx queue state check in qdisc_run()
> 
> The current check wrongly uses the state of one (currently the first)
> tx queue for all tx queues in case of non-default qdiscs. This check
> mainly prevented requeuing loop with __netif_schedule(), but now it's
> controlled inside __qdisc_run(), while dequeuing. The wrongness of
> this check was first noticed by Herbert Xu.
> 
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>

Agreed and applied, thanks Jarek.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index b786a5b..4082f39 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -90,10 +90,7 @@  extern void __qdisc_run(struct Qdisc *q);
 
 static inline void qdisc_run(struct Qdisc *q)
 {
-	struct netdev_queue *txq = q->dev_queue;
-
-	if (!netif_tx_queue_stopped(txq) &&
-	    !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))
+	if (!test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))
 		__qdisc_run(q);
 }