Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/786377/?format=api
{ "id": 786377, "url": "http://patchwork.ozlabs.org/api/patches/786377/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/20170710202319.22110-10-jacob.e.keller@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": "<20170710202319.22110-10-jacob.e.keller@intel.com>", "list_archive_url": null, "date": "2017-07-10T20:23:13", "name": "[v3,10/16] fm10k: prevent race condition of __FM10K_SERVICE_SCHED", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "4b86eaf0e1e381fc6d406ed1a2c1ab8e8db61a38", "submitter": { "id": 9784, "url": "http://patchwork.ozlabs.org/api/people/9784/?format=api", "name": "Jacob Keller", "email": "jacob.e.keller@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/20170710202319.22110-10-jacob.e.keller@intel.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/786377/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/786377/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 hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\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 3x5xW272hBz9s06\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 11 Jul 2017 06:23:42 +1000 (AEST)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 531C88873D;\n\tMon, 10 Jul 2017 20:23:41 +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 jqVBiEfQBKgU; Mon, 10 Jul 2017 20:23:38 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 32FCD884A6;\n\tMon, 10 Jul 2017 20:23:36 +0000 (UTC)", "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\tby ash.osuosl.org (Postfix) with ESMTP id A242F1CEAC7\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tMon, 10 Jul 2017 20:23:28 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 83AC4887E5\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tMon, 10 Jul 2017 20:23:28 +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 F0lqv3YMKr6d for <intel-wired-lan@lists.osuosl.org>;\n\tMon, 10 Jul 2017 20:23:22 +0000 (UTC)", "from mga14.intel.com (mga14.intel.com [192.55.52.115])\n\tby hemlock.osuosl.org (Postfix) with ESMTPS id 98B32875DF\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tMon, 10 Jul 2017 20:23:22 +0000 (UTC)", "from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t10 Jul 2017 13:23:22 -0700", "from jekeller-desk.amr.corp.intel.com (HELO\n\tjekeller-desk.jekeller.internal) ([134.134.177.230])\n\tby fmsmga004.fm.intel.com with ESMTP; 10 Jul 2017 13:23:21 -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,342,1496127600\"; d=\"scan'208\";a=\"285198867\"", "From": "Jacob Keller <jacob.e.keller@intel.com>", "To": "jtkirhse@osuosl.org, Intel Wired LAN <intel-wired-lan@lists.osuosl.org>", "Date": "Mon, 10 Jul 2017 13:23:13 -0700", "Message-Id": "<20170710202319.22110-10-jacob.e.keller@intel.com>", "X-Mailer": "git-send-email 2.13.0.615.gb09ed6e59a40", "In-Reply-To": "<20170710202319.22110-1-jacob.e.keller@intel.com>", "References": "<20170710202319.22110-1-jacob.e.keller@intel.com>", "Cc": "jekeller@osuosl.org", "Subject": "[Intel-wired-lan] [PATCH v3 10/16] fm10k: prevent race condition of\n\t__FM10K_SERVICE_SCHED", "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": "Although very unlikely, it is possible that cancel_work_sync() may stop\nthe service_task before it actually started. In this case, the\n__FM10K_SERVICE_SCHED bit will never be cleared. This results in the\nservice task being unable to reschedule in the future. Add a helper\nfunction which sets the service disable bit, waits for the service task\nto stop and clears the schedule bit, thus avoiding the race condition.\nWe know the schedule bit is safe to clear because the cancel_work_sync()\nguarantees the service task is not running.\n\nAdd a helper function also to restart the service task, for symmetry.\nThis is not strictly needed but helps the mental model of how to stop\nand start the service task.\n\nThis race could only happen in fm10k_suspend/fm10k_resume as this is the\nonly place where the service task is actually restarted. Thus,\nsuspend/resume testing would be ideal. However, note that the chance of\nthis happening is very slim as the service event is scheduled for\nimmediate execution, and you would have to trigger a suspend at almost\nthe exact same time as the service task was scheduled.\n\nSigned-off-by: Jacob Keller <jacob.e.keller@intel.com>\n---\n drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 32 ++++++++++++++++++++++------\n 1 file changed, 25 insertions(+), 7 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c\nindex 6a7b4c5429ae..cfbe25e90006 100644\n--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c\n+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c\n@@ -118,6 +118,27 @@ static void fm10k_service_event_complete(struct fm10k_intfc *interface)\n \t\tfm10k_service_event_schedule(interface);\n }\n \n+static void fm10k_stop_service_event(struct fm10k_intfc *interface)\n+{\n+\tset_bit(__FM10K_SERVICE_DISABLE, interface->state);\n+\tcancel_work_sync(&interface->service_task);\n+\n+\t/* It's possible that cancel_work_sync stopped the service task from\n+\t * running before it could actually start. In this case the\n+\t * __FM10K_SERVICE_SCHED bit will never be cleared. Since we know that\n+\t * the service task cannot be running at this point, we need to clear\n+\t * the scheduled bit, as otherwise the service task may never be\n+\t * restarted.\n+\t */\n+\tclear_bit(__FM10K_SERVICE_SCHED, interface->state);\n+}\n+\n+static void fm10k_start_service_event(struct fm10k_intfc *interface)\n+{\n+\tclear_bit(__FM10K_SERVICE_DISABLE, interface->state);\n+\tfm10k_service_event_schedule(interface);\n+}\n+\n /**\n * fm10k_service_timer - Timer Call-back\n * @data: pointer to interface cast into an unsigned long\n@@ -2116,8 +2137,7 @@ static void fm10k_remove(struct pci_dev *pdev)\n \n \tdel_timer_sync(&interface->service_timer);\n \n-\tset_bit(__FM10K_SERVICE_DISABLE, interface->state);\n-\tcancel_work_sync(&interface->service_task);\n+\tfm10k_stop_service_event(interface);\n \n \t/* free netdev, this may bounce the interrupts due to setup_tc */\n \tif (netdev->reg_state == NETREG_REGISTERED)\n@@ -2155,8 +2175,7 @@ static void fm10k_prepare_suspend(struct fm10k_intfc *interface)\n \t * stopped. We stop the watchdog task until after we resume software\n \t * activity.\n \t */\n-\tset_bit(__FM10K_SERVICE_DISABLE, interface->state);\n-\tcancel_work_sync(&interface->service_task);\n+\tfm10k_stop_service_event(interface);\n \n \tfm10k_prepare_for_reset(interface);\n }\n@@ -2183,9 +2202,8 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)\n \tinterface->link_down_event = jiffies + (HZ);\n \tset_bit(__FM10K_LINK_DOWN, interface->state);\n \n-\t/* clear the service task disable bit to allow service task to start */\n-\tclear_bit(__FM10K_SERVICE_DISABLE, interface->state);\n-\tfm10k_service_event_schedule(interface);\n+\t/* restart the service task */\n+\tfm10k_start_service_event(interface);\n \n \treturn err;\n }\n", "prefixes": [ "v3", "10/16" ] }