[dpdk-dev] [PATCH v2 2/5] bus/vdev: add lock on vdev device list

[Jianfeng Tan <jianfeng.tan@intel.com>]

As we could add virtual devices from different threads now, we
add a spin lock to protect the vdev device list.

Suggested-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/bus/vdev/vdev.c | 61 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 7eae319..1d1c642 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -61,6 +61,8 @@ TAILQ_HEAD(vdev_device_list, rte_vdev_device);
 
 static struct vdev_device_list vdev_device_list =
 	TAILQ_HEAD_INITIALIZER(vdev_device_list);
+static rte_spinlock_t vdev_device_list_lock = RTE_SPINLOCK_INITIALIZER;
+
 struct vdev_driver_list vdev_driver_list =
 	TAILQ_HEAD_INITIALIZER(vdev_driver_list);
 
@@ -177,6 +179,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
 	return ret;
 }
 
+/* The caller shall be responsible for thread-safe */
 static struct rte_vdev_device *
 find_vdev(const char *name)
 {
@@ -231,10 +234,6 @@ rte_vdev_init(const char *name, const char *args)
 	if (name == NULL)
 		return -EINVAL;
 
-	dev = find_vdev(name);
-	if (dev)
-		return -EEXIST;
-
 	devargs = alloc_devargs(name, args);
 	if (!devargs)
 		return -ENOMEM;
@@ -249,16 +248,28 @@ rte_vdev_init(const char *name, const char *args)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	dev->device.name = devargs->name;
 
+	rte_spinlock_lock(&vdev_device_list_lock);
+	if (find_vdev(name)) {
+		rte_spinlock_unlock(&vdev_device_list_lock);
+		ret = -EEXIST;
+		goto fail;
+	}
+	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
+	rte_spinlock_unlock(&vdev_device_list_lock);
+
 	ret = vdev_probe_all_drivers(dev);
 	if (ret) {
 		if (ret > 0)
 			VDEV_LOG(ERR, "no driver found for %s\n", name);
+		/* If fails, remove it from vdev list */
+		rte_spinlock_lock(&vdev_device_list_lock);
+		TAILQ_REMOVE(&vdev_device_list, dev, next);
+		rte_spinlock_unlock(&vdev_device_list_lock);
 		goto fail;
 	}
 
 	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
 
-	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 	return 0;
 
 fail:
@@ -294,17 +305,25 @@ rte_vdev_uninit(const char *name)
 	if (name == NULL)
 		return -EINVAL;
 
+	rte_spinlock_lock(&vdev_device_list_lock);
 	dev = find_vdev(name);
-	if (!dev)
+	if (!dev) {
+		rte_spinlock_unlock(&vdev_device_list_lock);
 		return -ENOENT;
+	}
+	TAILQ_REMOVE(&vdev_device_list, dev, next);
+	rte_spinlock_unlock(&vdev_device_list_lock);
 
 	devargs = dev->device.devargs;
 
 	ret = vdev_remove_driver(dev);
-	if (ret)
+	if (ret) {
+		/* If fails, add back to vdev list */
+		rte_spinlock_lock(&vdev_device_list_lock);
+		TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
+		rte_spinlock_unlock(&vdev_device_list_lock);
 		return ret;
-
-	TAILQ_REMOVE(&vdev_device_list, dev, next);
+	}
 
 	TAILQ_REMOVE(&devargs_list, devargs, next);
 
@@ -342,19 +361,25 @@ vdev_scan(void)
 		if (devargs->bus != &rte_vdev_bus)
 			continue;
 
-		dev = find_vdev(devargs->name);
-		if (dev)
-			continue;
-
 		dev = calloc(1, sizeof(*dev));
 		if (!dev)
 			return -1;
 
+		rte_spinlock_lock(&vdev_device_list_lock);
+
+		if (find_vdev(devargs->name)) {
+			rte_spinlock_unlock(&vdev_device_list_lock);
+			free(dev);
+			continue;
+		}
+
 		dev->device.devargs = devargs;
 		dev->device.numa_node = SOCKET_ID_ANY;
 		dev->device.name = devargs->name;
 
 		TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
+
+		rte_spinlock_unlock(&vdev_device_list_lock);
 	}
 
 	return 0;
@@ -368,6 +393,10 @@ vdev_probe(void)
 
 	/* call the init function for each virtual device */
 	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		/* we don't use the vdev lock here, as it's only used in DPDK
+		 * initialization; and we don't want to hold such a lock when
+		 * we call each driver probe.
+		 */
 
 		if (dev->device.driver)
 			continue;
@@ -388,14 +417,18 @@ vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 {
 	struct rte_vdev_device *dev;
 
+	rte_spinlock_lock(&vdev_device_list_lock);
 	TAILQ_FOREACH(dev, &vdev_device_list, next) {
 		if (start && &dev->device == start) {
 			start = NULL;
 			continue;
 		}
-		if (cmp(&dev->device, data) == 0)
+		if (cmp(&dev->device, data) == 0) {
+			rte_spinlock_unlock(&vdev_device_list_lock);
 			return &dev->device;
+		}
 	}
+	rte_spinlock_unlock(&vdev_device_list_lock);
 	return NULL;
 }
 
-- 
2.7.4