Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/995226/?format=api
{ "id": 995226, "url": "http://patchwork.ozlabs.org/api/patches/995226/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/20181108225532.26078-1-jeffrey.t.kirsher@intel.com/", "project": { "id": 46, "url": "http://patchwork.ozlabs.org/api/projects/46/?format=api", "name": "Intel Wired Ethernet development", "link_name": "intel-wired-lan", "list_id": "intel-wired-lan.osuosl.org", "list_email": "intel-wired-lan@osuosl.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20181108225532.26078-1-jeffrey.t.kirsher@intel.com>", "list_archive_url": null, "date": "2018-11-08T22:55:32", "name": "[net-next,v2,2/2] ethernet/intel: consolidate napi and napi exit", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "262ba87329e986ea7cb5bfbcc9d4784e726728f8", "submitter": { "id": 473, "url": "http://patchwork.ozlabs.org/api/people/473/?format=api", "name": "Kirsher, Jeffrey T", "email": "jeffrey.t.kirsher@intel.com" }, "delegate": { "id": 68, "url": "http://patchwork.ozlabs.org/api/users/68/?format=api", "username": "jtkirshe", "first_name": "Jeff", "last_name": "Kirsher", "email": "jeffrey.t.kirsher@intel.com" }, "mbox": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/20181108225532.26078-1-jeffrey.t.kirsher@intel.com/mbox/", "series": [ { "id": 74900, "url": "http://patchwork.ozlabs.org/api/series/74900/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/list/?series=74900", "date": "2018-11-08T22:55:32", "name": null, "version": 2, "mbox": "http://patchwork.ozlabs.org/series/74900/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/995226/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/995226/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<intel-wired-lan-bounces@osuosl.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Authentication-Results": [ "ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=osuosl.org\n\t(client-ip=140.211.166.136; helo=silver.osuosl.org;\n\tenvelope-from=intel-wired-lan-bounces@osuosl.org;\n\treceiver=<UNKNOWN>)", "ozlabs.org;\n\tdmarc=fail (p=none dis=none) header.from=intel.com" ], "Received": [ "from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 42rdt93Hnqz9s8J\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 9 Nov 2018 09:55:45 +1100 (AEDT)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id A17B72F6D2;\n\tThu, 8 Nov 2018 22:55:43 +0000 (UTC)", "from silver.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id w+FeoA9-u6DO; Thu, 8 Nov 2018 22:55:42 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby silver.osuosl.org (Postfix) with ESMTP id 09C022EE58;\n\tThu, 8 Nov 2018 22:55:42 +0000 (UTC)", "from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n\tby ash.osuosl.org (Postfix) with ESMTP id D8A9B1C1FF7\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 22:55:39 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby whitealder.osuosl.org (Postfix) with ESMTP id D44C8869BF\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 22:55:39 +0000 (UTC)", "from whitealder.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id CcuQLoY9zxc9 for <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 22:55:38 +0000 (UTC)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby whitealder.osuosl.org (Postfix) with ESMTPS id 98F2A86290\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 22:55:38 +0000 (UTC)", "from fmsmga007.fm.intel.com ([10.253.24.52])\n\tby orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t08 Nov 2018 14:55:37 -0800", "from jtkirshe-desk1.jf.intel.com ([134.134.177.96])\n\tby fmsmga007.fm.intel.com with ESMTP; 08 Nov 2018 14:55:37 -0800" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.54,481,1534834800\"; d=\"scan'208\";a=\"84428713\"", "From": "Jeff Kirsher <jeffrey.t.kirsher@intel.com>", "To": "intel-wired-lan@lists.osuosl.org", "Date": "Thu, 8 Nov 2018 14:55:32 -0800", "Message-Id": "<20181108225532.26078-1-jeffrey.t.kirsher@intel.com>", "X-Mailer": "git-send-email 2.19.1", "MIME-Version": "1.0", "Subject": "[Intel-wired-lan] [net-next v2 2/2] ethernet/intel: consolidate\n\tnapi and napi exit", "X-BeenThere": "intel-wired-lan@osuosl.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "Intel Wired Ethernet Linux Kernel Driver Development\n\t<intel-wired-lan.osuosl.org>", "List-Unsubscribe": "<https://lists.osuosl.org/mailman/options/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@osuosl.org?subject=unsubscribe>", "List-Archive": "<http://lists.osuosl.org/pipermail/intel-wired-lan/>", "List-Post": "<mailto:intel-wired-lan@osuosl.org>", "List-Help": "<mailto:intel-wired-lan-request@osuosl.org?subject=help>", "List-Subscribe": "<https://lists.osuosl.org/mailman/listinfo/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@osuosl.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "intel-wired-lan-bounces@osuosl.org", "Sender": "\"Intel-wired-lan\" <intel-wired-lan-bounces@osuosl.org>" }, "content": "From: Jesse Brandeburg <jesse.brandeburg@intel.com>\n\nWhile reviewing code, I noticed that Eric Dumazet recommends that\ndrivers check the return code of napi_complete_done, and use that\nto decide to enable interrupts or not when exiting poll. One of\nthe Intel drivers was already fixed (ixgbe).\n\nUpon looking at the Intel drivers as a whole, we are handling our\npolling and napi exit in a few different ways based on whether we\nhave multiqueue and whether we have tx cleanup included. Several\ndrivers had the bug of exiting napi with return 0, which appears\nto mess up the accounting in the stack.\n\nConsolidate all the napi routines to do best known way of exiting\nand to just mostly look like each other.\n1) check return code of napi_complete_done to control interrupt enable\n2) return the actual amount of work done.\n3) return budget immediately if need napi poll again\n\nTested the changes on e1000e with a high interrupt rate set, and\nit shows about an 8% reduction in the CPU utilization when busy\npolling because we aren't re-enabling interrupts when we're about\nto be polled.\n\nSigned-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>\n---\nv2: fixed conflicts when applied to my next-queue tree, dev-queue\n branch. Most notably the hunk toward the ice driver did not apply\n due to upstream driver changes already applied to my tree\n\n drivers/net/ethernet/intel/e100.c | 10 +++++----\n drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++-----\n drivers/net/ethernet/intel/e1000e/netdev.c | 17 +++++++-------\n drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 ++++-----\n drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++----\n drivers/net/ethernet/intel/iavf/iavf_txrx.c | 9 ++++----\n drivers/net/ethernet/intel/ice/ice_txrx.c | 10 +++++----\n drivers/net/ethernet/intel/igb/igb_main.c | 10 +++++----\n drivers/net/ethernet/intel/igbvf/netdev.c | 9 +++++---\n drivers/net/ethernet/intel/igc/igc_main.c | 10 +++++----\n .../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++--------\n 11 files changed, 73 insertions(+), 54 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c\nindex 7c4b55482f72..5e5c57db0d3f 100644\n--- a/drivers/net/ethernet/intel/e100.c\n+++ b/drivers/net/ethernet/intel/e100.c\n@@ -2225,11 +2225,13 @@ static int e100_poll(struct napi_struct *napi, int budget)\n \te100_rx_clean(nic, &work_done, budget);\n \te100_tx_clean(nic);\n \n-\t/* If budget not fully consumed, exit the polling mode */\n-\tif (work_done < budget) {\n-\t\tnapi_complete_done(napi, work_done);\n+\t/* If budget fully consumed, continue polling */\n+\tif (work_done == budget)\n+\t\treturn budget;\n+\n+\t/* only re-enable interrupt if stack agrees polling is really done */\n+\tif (likely(napi_complete_done(napi, work_done)))\n \t\te100_enable_irq(nic);\n-\t}\n \n \treturn work_done;\n }\ndiff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c\nindex 43b6d3cec3b3..8fe9af0e2ab7 100644\n--- a/drivers/net/ethernet/intel/e1000/e1000_main.c\n+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c\n@@ -3803,14 +3803,15 @@ static int e1000_clean(struct napi_struct *napi, int budget)\n \n \tadapter->clean_rx(adapter, &adapter->rx_ring[0], &work_done, budget);\n \n-\tif (!tx_clean_complete)\n-\t\twork_done = budget;\n+\tif (!tx_clean_complete || work_done == budget)\n+\t\treturn budget;\n \n-\t/* If budget not fully consumed, exit the polling mode */\n-\tif (work_done < budget) {\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done))) {\n \t\tif (likely(adapter->itr_setting & 3))\n \t\t\te1000_set_itr(adapter);\n-\t\tnapi_complete_done(napi, work_done);\n \t\tif (!test_bit(__E1000_DOWN, &adapter->flags))\n \t\t\te1000_irq_enable(adapter);\n \t}\ndiff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c\nindex a387b21312e8..4244983fcd37 100644\n--- a/drivers/net/ethernet/intel/e1000e/netdev.c\n+++ b/drivers/net/ethernet/intel/e1000e/netdev.c\n@@ -2653,9 +2653,9 @@ static int e1000_alloc_queues(struct e1000_adapter *adapter)\n /**\n * e1000e_poll - NAPI Rx polling callback\n * @napi: struct associated with this polling callback\n- * @weight: number of packets driver is allowed to process this poll\n+ * @budget: number of packets driver is allowed to process this poll\n **/\n-static int e1000e_poll(struct napi_struct *napi, int weight)\n+static int e1000e_poll(struct napi_struct *napi, int budget)\n {\n \tstruct e1000_adapter *adapter = container_of(napi, struct e1000_adapter,\n \t\t\t\t\t\t napi);\n@@ -2669,16 +2669,17 @@ static int e1000e_poll(struct napi_struct *napi, int weight)\n \t (adapter->rx_ring->ims_val & adapter->tx_ring->ims_val))\n \t\ttx_cleaned = e1000_clean_tx_irq(adapter->tx_ring);\n \n-\tadapter->clean_rx(adapter->rx_ring, &work_done, weight);\n+\tadapter->clean_rx(adapter->rx_ring, &work_done, budget);\n \n-\tif (!tx_cleaned)\n-\t\twork_done = weight;\n+\tif (!tx_cleaned || work_done == budget)\n+\t\treturn budget;\n \n-\t/* If weight not fully consumed, exit the polling mode */\n-\tif (work_done < weight) {\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done))) {\n \t\tif (adapter->itr_setting & 3)\n \t\t\te1000_set_itr(adapter);\n-\t\tnapi_complete_done(napi, work_done);\n \t\tif (!test_bit(__E1000_DOWN, &adapter->state)) {\n \t\t\tif (adapter->msix_entries)\n \t\t\t\tew32(IMS, adapter->rx_ring->ims_val);\ndiff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c\nindex 5b2a50e5798f..6fd15a734324 100644\n--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c\n+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c\n@@ -1465,11 +1465,11 @@ static int fm10k_poll(struct napi_struct *napi, int budget)\n \tif (!clean_complete)\n \t\treturn budget;\n \n-\t/* all work done, exit the polling mode */\n-\tnapi_complete_done(napi, work_done);\n-\n-\t/* re-enable the q_vector */\n-\tfm10k_qv_enable(q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\tfm10k_qv_enable(q_vector);\n \n \treturn min(work_done, budget - 1);\n }\ndiff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c\nindex c4d44096cdaf..a0b1575468fc 100644\n--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n@@ -2667,10 +2667,11 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)\n \tif (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)\n \t\tq_vector->arm_wb_state = false;\n \n-\t/* Work is done so exit the polling mode and re-enable the interrupt */\n-\tnapi_complete_done(napi, work_done);\n-\n-\ti40e_update_enable_itr(vsi, q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\ti40e_update_enable_itr(vsi, q_vector);\n \n \treturn min(work_done, budget - 1);\n }\ndiff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c\nindex 3b1dc77ae368..9b4d7cec2e18 100644\n--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c\n+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c\n@@ -1761,10 +1761,11 @@ int iavf_napi_poll(struct napi_struct *napi, int budget)\n \tif (vsi->back->flags & IAVF_TXR_FLAGS_WB_ON_ITR)\n \t\tq_vector->arm_wb_state = false;\n \n-\t/* Work is done so exit the polling mode and re-enable the interrupt */\n-\tnapi_complete_done(napi, work_done);\n-\n-\tiavf_update_enable_itr(vsi, q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\tiavf_update_enable_itr(vsi, q_vector);\n \n \treturn min(work_done, budget - 1);\n }\ndiff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c\nindex 4b92863b3500..49fc38094185 100644\n--- a/drivers/net/ethernet/intel/ice/ice_txrx.c\n+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c\n@@ -1103,10 +1103,12 @@ int ice_napi_poll(struct napi_struct *napi, int budget)\n \tif (!clean_complete)\n \t\treturn budget;\n \n-\t/* Work is done so exit the polling mode and re-enable the interrupt */\n-\tnapi_complete_done(napi, work_done);\n-\tif (test_bit(ICE_FLAG_MSIX_ENA, pf->flags))\n-\t\tice_irq_dynamic_ena(&vsi->back->hw, vsi, q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\tif (test_bit(ICE_FLAG_MSIX_ENA, pf->flags))\n+\t\t\tice_irq_dynamic_ena(&vsi->back->hw, vsi, q_vector);\n \n \treturn min(work_done, budget - 1);\n }\ndiff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c\nindex b0f17d9f3cb0..bb4f3f64fbf0 100644\n--- a/drivers/net/ethernet/intel/igb/igb_main.c\n+++ b/drivers/net/ethernet/intel/igb/igb_main.c\n@@ -7752,11 +7752,13 @@ static int igb_poll(struct napi_struct *napi, int budget)\n \tif (!clean_complete)\n \t\treturn budget;\n \n-\t/* If not enough Rx work done, exit the polling mode */\n-\tnapi_complete_done(napi, work_done);\n-\tigb_ring_irq_enable(q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\tigb_ring_irq_enable(q_vector);\n \n-\treturn 0;\n+\treturn min(work_done, budget - 1);\n }\n \n /**\ndiff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c\nindex 820d49eb41ab..4eab83faec62 100644\n--- a/drivers/net/ethernet/intel/igbvf/netdev.c\n+++ b/drivers/net/ethernet/intel/igbvf/netdev.c\n@@ -1186,10 +1186,13 @@ static int igbvf_poll(struct napi_struct *napi, int budget)\n \n \tigbvf_clean_rx_irq(adapter, &work_done, budget);\n \n-\t/* If not enough Rx work done, exit the polling mode */\n-\tif (work_done < budget) {\n-\t\tnapi_complete_done(napi, work_done);\n+\tif (work_done == budget)\n+\t\treturn budget;\n \n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done))) {\n \t\tif (adapter->requested_itr & 3)\n \t\t\tigbvf_set_itr(adapter);\n \ndiff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c\nindex d002055c0623..28ffe98f8921 100644\n--- a/drivers/net/ethernet/intel/igc/igc_main.c\n+++ b/drivers/net/ethernet/intel/igc/igc_main.c\n@@ -2852,11 +2852,13 @@ static int igc_poll(struct napi_struct *napi, int budget)\n \tif (!clean_complete)\n \t\treturn budget;\n \n-\t/* If not enough Rx work done, exit the polling mode */\n-\tnapi_complete_done(napi, work_done);\n-\tigc_ring_irq_enable(q_vector);\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done)))\n+\t\tigc_ring_irq_enable(q_vector);\n \n-\treturn 0;\n+\treturn min(work_done, budget - 1);\n }\n \n /**\ndiff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c\nindex 196b890467b2..2de81f046fb5 100644\n--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c\n+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c\n@@ -1293,16 +1293,20 @@ static int ixgbevf_poll(struct napi_struct *napi, int budget)\n \t/* If all work not completed, return budget and keep polling */\n \tif (!clean_complete)\n \t\treturn budget;\n-\t/* all work done, exit the polling mode */\n-\tnapi_complete_done(napi, work_done);\n-\tif (adapter->rx_itr_setting == 1)\n-\t\tixgbevf_set_itr(q_vector);\n-\tif (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&\n-\t !test_bit(__IXGBEVF_REMOVING, &adapter->state))\n-\t\tixgbevf_irq_enable_queues(adapter,\n-\t\t\t\t\t BIT(q_vector->v_idx));\n \n-\treturn 0;\n+\t/* Exit the polling mode, but don't re-enable interrupts if stack might\n+\t * poll us due to busy-polling\n+\t */\n+\tif (likely(napi_complete_done(napi, work_done))) {\n+\t\tif (adapter->rx_itr_setting == 1)\n+\t\t\tixgbevf_set_itr(q_vector);\n+\t\tif (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&\n+\t\t !test_bit(__IXGBEVF_REMOVING, &adapter->state))\n+\t\t\tixgbevf_irq_enable_queues(adapter,\n+\t\t\t\t\t\t BIT(q_vector->v_idx));\n+\t}\n+\n+\treturn min(work_done, budget - 1);\n }\n \n /**\n", "prefixes": [ "net-next", "v2", "2/2" ] }