From def2d0ac3ec39fe97e8920d1cf8b7adb0ed1eaa2 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Sun, 24 Mar 2019 13:09:42 +0100 Subject: [PATCH] env/dpdk: detach pci devices from EAL interrupt thread While detaching the device, DPDK may try to unregister a VFIO interrupt callback which is currently "in use". The unregister call may fail, but the error doesn't get propagated to upper DPDK layers. Practically, detaching the device may stop in the middle but still return 0 to SPDK. This effectively breaks hotremove as the device would be neither usable or removable. We work around it in SPDK by internally scheduling the DPDK device detach on the DPDK interrupt thread. This prevents any other interrupt callback to be "in use" while the device is detached. Since device detach in SPDK can be asynchronous now, we add a few checks to prevent re-attaching devices that are still being detached. Change-Id: Ibb56a8017e34418db0304fe32774811427b056aa Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448928 Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris --- lib/env_dpdk/pci.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/env_dpdk/pci.c b/lib/env_dpdk/pci.c index 124735ae2..49cbd1d79 100644 --- a/lib/env_dpdk/pci.c +++ b/lib/env_dpdk/pci.c @@ -33,6 +33,7 @@ #include "env_internal.h" +#include #include "spdk/env.h" #define SYSFS_PCI_DRIVERS "/sys/bus/pci/drivers" @@ -97,9 +98,9 @@ spdk_cfg_write_rte(struct spdk_pci_device *dev, void *value, uint32_t len, uint3 } static void -spdk_detach_rte(struct spdk_pci_device *dev) +spdk_detach_rte_cb(void *_dev) { - struct rte_pci_device *rte_dev = dev->dev_handle; + struct rte_pci_device *rte_dev = _dev; #if RTE_VERSION >= RTE_VERSION_NUM(18, 11, 0, 0) char bdf[32]; @@ -114,6 +115,20 @@ spdk_detach_rte(struct spdk_pci_device *dev) #endif } +static void +spdk_detach_rte(struct spdk_pci_device *dev) +{ + /* The device was already marked as available and could be attached + * again while we go asynchronous, so we explicitly forbid that. + */ + dev->internal.pending_removal = true; + if (spdk_process_is_primary()) { + rte_eal_alarm_set(10, spdk_detach_rte_cb, dev->dev_handle); + } else { + spdk_detach_rte_cb(dev->dev_handle); + } +} + void spdk_pci_driver_register(struct spdk_pci_driver *driver) { @@ -317,7 +332,7 @@ spdk_pci_device_attach(struct spdk_pci_driver *driver, } if (dev != NULL && dev->internal.driver == driver) { - if (dev->internal.attached) { + if (dev->internal.attached || dev->internal.pending_removal) { pthread_mutex_unlock(&g_pci_mutex); return -1; } @@ -377,7 +392,9 @@ spdk_pci_enumerate(struct spdk_pci_driver *driver, pthread_mutex_lock(&g_pci_mutex); TAILQ_FOREACH(dev, &g_pci_devices, internal.tailq) { - if (dev->internal.attached || dev->internal.driver != driver) { + if (dev->internal.attached || + dev->internal.driver != driver || + dev->internal.pending_removal) { continue; }