Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/994656/?format=api
{ "id": 994656, "url": "http://patchwork.ozlabs.org/api/patches/994656/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/20181108054018.32293-2-jesse.brandeburg@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": "<20181108054018.32293-2-jesse.brandeburg@intel.com>", "list_archive_url": null, "date": "2018-11-08T05:40:18", "name": "[net-next,v1,2/2] ethernet/intel: consolidate napi and napi exit", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "336dec0ece47bc5895e952b97e2e3093aae4b743", "submitter": { "id": 189, "url": "http://patchwork.ozlabs.org/api/people/189/?format=api", "name": "Jesse Brandeburg", "email": "jesse.brandeburg@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/20181108054018.32293-2-jesse.brandeburg@intel.com/mbox/", "series": [ { "id": 74661, "url": "http://patchwork.ozlabs.org/api/series/74661/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/list/?series=74661", "date": "2018-11-08T05:40:17", "name": "[net-next,v1,1/2] docs-networking: fix typo in define", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/74661/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/994656/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/994656/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 42rBvb0nTtz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 8 Nov 2018 16:40:26 +1100 (AEDT)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id 1351522022;\n\tThu, 8 Nov 2018 05:40:25 +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 BiBH5oDAfHVq; Thu, 8 Nov 2018 05:40:23 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby silver.osuosl.org (Postfix) with ESMTP id 78A9322098;\n\tThu, 8 Nov 2018 05:40:23 +0000 (UTC)", "from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\tby ash.osuosl.org (Postfix) with ESMTP id EDB2E1BF5A3\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 05:40:20 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id EAB2122022\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 05:40:20 +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 NAQvt3f4uYud for <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 05:40:20 +0000 (UTC)", "from mga03.intel.com (mga03.intel.com [134.134.136.65])\n\tby silver.osuosl.org (Postfix) with ESMTPS id E7A5822098\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tThu, 8 Nov 2018 05:40:19 +0000 (UTC)", "from fmsmga006.fm.intel.com ([10.253.24.20])\n\tby orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t07 Nov 2018 21:40:19 -0800", "from jbrandeb-desk.jf.intel.com ([10.166.244.194])\n\tby fmsmga006.fm.intel.com with ESMTP; 07 Nov 2018 21:40:18 -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,478,1534834800\"; d=\"scan'208\";a=\"279311020\"", "From": "Jesse Brandeburg <jesse.brandeburg@intel.com>", "To": "intel-wired-lan@lists.osuosl.org", "Date": "Wed, 7 Nov 2018 21:40:18 -0800", "Message-Id": "<20181108054018.32293-2-jesse.brandeburg@intel.com>", "X-Mailer": "git-send-email 2.19.1", "In-Reply-To": "<20181108054018.32293-1-jesse.brandeburg@intel.com>", "References": "<20181108054018.32293-1-jesse.brandeburg@intel.com>", "MIME-Version": "1.0", "Subject": "[Intel-wired-lan] [net-next v1 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": "While 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---\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 | 13 ++++++-----\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, 75 insertions(+), 55 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 16a73bd9f4cb..698639f50eac 100644\n--- a/drivers/net/ethernet/intel/e1000e/netdev.c\n+++ b/drivers/net/ethernet/intel/e1000e/netdev.c\n@@ -2651,9 +2651,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@@ -2667,16 +2667,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 aef3c89ee79c..4c796ac80daa 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 fb9bfad96daf..cf965e5eb7ec 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 5dae968d853e..ffc3e3bbb3cd 100644\n--- a/drivers/net/ethernet/intel/ice/ice_txrx.c\n+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c\n@@ -1103,11 +1103,14 @@ 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-\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 (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 }\n \n /* helper function for building cmd/type/offset */\ndiff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c\nindex 5df88ad8ac81..f5592910978d 100644\n--- a/drivers/net/ethernet/intel/igb/igb_main.c\n+++ b/drivers/net/ethernet/intel/igb/igb_main.c\n@@ -7753,11 +7753,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 9d85707e8a81..3ca8d859caf3 100644\n--- a/drivers/net/ethernet/intel/igc/igc_main.c\n+++ b/drivers/net/ethernet/intel/igc/igc_main.c\n@@ -2866,11 +2866,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 5e47ede7e832..30a3c800a6ea 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", "v1", "2/2" ] }