From patchwork Thu Nov 15 09:18:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guo, Jia" X-Patchwork-Id: 48109 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0B6C94F91; Thu, 15 Nov 2018 10:14:57 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id DFF6E4CBB for ; Thu, 15 Nov 2018 10:14:52 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Nov 2018 01:14:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,236,1539673200"; d="scan'208";a="108266243" Received: from jeffguo-s2600wt2.sh.intel.com (HELO localhost.localdomain) ([10.67.110.10]) by fmsmga001.fm.intel.com with ESMTP; 15 Nov 2018 01:14:50 -0800 From: Jeff Guo To: konstantin.ananyev@intel.com, anatoly.burakov@intel.com, thomas@monjalon.net, bernard.iremonger@intel.com, jingjing.wu@intel.com, wenzhuo.lu@intel.com Cc: ferruh.yigit@intel.com, dev@dpdk.org, jia.guo@intel.com, helin.zhang@intel.com, matan@mellanox.com, shaopeng.he@intel.com Date: Thu, 15 Nov 2018 17:18:22 +0800 Message-Id: <1542273504-127387-3-git-send-email-jia.guo@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1542273504-127387-1-git-send-email-jia.guo@intel.com> References: <1541484436-91320-1-git-send-email-jia.guo@intel.com> <1542273504-127387-1-git-send-email-jia.guo@intel.com> Subject: [dpdk-dev] [PATCH V2 1/3] eal: fix lock issue for hot-unplug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" When device be hot-unplugged, the hot-unplug handler will be invoked by uio remove event and the device will be detached, then kernel will sent another pci remove event. So if there is any unlock miss, it will cause a dead lock issue. This patch will add this missing unlock for hot-unplug handler. Fixes: 0fc54536b14a ("eal: add failure handling for hot-unplug") Signed-off-by: Jeff Guo --- v2->v1: refine commit log --- lib/librte_eal/linuxapp/eal/eal_dev.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_dev.c b/lib/librte_eal/linuxapp/eal/eal_dev.c index d589c69..2830c86 100644 --- a/lib/librte_eal/linuxapp/eal/eal_dev.c +++ b/lib/librte_eal/linuxapp/eal/eal_dev.c @@ -258,7 +258,7 @@ dev_uev_handler(__rte_unused void *param) if (bus == NULL) { RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname); - return; + goto failure_handle_err; } dev = bus->find_device(NULL, cmp_dev_name, @@ -266,19 +266,23 @@ dev_uev_handler(__rte_unused void *param) if (dev == NULL) { RTE_LOG(ERR, EAL, "Cannot find device (%s) on " "bus (%s)\n", uevent.devname, busname); - return; + goto failure_handle_err; } ret = bus->hot_unplug_handler(dev); - rte_spinlock_unlock(&failure_handle_lock); if (ret) { RTE_LOG(ERR, EAL, "Can not handle hot-unplug " "for device (%s)\n", dev->name); - return; } + rte_spinlock_unlock(&failure_handle_lock); } rte_dev_event_callback_process(uevent.devname, uevent.type); } + + return; + +failure_handle_err: + rte_spinlock_unlock(&failure_handle_lock); } int __rte_experimental From patchwork Thu Nov 15 09:18:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guo, Jia" X-Patchwork-Id: 48110 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C9CDB4F9C; Thu, 15 Nov 2018 10:14:58 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 4C51D4CE4 for ; Thu, 15 Nov 2018 10:14:55 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Nov 2018 01:14:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,236,1539673200"; d="scan'208";a="108266248" Received: from jeffguo-s2600wt2.sh.intel.com (HELO localhost.localdomain) ([10.67.110.10]) by fmsmga001.fm.intel.com with ESMTP; 15 Nov 2018 01:14:52 -0800 From: Jeff Guo To: konstantin.ananyev@intel.com, anatoly.burakov@intel.com, thomas@monjalon.net, bernard.iremonger@intel.com, jingjing.wu@intel.com, wenzhuo.lu@intel.com Cc: ferruh.yigit@intel.com, dev@dpdk.org, jia.guo@intel.com, helin.zhang@intel.com, matan@mellanox.com, shaopeng.he@intel.com Date: Thu, 15 Nov 2018 17:18:23 +0800 Message-Id: <1542273504-127387-4-git-send-email-jia.guo@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1542273504-127387-1-git-send-email-jia.guo@intel.com> References: <1541484436-91320-1-git-send-email-jia.guo@intel.com> <1542273504-127387-1-git-send-email-jia.guo@intel.com> Subject: [dpdk-dev] [PATCH V2 2/3] vfio: fix to add handler lock for hot-unplug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" When the sigbus handler be enabled for hot-unplug, whatever hot-unplug sigbus or origin sigbus occur, the sigbus handler will be invoked and it will access the bus and device. While in the control path, the vfio req handler also will process the bus and device, so a protection of the resources in vfio req handler should be need. This patch add a lock in vfio req handler when process bus and device resource, to avoid the synchronization issue when device be hot-unplugged. Fixes: c115fd000c32 ("vfio: handle hotplug request notifier") Signed-off-by: Jeff Guo --- v2->v1: refine commit log --- drivers/bus/pci/linux/pci_vfio.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c index 305cc06..d2c8410 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "eal_filesystem.h" @@ -35,6 +36,14 @@ * This file is only compiled if CONFIG_RTE_EAL_VFIO is set to "y". */ +/* + * spinlock for device hot-unplug failure handling. If it try to access bus or + * device, such as handle sigbus on bus or handle memory failure for device + * just need to use this lock. It could protect the bus and the device to avoid + * race condition. + */ +static rte_spinlock_t failure_handle_lock = RTE_SPINLOCK_INITIALIZER; + #ifdef VFIO_PRESENT #ifndef PAGE_SIZE @@ -289,11 +298,12 @@ pci_vfio_req_handler(void *param) int ret; struct rte_device *device = (struct rte_device *)param; + rte_spinlock_lock(&failure_handle_lock); bus = rte_bus_find_by_device(device); if (bus == NULL) { RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", device->name); - return; + goto handle_end; } /* @@ -306,6 +316,8 @@ pci_vfio_req_handler(void *param) RTE_LOG(ERR, EAL, "Can not handle hot-unplug for device (%s)\n", device->name); +handle_end: + rte_spinlock_unlock(&failure_handle_lock); } /* enable notifier (only enable req now) */ From patchwork Thu Nov 15 09:18:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Guo, Jia" X-Patchwork-Id: 48111 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BDEFB5587; Thu, 15 Nov 2018 10:15:00 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 566B64F9A for ; Thu, 15 Nov 2018 10:14:57 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Nov 2018 01:14:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,236,1539673200"; d="scan'208";a="108266256" Received: from jeffguo-s2600wt2.sh.intel.com (HELO localhost.localdomain) ([10.67.110.10]) by fmsmga001.fm.intel.com with ESMTP; 15 Nov 2018 01:14:54 -0800 From: Jeff Guo To: konstantin.ananyev@intel.com, anatoly.burakov@intel.com, thomas@monjalon.net, bernard.iremonger@intel.com, jingjing.wu@intel.com, wenzhuo.lu@intel.com Cc: ferruh.yigit@intel.com, dev@dpdk.org, jia.guo@intel.com, helin.zhang@intel.com, matan@mellanox.com, shaopeng.he@intel.com Date: Thu, 15 Nov 2018 17:18:24 +0800 Message-Id: <1542273504-127387-5-git-send-email-jia.guo@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1542273504-127387-1-git-send-email-jia.guo@intel.com> References: <1541484436-91320-1-git-send-email-jia.guo@intel.com> <1542273504-127387-1-git-send-email-jia.guo@intel.com> Subject: [dpdk-dev] [PATCH V2 3/3] app/testpmd: fix callback issue for hot-unplug X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Because the user's callback is invoked in eal interrupt callback, the interrupt callback need to be finished before it can be unregistered when detaching device. So finish callback soon and use a deferred removal to detach device is need. It is a workaround, once the device detaching be moved into the eal in the future, the deferred removal could be deleted. This patch aim to add this workaround and refine the function name and the description to be more explicit and comment the limitation. Fixes: 2049c5113fe8 ("app/testpmd: use hotplug failure handler") Signed-off-by: Jeff Guo --- v2->v1: rename the function, refine the doc, and show the limitation. --- app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 9c0edca..4c75587 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -506,7 +506,7 @@ static void check_all_ports_link_status(uint32_t port_mask); static int eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param, void *ret_param); -static void eth_dev_event_callback(const char *device_name, +static void dev_event_callback(const char *device_name, enum rte_dev_event_type type, void *param); @@ -2434,7 +2434,7 @@ pmd_test_exit(void) } ret = rte_dev_event_callback_unregister(NULL, - eth_dev_event_callback, NULL); + dev_event_callback, NULL); if (ret < 0) { RTE_LOG(ERR, EAL, "fail to unregister device event callback.\n"); @@ -2516,8 +2516,14 @@ check_all_ports_link_status(uint32_t port_mask) } } +/* + * This callback is for remove a port for a device. It has limitation because + * it is not for multiple port removal for a device. + * TODO: the device detach invoke will plan to be removed from user side to + * eal. And convert all PMDs to free port resources on ether device closing. + */ static void -rmv_event_callback(void *arg) +rmv_port_callback(void *arg) { int need_to_start = 0; int org_no_link_check = no_link_check; @@ -2565,7 +2571,7 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param, if (port_id_is_invalid(port_id, DISABLED_WARN)) break; if (rte_eal_alarm_set(100000, - rmv_event_callback, (void *)(intptr_t)port_id)) + rmv_port_callback, (void *)(intptr_t)port_id)) fprintf(stderr, "Could not set up deferred device removal\n"); break; default: @@ -2598,7 +2604,7 @@ register_eth_event_callback(void) /* This function is used by the interrupt thread */ static void -eth_dev_event_callback(const char *device_name, enum rte_dev_event_type type, +dev_event_callback(const char *device_name, enum rte_dev_event_type type, __rte_unused void *arg) { uint16_t port_id; @@ -2612,7 +2618,7 @@ eth_dev_event_callback(const char *device_name, enum rte_dev_event_type type, switch (type) { case RTE_DEV_EVENT_REMOVE: - RTE_LOG(ERR, EAL, "The device: %s has been removed!\n", + RTE_LOG(DEBUG, EAL, "The device: %s has been removed!\n", device_name); ret = rte_eth_dev_get_port_by_name(device_name, &port_id); if (ret) { @@ -2620,7 +2626,19 @@ eth_dev_event_callback(const char *device_name, enum rte_dev_event_type type, device_name); return; } - rmv_event_callback((void *)(intptr_t)port_id); + /* + * Because the user's callback is invoked in eal interrupt + * callback, the interrupt callback need to be finished before + * it can be unregistered when detaching device. So finish + * callback soon and use a deferred removal to detach device + * is need. It is a workaround, once the device detaching be + * moved into the eal in the future, the deferred removal could + * be deleted. + */ + if (rte_eal_alarm_set(100000, + rmv_port_callback, (void *)(intptr_t)port_id)) + RTE_LOG(ERR, EAL, + "Could not set up deferred device removal\n"); break; case RTE_DEV_EVENT_ADD: RTE_LOG(ERR, EAL, "The device: %s has been added!\n", @@ -3167,7 +3185,7 @@ main(int argc, char** argv) } ret = rte_dev_event_callback_register(NULL, - eth_dev_event_callback, NULL); + dev_event_callback, NULL); if (ret) { RTE_LOG(ERR, EAL, "fail to register device event callback\n");