{"id":773,"url":"http://patchwork.ozlabs.org/api/1.0/patches/773/?format=json","project":{"id":7,"url":"http://patchwork.ozlabs.org/api/1.0/projects/7/?format=json","name":"Linux network development","link_name":"netdev","list_id":"netdev.vger.kernel.org","list_email":"netdev@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null},"msgid":"<20080920.225033.261544815.davem@davemloft.net>","date":"2008-09-21T05:50:33","name":"[take,2] pkt_sched: Fix qdisc_watchdog() vs. dev_deactivate() race","commit_ref":null,"pull_url":null,"state":"rejected","archived":true,"hash":"b3bc6f38dc4b98638201c276b35802f899c397c4","submitter":{"id":15,"url":"http://patchwork.ozlabs.org/api/1.0/people/15/?format=json","name":"David Miller","email":"davem@davemloft.net"},"delegate":{"id":34,"url":"http://patchwork.ozlabs.org/api/1.0/users/34/?format=json","username":"davem","first_name":"David","last_name":"Miller","email":"davem@davemloft.net"},"mbox":"http://patchwork.ozlabs.org/project/netdev/patch/20080920.225033.261544815.davem@davemloft.net/mbox/","series":[],"check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/773/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Received":["from vger.kernel.org (vger.kernel.org [209.132.176.167])\n\tby ozlabs.org (Postfix) with ESMTP id 6B486DDE2A\n\tfor ;\n\tSun, 21 Sep 2008 15:59:15 +1000 (EST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751007AbYIUFuq (ORCPT );\n\tSun, 21 Sep 2008 01:50:46 -0400","(majordomo@vger.kernel.org) by vger.kernel.org id S1751119AbYIUFuq\n\t(ORCPT ); Sun, 21 Sep 2008 01:50:46 -0400","from 74-93-104-97-Washington.hfc.comcastbusiness.net\n\t([74.93.104.97]:39756\n\t\"EHLO sunset.davemloft.net\" rhost-flags-OK-FAIL-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750997AbYIUFup (ORCPT\n\t); Sun, 21 Sep 2008 01:50:45 -0400","from localhost (localhost [127.0.0.1])\n\tby sunset.davemloft.net (Postfix) with ESMTP id 933A3C8C181;\n\tSat, 20 Sep 2008 22:50:33 -0700 (PDT)"],"Date":"Sat, 20 Sep 2008 22:50:33 -0700 (PDT)","Message-Id":"<20080920.225033.261544815.davem@davemloft.net>","To":"jarkao2@gmail.com","Cc":"herbert@gondor.apana.org.au, netdev@vger.kernel.org,\n\tkaber@trash.net","Subject":"Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs.\n\tdev_deactivate() race","From":"David Miller ","In-Reply-To":"<20080920.223538.130375517.davem@davemloft.net>","References":"<20080920.002137.108837580.davem@davemloft.net>\n\t<20080920234843.GA2531@ami.dom.local>\n\t<20080920.223538.130375517.davem@davemloft.net>","X-Mailer":"Mew version 6.1 on Emacs 22.1 / Mule 5.0 (SAKAKI)","Mime-Version":"1.0","Content-Type":"Text/Plain; charset=us-ascii","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"},"content":"From: David Miller \nDate: Sat, 20 Sep 2008 22:35:38 -0700 (PDT)\n\n> Therefore it seems logical that what really needs to happen is that\n> we simply pick some new local special token value for 'ret' so that\n> we can handle that case. \"-1\" would probably work fine.\n ...\n> I also think the qdisc_run() test needs to be there. When the TX\n> queue fills up, we will doing tons of completely useless work going:\n\nOk, here is the kind of thing I'm suggesting in all of this.\n\nIt gets rid of bouncing unnecessarily into __qdisc_run() when\ndev_queue_xmit()'s finally selected TXQ is stopped.\n\nIt also gets rid of the dev_requeue_skb() looping case Jarek\ndiscovered.\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe netdev\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","diff":"diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h\nindex b786a5b..4082f39 100644\n--- a/include/net/pkt_sched.h\n+++ b/include/net/pkt_sched.h\n@@ -90,10 +90,7 @@ extern void __qdisc_run(struct Qdisc *q);\n \n static inline void qdisc_run(struct Qdisc *q)\n {\n-\tstruct netdev_queue *txq = q->dev_queue;\n-\n-\tif (!netif_tx_queue_stopped(txq) &&\n-\t !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))\n+\tif (!test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))\n \t\t__qdisc_run(q);\n }\n \ndiff --git a/net/core/dev.c b/net/core/dev.c\nindex fdfc4b6..4654127 100644\n--- a/net/core/dev.c\n+++ b/net/core/dev.c\n@@ -1809,7 +1809,15 @@ gso:\n \t\t\trc = NET_XMIT_DROP;\n \t\t} else {\n \t\t\trc = qdisc_enqueue_root(skb, q);\n-\t\t\tqdisc_run(q);\n+\n+\t\t\ttxq = NULL;\n+\t\t\tif (rc == NET_XMIT_SUCCESS) {\n+\t\t\t\tint map = skb_get_queue_mapping(skb);\n+\t\t\t\ttxq = netdev_get_tx_queue(dev, map);\n+\t\t\t}\n+\n+\t\t\tif (!txq || !netif_tx_queue_stopped(txq))\n+\t\t\t\tqdisc_run(q);\n \t\t}\n \t\tspin_unlock(root_lock);\n \ndiff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c\nindex ec0a083..b6e6926 100644\n--- a/net/sched/sch_generic.c\n+++ b/net/sched/sch_generic.c\n@@ -117,7 +117,7 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,\n static inline int qdisc_restart(struct Qdisc *q)\n {\n \tstruct netdev_queue *txq;\n-\tint ret = NETDEV_TX_BUSY;\n+\tint ret = -2;\n \tstruct net_device *dev;\n \tspinlock_t *root_lock;\n \tstruct sk_buff *skb;\n@@ -158,7 +158,7 @@ static inline int qdisc_restart(struct Qdisc *q)\n \t\tif (unlikely (ret != NETDEV_TX_BUSY && net_ratelimit()))\n \t\t\tprintk(KERN_WARNING \"BUG %s code %d qlen %d\\n\",\n \t\t\t dev->name, ret, q->q.qlen);\n-\n+\tcase -2:\n \t\tret = dev_requeue_skb(skb, q);\n \t\tbreak;\n \t}\n","prefixes":["take","2"]}