Commit cf0abd0e authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Ben Walker
Browse files

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
parent 49c12890

include/spdk/env.h

+6 −0
Original line number Diff line number Diff line
@@ -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;

};

lib/env_dpdk/pci.c

+25 −3
Original line number Diff line number Diff line
@@ -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 ||