From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <jianfeng.tan@intel.com>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 31A63100C
 for <dev@dpdk.org>; Tue, 24 Apr 2018 07:49:24 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga008.jf.intel.com ([10.7.209.65])
 by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 23 Apr 2018 22:49:20 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.49,321,1520924400"; d="scan'208";a="35925670"
Received: from dpdk06.sh.intel.com ([10.67.110.196])
 by orsmga008.jf.intel.com with ESMTP; 23 Apr 2018 22:49:20 -0700
From: Jianfeng Tan <jianfeng.tan@intel.com>
To: dev@dpdk.org
Cc: thomas@monjalon.net,
	Jianfeng Tan <jianfeng.tan@intel.com>
Date: Tue, 24 Apr 2018 05:51:21 +0000
Message-Id: <1524549084-139088-3-git-send-email-jianfeng.tan@intel.com>
X-Mailer: git-send-email 2.7.4
In-Reply-To: <1524549084-139088-1-git-send-email-jianfeng.tan@intel.com>
References: <1520177405-59091-1-git-send-email-jianfeng.tan@intel.com>
 <1524549084-139088-1-git-send-email-jianfeng.tan@intel.com>
Subject: [dpdk-dev] [PATCH v5 2/5] bus/vdev: add lock on vdev device list
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 24 Apr 2018 05:49:25 -0000

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>
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/bus/vdev/vdev.c | 95 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 26 deletions(-)

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index f8dd1f5..70964f5 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -33,6 +33,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);
 
@@ -149,6 +151,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)
 {
@@ -193,8 +196,8 @@ alloc_devargs(const char *name, const char *args)
 	return devargs;
 }
 
-int
-rte_vdev_init(const char *name, const char *args)
+static int
+insert_vdev(const char *name, const char *args, struct rte_vdev_device **p_dev)
 {
 	struct rte_vdev_device *dev;
 	struct rte_devargs *devargs;
@@ -203,10 +206,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;
@@ -221,18 +220,18 @@ rte_vdev_init(const char *name, const char *args)
 	dev->device.numa_node = SOCKET_ID_ANY;
 	dev->device.name = devargs->name;
 
-	ret = vdev_probe_all_drivers(dev);
-	if (ret) {
-		if (ret > 0)
-			VDEV_LOG(ERR, "no driver found for %s\n", name);
+	if (find_vdev(name)) {
+		ret = -EEXIST;
 		goto fail;
 	}
 
+	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
 	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
 
-	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
-	return 0;
+	if (p_dev)
+		*p_dev = dev;
 
+	return 0;
 fail:
 	free(devargs->args);
 	free(devargs);
@@ -240,6 +239,33 @@ rte_vdev_init(const char *name, const char *args)
 	return ret;
 }
 
+int
+rte_vdev_init(const char *name, const char *args)
+{
+	struct rte_vdev_device *dev;
+	struct rte_devargs *devargs;
+	int ret;
+
+	rte_spinlock_lock(&vdev_device_list_lock);
+	ret = insert_vdev(name, args, &dev);
+	if (ret == 0) {
+		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 */
+			devargs = dev->device.devargs;
+			TAILQ_REMOVE(&vdev_device_list, dev, next);
+			TAILQ_REMOVE(&devargs_list, devargs, next);
+			free(devargs->args);
+			free(devargs);
+			free(dev);
+		}
+	}
+	rte_spinlock_unlock(&vdev_device_list_lock);
+	return ret;
+}
+
 static int
 vdev_remove_driver(struct rte_vdev_device *dev)
 {
@@ -266,24 +292,28 @@ rte_vdev_uninit(const char *name)
 	if (name == NULL)
 		return -EINVAL;
 
-	dev = find_vdev(name);
-	if (!dev)
-		return -ENOENT;
+	rte_spinlock_lock(&vdev_device_list_lock);
 
-	devargs = dev->device.devargs;
+	dev = find_vdev(name);
+	if (!dev) {
+		ret = -ENOENT;
+		goto unlock;
+	}
 
 	ret = vdev_remove_driver(dev);
 	if (ret)
-		return ret;
+		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-
+	devargs = dev->device.devargs;
 	TAILQ_REMOVE(&devargs_list, devargs, next);
-
 	free(devargs->args);
 	free(devargs);
 	free(dev);
-	return 0;
+
+unlock:
+	rte_spinlock_unlock(&vdev_device_list_lock);
+	return ret;
 }
 
 static int
@@ -314,19 +344,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;
@@ -340,6 +376,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;
@@ -360,15 +400,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)
-			return &dev->device;
+			break;
 	}
-	return NULL;
+	rte_spinlock_unlock(&vdev_device_list_lock);
+
+	return dev ? &dev->device : NULL;
 }
 
 static int
-- 
2.7.4