Commit def2d0ac authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Jim Harris
Browse files

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: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448928


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 81523d9d

lib/env_dpdk/pci.c

+21 −4
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ 


#include "env_internal.h"



#include <rte_alarm.h>

#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;

		}