From cf0abd0e8374727f18a3566f662cf434cfed6b2c Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Wed, 19 Jun 2019 06:52:03 +0200 Subject: [PATCH] env_dpdk/pci: don't hotremove devices directly on the dpdk intr thread To safely access the global pci device list on an spdk thread, we'll need not to modify this list on any other thread. When device gets hotremoved on a dpdk thread, it will now set a new per-device `removed` flag. Then any subsequently called public pci function will remove it from the list. Change-Id: I0f16237617e0bea75b322ab402407780616424c3 Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458931 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto --- include/spdk/env.h | 6 ++++++ lib/env_dpdk/pci.c | 28 +++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/include/spdk/env.h b/include/spdk/env.h index cb9f42b05..5d6ed8d96 100644 --- a/include/spdk/env.h +++ b/include/spdk/env.h @@ -668,6 +668,12 @@ struct spdk_pci_device { struct spdk_pci_driver *driver; bool attached; bool pending_removal; + /* The device was successfully removed on a DPDK interrupt thread, + * but to prevent data races we couldn't remove it from the global + * device list right away. It'll be removed as soon as possible + * on a regular thread when any public pci function is called. + */ + bool removed; TAILQ_ENTRY(spdk_pci_device) tailq; } internal; }; diff --git a/lib/env_dpdk/pci.c b/lib/env_dpdk/pci.c index 53ac3bd8a..ace21dff4 100644 --- a/lib/env_dpdk/pci.c +++ b/lib/env_dpdk/pci.c @@ -169,6 +169,22 @@ spdk_pci_device_rte_hotremove(const char *device_name, } #endif +static void +cleanup_pci_devices(void) +{ + struct spdk_pci_device *dev, *tmp; + + TAILQ_FOREACH_SAFE(dev, &g_pci_devices, internal.tailq, tmp) { + if (!dev->internal.removed) { + continue; + } + + spdk_vtophys_pci_device_removed(dev->dev_handle); + TAILQ_REMOVE(&g_pci_devices, dev, internal.tailq); + free(dev); + } +} + void spdk_pci_init(void) { @@ -209,6 +225,7 @@ spdk_pci_fini(void) struct spdk_pci_device *dev; char bdf[32]; + cleanup_pci_devices(); TAILQ_FOREACH(dev, &g_pci_devices, internal.tailq) { if (dev->internal.attached) { spdk_pci_addr_fmt(bdf, sizeof(bdf), &dev->addr); @@ -299,10 +316,9 @@ spdk_pci_device_fini(struct rte_pci_device *_dev) return -1; } - spdk_vtophys_pci_device_removed(dev->dev_handle); - TAILQ_REMOVE(&g_pci_devices, dev, internal.tailq); + assert(!dev->internal.removed); + dev->internal.removed = true; pthread_mutex_unlock(&g_pci_mutex); - free(dev); return 0; } @@ -313,6 +329,10 @@ spdk_pci_device_detach(struct spdk_pci_device *dev) assert(dev->internal.attached); dev->internal.attached = false; dev->detach(dev); + + pthread_mutex_lock(&g_pci_mutex); + cleanup_pci_devices(); + pthread_mutex_unlock(&g_pci_mutex); } int @@ -327,6 +347,7 @@ spdk_pci_device_attach(struct spdk_pci_driver *driver, spdk_pci_addr_fmt(bdf, sizeof(bdf), pci_address); pthread_mutex_lock(&g_pci_mutex); + cleanup_pci_devices(); TAILQ_FOREACH(dev, &g_pci_devices, internal.tailq) { if (spdk_pci_addr_compare(&dev->addr, pci_address) == 0) { break; @@ -391,6 +412,7 @@ spdk_pci_enumerate(struct spdk_pci_driver *driver, int rc; pthread_mutex_lock(&g_pci_mutex); + cleanup_pci_devices(); TAILQ_FOREACH(dev, &g_pci_devices, internal.tailq) { if (dev->internal.attached || dev->internal.driver != driver ||