From patchwork Tue Sep 23 08:02:40 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [take,2] pkt_sched: Fix qdisc_watchdog() vs. dev_deactivate() race Date: Mon, 22 Sep 2008 22:02:40 -0000 From: Jarek Poplawski X-Patchwork-Id: 1027 Message-Id: <20080923080240.GA4692@ff.dom.local> To: David Miller Cc: herbert@gondor.apana.org.au, netdev@vger.kernel.org, kaber@trash.net On Mon, Sep 22, 2008 at 10:16:58PM -0700, David Miller wrote: > From: Jarek Poplawski > 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 --- 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 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); }