Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/788792/?format=api
{ "id": 788792, "url": "http://patchwork.ozlabs.org/api/patches/788792/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/20170714131019.52530-5-alice.michael@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": "<20170714131019.52530-5-alice.michael@intel.com>", "list_archive_url": null, "date": "2017-07-14T13:10:11", "name": "[next,S76-V2,05/13] i40e: invert logic for checking incorrect cpu vs irq affinity", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "3dc4a4ddf076735c5bb5299f487880aa08aab6f5", "submitter": { "id": 71123, "url": "http://patchwork.ozlabs.org/api/people/71123/?format=api", "name": "Michael, Alice", "email": "alice.michael@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/20170714131019.52530-5-alice.michael@intel.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/788792/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/788792/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" ], "Received": [ "from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3x8QRn3k57z9sNv\n\tfor <incoming@patchwork.ozlabs.org>;\n\tSat, 15 Jul 2017 07:14:29 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby fraxinus.osuosl.org (Postfix) with ESMTP id D51F38842C;\n\tFri, 14 Jul 2017 21:14:27 +0000 (UTC)", "from fraxinus.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id cpSyDvGJHYZh; Fri, 14 Jul 2017 21:14:26 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby fraxinus.osuosl.org (Postfix) with ESMTP id 110CE88438;\n\tFri, 14 Jul 2017 21:14:25 +0000 (UTC)", "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\tby ash.osuosl.org (Postfix) with ESMTP id BE7CD1C0683\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 14 Jul 2017 21:14:20 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id BA7CE8A41C\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 14 Jul 2017 21:14:20 +0000 (UTC)", "from hemlock.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id BvuC5R1FQEUd for <intel-wired-lan@lists.osuosl.org>;\n\tFri, 14 Jul 2017 21:14:18 +0000 (UTC)", "from mga05.intel.com (mga05.intel.com [192.55.52.43])\n\tby hemlock.osuosl.org (Postfix) with ESMTPS id B9B1C8A426\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tFri, 14 Jul 2017 21:14:18 +0000 (UTC)", "from orsmga002.jf.intel.com ([10.7.209.21])\n\tby fmsmga105.fm.intel.com with ESMTP; 14 Jul 2017 14:14:18 -0700", "from unknown (HELO localhost.jf.intel.com) ([10.166.16.121])\n\tby orsmga002.jf.intel.com with ESMTP; 14 Jul 2017 14:14:17 -0700" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.40,360,1496127600\"; d=\"scan'208\";a=\"111486589\"", "From": "Alice Michael <alice.michael@intel.com>", "To": "alice.michael@intel.com,\n\tintel-wired-lan@lists.osuosl.org", "Date": "Fri, 14 Jul 2017 09:10:11 -0400", "Message-Id": "<20170714131019.52530-5-alice.michael@intel.com>", "X-Mailer": "git-send-email 2.9.3", "In-Reply-To": "<20170714131019.52530-1-alice.michael@intel.com>", "References": "<20170714131019.52530-1-alice.michael@intel.com>", "Subject": "[Intel-wired-lan] [next PATCH S76-V2 05/13] i40e: invert logic for\n\tchecking incorrect cpu vs irq affinity", "X-BeenThere": "intel-wired-lan@osuosl.org", "X-Mailman-Version": "2.1.18-1", "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>", "MIME-Version": "1.0", "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: Jacob Keller <jacob.e.keller@intel.com>\n\nIn commit 96db776a3682 (\"i40e/vf: fix interrupt affinity bug\")\nwe added some code to force exit of polling incase we did\nnot have the correct CPU. This is important since it was possible for\nthe IRQ affinity to be changed while the CPU is pegged at 100%. This can\nresult in the polling routine being stuck on the wrong CPU until\ntraffic finally stops.\n\nUnfortunately, the implementation, \"if the CPU is correct, exit as\nnormal, otherwise, fall-through to the end-polling exit\" is incredibly\nconfusing to reason about. In this case, the normal flow looks like the\nexception, while the exception actually occurs far away from the if\nstatement and comment.\n\nWe recently discovered and fixed a bug in this code because we were\nincorrectly initializing the affinity mask.\n\nRe-write the code so that the exceptional case is handled at the check,\nrather than having the logic be spread through the regular exit flow.\nThis does end up with minor code duplication, but the resulting code is\nmuch easier to reason about.\n\nThe new logic is identical, but inverted. If we are running on a CPU not\nin our affinity mask, we'll exit polling. However, the code flow is much\neasier to understand.\n\nNote that we don't actually have to check for MSI-X, because in the MSI\ncase we'll only have one q_vector, but its default affinity mask should\nbe correct as it includes all CPUs when it's initialized. Further, we\ncould at some point add code to setup the notifier for the non-MSI-X\ncase and enable this workaround for that case too, if desired, though\nthere isn't much gain since its unlikely to be the common case.\n\nSigned-off-by: Jacob Keller <jacob.e.keller@intel.com>\n---\n drivers/net/ethernet/intel/i40e/i40e_txrx.c | 31 +++++++++++++--------------\n drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 30 +++++++++++++-------------\n 2 files changed, 30 insertions(+), 31 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c\nindex d96970f..b323677 100644\n--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n@@ -2367,7 +2367,6 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)\n \n \t/* If work not completed, return budget and polling will return */\n \tif (!clean_complete) {\n-\t\tconst cpumask_t *aff_mask = &q_vector->affinity_mask;\n \t\tint cpu_id = smp_processor_id();\n \n \t\t/* It is possible that the interrupt affinity has changed but,\n@@ -2377,15 +2376,22 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)\n \t\t * continue to poll, otherwise we must stop polling so the\n \t\t * interrupt can move to the correct cpu.\n \t\t */\n-\t\tif (likely(cpumask_test_cpu(cpu_id, aff_mask) ||\n-\t\t\t !(vsi->back->flags & I40E_FLAG_MSIX_ENABLED))) {\n+\t\tif (!cpumask_test_cpu(cpu_id, &q_vector->affinity_mask)) {\n+\t\t\t/* Tell napi that we are done polling */\n+\t\t\tnapi_complete_done(napi, work_done);\n+\n+\t\t\t/* Force an interrupt */\n+\t\t\ti40e_force_wb(vsi, q_vector);\n+\n+\t\t\t/* Return budget-1 so that polling stops */\n+\t\t\treturn budget - 1;\n+\t\t}\n tx_only:\n-\t\t\tif (arm_wb) {\n-\t\t\t\tq_vector->tx.ring[0].tx_stats.tx_force_wb++;\n-\t\t\t\ti40e_enable_wb_on_itr(vsi, q_vector);\n-\t\t\t}\n-\t\t\treturn budget;\n+\t\tif (arm_wb) {\n+\t\t\tq_vector->tx.ring[0].tx_stats.tx_force_wb++;\n+\t\t\ti40e_enable_wb_on_itr(vsi, q_vector);\n \t\t}\n+\t\treturn budget;\n \t}\n \n \tif (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)\n@@ -2394,14 +2400,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)\n \t/* Work is done so exit the polling mode and re-enable the interrupt */\n \tnapi_complete_done(napi, work_done);\n \n-\t/* If we're prematurely stopping polling to fix the interrupt\n-\t * affinity we want to make sure polling starts back up so we\n-\t * issue a call to i40e_force_wb which triggers a SW interrupt.\n-\t */\n-\tif (!clean_complete)\n-\t\ti40e_force_wb(vsi, q_vector);\n-\telse\n-\t\ti40e_update_enable_itr(vsi, q_vector);\n+\ti40e_update_enable_itr(vsi, q_vector);\n \n \treturn min(work_done, budget - 1);\n }\ndiff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\nindex 4bf7e35..a8dc1d9 100644\n--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\n+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c\n@@ -1575,7 +1575,6 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)\n \n \t/* If work not completed, return budget and polling will return */\n \tif (!clean_complete) {\n-\t\tconst cpumask_t *aff_mask = &q_vector->affinity_mask;\n \t\tint cpu_id = smp_processor_id();\n \n \t\t/* It is possible that the interrupt affinity has changed but,\n@@ -1585,14 +1584,22 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)\n \t\t * continue to poll, otherwise we must stop polling so the\n \t\t * interrupt can move to the correct cpu.\n \t\t */\n-\t\tif (likely(cpumask_test_cpu(cpu_id, aff_mask))) {\n+\t\tif (!cpumask_test_cpu(cpu_id, &q_vector->affinity_mask)) {\n+\t\t\t/* Tell napi that we are done polling */\n+\t\t\tnapi_complete_done(napi, work_done);\n+\n+\t\t\t/* Force an interrupt */\n+\t\t\ti40evf_force_wb(vsi, q_vector);\n+\n+\t\t\t/* Return budget-1 so that polling stops */\n+\t\t\treturn budget - 1;\n+\t\t}\n tx_only:\n-\t\t\tif (arm_wb) {\n-\t\t\t\tq_vector->tx.ring[0].tx_stats.tx_force_wb++;\n-\t\t\t\ti40e_enable_wb_on_itr(vsi, q_vector);\n-\t\t\t}\n-\t\t\treturn budget;\n+\t\tif (arm_wb) {\n+\t\t\tq_vector->tx.ring[0].tx_stats.tx_force_wb++;\n+\t\t\ti40e_enable_wb_on_itr(vsi, q_vector);\n \t\t}\n+\t\treturn budget;\n \t}\n \n \tif (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)\n@@ -1601,14 +1608,7 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)\n \t/* Work is done so exit the polling mode and re-enable the interrupt */\n \tnapi_complete_done(napi, work_done);\n \n-\t/* If we're prematurely stopping polling to fix the interrupt\n-\t * affinity we want to make sure polling starts back up so we\n-\t * issue a call to i40evf_force_wb which triggers a SW interrupt.\n-\t */\n-\tif (!clean_complete)\n-\t\ti40evf_force_wb(vsi, q_vector);\n-\telse\n-\t\ti40e_update_enable_itr(vsi, q_vector);\n+\ti40e_update_enable_itr(vsi, q_vector);\n \n \treturn min(work_done, budget - 1);\n }\n", "prefixes": [ "next", "S76-V2", "05/13" ] }