Patch Detail

HTTP 200 OK
Allow: GET, PUT, PATCH, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "id": 596,
    "url": "http://patchwork.ozlabs.org/api/patches/596/?format=api",
    "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/20080919103225.GB9135@ff.dom.local/",
    "project": {
        "id": 7,
        "url": "http://patchwork.ozlabs.org/api/projects/7/?format=api",
        "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,
        "list_archive_url": "",
        "list_archive_url_format": "",
        "commit_url_format": ""
    },
    "msgid": "<20080919103225.GB9135@ff.dom.local>",
    "list_archive_url": null,
    "date": "2008-09-19T10:32:25",
    "name": "[take,2] Re: [RFC PATCH] sched: only dequeue if packet can be queued to hardware queue.",
    "commit_ref": null,
    "pull_url": null,
    "state": "rfc",
    "archived": true,
    "hash": "6025fb33ad7975df1de7e038c768ed03d1981716",
    "submitter": {
        "id": 277,
        "url": "http://patchwork.ozlabs.org/api/people/277/?format=api",
        "name": "Jarek Poplawski",
        "email": "jarkao2@gmail.com"
    },
    "delegate": {
        "id": 34,
        "url": "http://patchwork.ozlabs.org/api/users/34/?format=api",
        "username": "davem",
        "first_name": "David",
        "last_name": "Miller",
        "email": "davem@davemloft.net"
    },
    "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/20080919103225.GB9135@ff.dom.local/mbox/",
    "series": [],
    "comments": "http://patchwork.ozlabs.org/api/patches/596/comments/",
    "check": "pending",
    "checks": "http://patchwork.ozlabs.org/api/patches/596/checks/",
    "tags": {},
    "related": [],
    "headers": {
        "Return-Path": "<netdev-owner@vger.kernel.org>",
        "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 9A534DDE24\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri, 19 Sep 2008 20:32:49 +1000 (EST)",
            "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751294AbYISKcp (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 19 Sep 2008 06:32:45 -0400",
            "(majordomo@vger.kernel.org) by vger.kernel.org id S1751263AbYISKco\n\t(ORCPT <rfc822; netdev-outgoing>); Fri, 19 Sep 2008 06:32:44 -0400",
            "from fg-out-1718.google.com ([72.14.220.152]:38819 \"EHLO\n\tfg-out-1718.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750836AbYISKcn (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 19 Sep 2008 06:32:43 -0400",
            "by fg-out-1718.google.com with SMTP id 19so521143fgg.17\n\tfor <netdev@vger.kernel.org>; Fri, 19 Sep 2008 03:32:42 -0700 (PDT)",
            "by 10.180.220.5 with SMTP id s5mr3267395bkg.31.1221820361821;\n\tFri, 19 Sep 2008 03:32:41 -0700 (PDT)",
            "from ff.dom.local ( [80.53.205.170])\n\tby mx.google.com with ESMTPS id 13sm605292fks.6.2008.09.19.03.32.29\n\t(version=SSLv3 cipher=RC4-MD5);\n\tFri, 19 Sep 2008 03:32:40 -0700 (PDT)"
        ],
        "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\n\th=domainkey-signature:received:received:date:from:to:cc:subject\n\t:message-id:references:mime-version:content-type:content-disposition\n\t:in-reply-to:user-agent;\n\tbh=K4wK/dbjq+AjB8KiDRznyfQNnDavz6cp4by0KAY32hk=;\n\tb=EIXikcnLidsUkeV4Xgm30uivMPLUdCIrSPEXq4HVEoy5h97Z4E+y/1aMQIvVh8Vbfm\n\tYgG6Sm7E6Gnc85UGFZ71Q+ZF5Qpvk7I8kpuvxp3elezlKTHAtphs+xecfa01N0Stf2mX\n\twRAR+AapsvWKL5Y+1Kcc4Br0/9oty1MMixFPE=",
        "DomainKey-Signature": "a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-type:content-disposition:in-reply-to:user-agent;\n\tb=NQoVSOhPEODx7Fycug83hWqf73Z4LWt+dfmvrNoIgf9kroor9CAvLEeapXpof/HDXw\n\tUy93BjEArv+BBaKIEpQa7o70Efyq5EP3jnhbTVwAqajTY+kH3JXnTx66+wiuIPWSXNCZ\n\tFIqolR1+zAkRC+jgppSWp6rOkfypcI+tpugh4=",
        "Date": "Fri, 19 Sep 2008 10:32:25 +0000",
        "From": "Jarek Poplawski <jarkao2@gmail.com>",
        "To": "Alexander Duyck <alexander.duyck@gmail.com>",
        "Cc": "Alexander Duyck <alexander.h.duyck@intel.com>,\n\tnetdev@vger.kernel.org, herbert@gondor.apana.org.au,\n\tdavem@davemloft.net, kaber@trash.net",
        "Subject": "[take 2] Re: [RFC PATCH] sched: only dequeue if packet can be\n\tqueued to hardware queue.",
        "Message-ID": "<20080919103225.GB9135@ff.dom.local>",
        "References": "<20080918063036.27934.91273.stgit@localhost.localdomain>\n\t<20080918194419.GA2982@ami.dom.local>\n\t<5f2db9d90809181811q74c3211fp9a099acb8e895fd4@mail.gmail.com>",
        "MIME-Version": "1.0",
        "Content-Type": "text/plain; charset=us-ascii",
        "Content-Disposition": "inline",
        "In-Reply-To": "<5f2db9d90809181811q74c3211fp9a099acb8e895fd4@mail.gmail.com>",
        "User-Agent": "Mutt/1.5.18 (2008-05-17)",
        "Sender": "netdev-owner@vger.kernel.org",
        "Precedence": "bulk",
        "List-ID": "<netdev.vger.kernel.org>",
        "X-Mailing-List": "netdev@vger.kernel.org"
    },
    "content": "Take 2:\nI missed __netif_schedule() in my patch, so the update below.\n\nBut, because of this a little change of mind: there is no reason to\nrepeat __netif_schedule() if there is a full stop or freeze of all\nqueues, so it seems we need additional check for this anyway. I'll\nsend my proposal of this test soon.\n\nJarek P.\n\nOn Thu, Sep 18, 2008 at 06:11:45PM -0700, Alexander Duyck wrote:\n> Once again if you have a suggestion on approach feel free to modify\n> the patch and see how it works for you.  My only concern is that there\n> are several qdiscs which won't give you the same packet twice and so\n> you don't know what is going to pop out until you go in and check.\n...\n\nActually, here is my suggestion: I think, that your obviously more\ncomplex solution should be compared with something simple which IMHO\nhas similar feature, i.e. limited overhead of requeuing.\n\nMy proposal is to use partly David's idea of killing ->ops->requeue(),\nfor now only the first two patches, so don't requeue back to the\nqdiscs, but leave qdisc->ops->requeue() for internal usage (like in\nHFSC's qdisc_peek_len() hack). Additionaly I use your idea of early\nchecking the state of tx queue to make this even lighter after\npossibly removing the entry check from qdisc_run().\n\nI attach my patch at the end, after original David's two patches.\n\nThanks,\nJarek P.\n\n---------------> patch 1/3\nSubject: [PATCH 1/9]: pkt_sched: Make qdisc->gso_skb a list.\nDate: Mon, 18 Aug 2008 01:36:47 -0700 (PDT)\nFrom: David Miller <davem@davemloft.net>\nTo: netdev@vger.kernel.org\nCC: jarkao2@gmail.com\nNewsgroups: gmane.linux.network\n\n\npkt_sched: Make qdisc->gso_skb a list.\n\nThe idea is that we can use this to get rid of\n->requeue().\n\nSigned-off-by: David S. Miller <davem@davemloft.net>",
    "diff": "diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h\nindex 84d25f2..140c48b 100644\n--- a/include/net/sch_generic.h\n+++ b/include/net/sch_generic.h\n@@ -52,7 +52,7 @@ struct Qdisc\n \tu32\t\t\tparent;\n \tatomic_t\t\trefcnt;\n \tunsigned long\t\tstate;\n-\tstruct sk_buff\t\t*gso_skb;\n+\tstruct sk_buff_head\trequeue;\n \tstruct sk_buff_head\tq;\n \tstruct netdev_queue\t*dev_queue;\n \tstruct Qdisc\t\t*next_sched;\ndiff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c\nindex 6f96b7b..39d969e 100644\n--- a/net/sched/sch_generic.c\n+++ b/net/sched/sch_generic.c\n@@ -45,7 +45,7 @@ static inline int qdisc_qlen(struct Qdisc *q)\n static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)\n {\n \tif (unlikely(skb->next))\n-\t\tq->gso_skb = skb;\n+\t\t__skb_queue_head(&q->requeue, skb);\n \telse\n \t\tq->ops->requeue(skb, q);\n \n@@ -57,9 +57,8 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)\n {\n \tstruct sk_buff *skb;\n \n-\tif ((skb = q->gso_skb))\n-\t\tq->gso_skb = NULL;\n-\telse\n+\tskb = __skb_dequeue(&q->requeue);\n+\tif (!skb)\n \t\tskb = q->dequeue(q);\n \n \treturn skb;\n@@ -328,6 +327,7 @@ struct Qdisc noop_qdisc = {\n \t.flags\t\t=\tTCQ_F_BUILTIN,\n \t.ops\t\t=\t&noop_qdisc_ops,\n \t.list\t\t=\tLIST_HEAD_INIT(noop_qdisc.list),\n+\t.requeue.lock\t=\t__SPIN_LOCK_UNLOCKED(noop_qdisc.q.lock),\n \t.q.lock\t\t=\t__SPIN_LOCK_UNLOCKED(noop_qdisc.q.lock),\n \t.dev_queue\t=\t&noop_netdev_queue,\n };\n@@ -353,6 +353,7 @@ static struct Qdisc noqueue_qdisc = {\n \t.flags\t\t=\tTCQ_F_BUILTIN,\n \t.ops\t\t=\t&noqueue_qdisc_ops,\n \t.list\t\t=\tLIST_HEAD_INIT(noqueue_qdisc.list),\n+\t.requeue.lock\t=\t__SPIN_LOCK_UNLOCKED(noqueue_qdisc.q.lock),\n \t.q.lock\t\t=\t__SPIN_LOCK_UNLOCKED(noqueue_qdisc.q.lock),\n \t.dev_queue\t=\t&noqueue_netdev_queue,\n };\n@@ -473,6 +474,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,\n \tsch->padded = (char *) sch - (char *) p;\n \n \tINIT_LIST_HEAD(&sch->list);\n+\tskb_queue_head_init(&sch->requeue);\n \tskb_queue_head_init(&sch->q);\n \tsch->ops = ops;\n \tsch->enqueue = ops->enqueue;\n@@ -543,7 +545,7 @@ void qdisc_destroy(struct Qdisc *qdisc)\n \tmodule_put(ops->owner);\n \tdev_put(qdisc_dev(qdisc));\n \n-\tkfree_skb(qdisc->gso_skb);\n+\t__skb_queue_purge(&qdisc->requeue);\n \n \tkfree((char *) qdisc - qdisc->padded);\n }\n\n\n-------------> patch 2/3\nSubject: [PATCH 2/9]: pkt_sched: Always use q->requeue in dev_requeue_skb().\nDate: Mon, 18 Aug 2008 01:36:50 -0700 (PDT)\nFrom: David Miller <davem@davemloft.net>\nTo: netdev@vger.kernel.org\nCC: jarkao2@gmail.com\nNewsgroups: gmane.linux.network\n\n\npkt_sched: Always use q->requeue in dev_requeue_skb().\n\nThere is no reason to call into the complicated qdiscs\njust to remember the last SKB where we found the device\nblocked.\n\nThe SKB is outside of the qdiscs realm at this point.\n\nSigned-off-by: David S. Miller <davem@davemloft.net>\n---\n net/sched/sch_generic.c |    5 +----\n 1 files changed, 1 insertions(+), 4 deletions(-)\n\ndiff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c\nindex 39d969e..96d7d08 100644\n--- a/net/sched/sch_generic.c\n+++ b/net/sched/sch_generic.c\n@@ -44,10 +44,7 @@ static inline int qdisc_qlen(struct Qdisc *q)\n \n static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)\n {\n-\tif (unlikely(skb->next))\n-\t\t__skb_queue_head(&q->requeue, skb);\n-\telse\n-\t\tq->ops->requeue(skb, q);\n+\t__skb_queue_head(&q->requeue, skb);\n \n \t__netif_schedule(q);\n \treturn 0;\n\n\n----------------> patch 3/3 (take 2)\n\npkt_sched: Check the state of tx_queue in dequeue_skb()\n\nCheck in dequeue_skb() the state of tx_queue for requeued skb to save\non locking and re-requeuing, and possibly remove the current check in\nqdisc_run(). Based on the idea of Alexander Duyck.\n\nSigned-off-by: Jarek Poplawski <jarkao2@gmail.com>\n\n---\n\ndiff -Nurp net-next-2.6-/net/sched/sch_generic.c net-next-2.6+/net/sched/sch_generic.c\n--- net-next-2.6-/net/sched/sch_generic.c\t2008-09-19 07:21:44.000000000 +0000\n+++ net-next-2.6+/net/sched/sch_generic.c\t2008-09-19 10:11:04.000000000 +0000\n@@ -52,11 +52,24 @@ static inline int dev_requeue_skb(struct\n \n static inline struct sk_buff *dequeue_skb(struct Qdisc *q)\n {\n-\tstruct sk_buff *skb;\n+\tstruct sk_buff *skb = skb_peek(&q->requeue);\n \n-\tskb = __skb_dequeue(&q->requeue);\n-\tif (!skb)\n+\tif (unlikely(skb)) {\n+\t\tstruct net_device *dev = qdisc_dev(q);\n+\t\tstruct netdev_queue *txq;\n+\n+\t\t/* check the reason of requeuing without tx lock first */\n+\t\ttxq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));\n+\t\tif (!netif_tx_queue_stopped(txq) &&\n+\t\t    !netif_tx_queue_frozen(txq)) {\n+\t\t\t__skb_unlink(skb, &q->requeue);\n+\t\t} else {\n+\t\t\tskb = NULL;\n+\t\t\t__netif_schedule(q);\n+\t\t}\n+\t} else {\n \t\tskb = q->dequeue(q);\n+\t}\n \n \treturn skb;\n }\n",
    "prefixes": [
        "take",
        "2"
    ]
}