Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/806317/?format=api
{ "id": 806317, "url": "http://patchwork.ozlabs.org/api/patches/806317/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/20170828001603.75876-14-jeffrey.t.kirsher@intel.com/", "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": "<20170828001603.75876-14-jeffrey.t.kirsher@intel.com>", "list_archive_url": null, "date": "2017-08-28T00:16:01", "name": "[net-next,13/15] i40e: invert logic for checking incorrect cpu vs irq affinity", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "3dc4a4ddf076735c5bb5299f487880aa08aab6f5", "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": 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/20170828001603.75876-14-jeffrey.t.kirsher@intel.com/mbox/", "series": [ { "id": 52, "url": "http://patchwork.ozlabs.org/api/series/52/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/list/?series=52", "date": "2017-08-28T00:15:49", "name": "40GbE Intel Wired LAN Driver Updates 2017-08-27", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/52/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/806317/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/806317/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xgXPn3Kq1z9s82\n\tfor <patchwork-incoming@ozlabs.org>;\n\tMon, 28 Aug 2017 10:16:45 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751794AbdH1AQm (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tSun, 27 Aug 2017 20:16:42 -0400", "from mga14.intel.com ([192.55.52.115]:36375 \"EHLO mga14.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751700AbdH1AQV (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tSun, 27 Aug 2017 20:16:21 -0400", "from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t27 Aug 2017 17:16:21 -0700", "from davidpwo-mobl1.amr.corp.intel.com (HELO\n\tjtkirshe-DESK.amr.corp.intel.com.com) ([10.254.22.134])\n\tby fmsmga004.fm.intel.com with ESMTP; 27 Aug 2017 17:16:20 -0700" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.41,439,1498546800\"; d=\"scan'208\";a=\"304981099\"", "From": "Jeff Kirsher <jeffrey.t.kirsher@intel.com>", "To": "davem@davemloft.net", "Cc": "Jacob Keller <jacob.e.keller@intel.com>, netdev@vger.kernel.org,\n\tnhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com,\n\tJeff Kirsher <jeffrey.t.kirsher@intel.com>", "Subject": "[net-next 13/15] i40e: invert logic for checking incorrect cpu vs\n\tirq affinity", "Date": "Sun, 27 Aug 2017 17:16:01 -0700", "Message-Id": "<20170828001603.75876-14-jeffrey.t.kirsher@intel.com>", "X-Mailer": "git-send-email 2.14.1", "In-Reply-To": "<20170828001603.75876-1-jeffrey.t.kirsher@intel.com>", "References": "<20170828001603.75876-1-jeffrey.t.kirsher@intel.com>", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.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 in case 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>\nTested-by: Andrew Bowers <andrewx.bowers@intel.com>\nSigned-off-by: Jeff Kirsher <jeffrey.t.kirsher@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 5c1edcce9459..3999afea518b 100644\n--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c\n@@ -2369,7 +2369,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@@ -2379,15 +2378,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@@ -2396,14 +2402,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 d91676ccf125..f15e341ada9e 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": [ "net-next", "13/15" ] }