lib/vhost: separate out rte_vhost code from vhost

This patch separates out rte_vhost code responsible for
vhost init/fini and vdev register/unregister from vhost.

Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: Ie69ecd3b2659c805c9c0b0a0076996ef85c8fe71
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9535
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>

lib/vhost/rte_vhost_user.c

@@ -117,6 +117,49 @@ vhost_session_mem_unregister(struct rte_vhost_memory *mem)
 	}
 }
 
+int
+_stop_session(struct spdk_vhost_session *vsession)
+{
+	struct spdk_vhost_dev *vdev = vsession->vdev;
+	struct spdk_vhost_virtqueue *q;
+	int rc;
+	uint16_t i;
+
+	rc = vdev->backend->stop_session(vsession);
+	if (rc != 0) {
+		SPDK_ERRLOG("Couldn't stop device with vid %d.\n", vsession->vid);
+		return rc;
+	}
+
+	for (i = 0; i < vsession->max_queues; i++) {
+		q = &vsession->virtqueue[i];
+
+		/* vring.desc and vring.desc_packed are in a union struct
+		 * so q->vring.desc can replace q->vring.desc_packed.
+		 */
+		if (q->vring.desc == NULL) {
+			continue;
+		}
+
+		/* Packed virtqueues support up to 2^15 entries each
+		 * so left one bit can be used as wrap counter.
+		 */
+		if (q->packed.packed_ring) {
+			q->last_avail_idx = q->last_avail_idx |
+					    ((uint16_t)q->packed.avail_phase << 15);
+			q->last_used_idx = q->last_used_idx |
+					   ((uint16_t)q->packed.used_phase << 15);
+		}
+
+		rte_vhost_set_vring_base(vsession->vid, i, q->last_avail_idx, q->last_used_idx);
+	}
+
+	vhost_session_mem_unregister(vsession->mem);
+	free(vsession->mem);
+
+	return 0;
+}
+
 static int
 new_connection(int vid)
 {
@@ -488,3 +531,150 @@ spdk_vhost_set_socket_path(const char *basename)
 
 	return 0;
 }
+
+static void
+vhost_dev_thread_exit(void *arg1)
+{
+	spdk_thread_exit(spdk_get_thread());
+}
+
+int
+vhost_user_dev_register(struct spdk_vhost_dev *vdev, const char *name, struct spdk_cpuset *cpumask,
+			const struct spdk_vhost_dev_backend *backend)
+{
+	char path[PATH_MAX];
+
+	if (snprintf(path, sizeof(path), "%s%s", g_vhost_user_dev_dirname, name) >= (int)sizeof(path)) {
+		SPDK_ERRLOG("Resulting socket path for controller %s is too long: %s%s\n",
+				name,g_vhost_user_dev_dirname, name);
+		return -EINVAL;
+	}
+
+	vdev->path = strdup(path);
+	if (vdev->path == NULL) {
+		return -EIO;
+	}
+
+	vdev->thread = spdk_thread_create(vdev->name, cpumask);
+	if (vdev->thread == NULL) {
+		free(vdev->path);
+		SPDK_ERRLOG("Failed to create thread for vhost controller %s.\n", name);
+		return -EIO;
+	}
+
+	vdev->registered = true;
+	vdev->backend = backend;
+	TAILQ_INIT(&vdev->vsessions);
+
+	vhost_user_dev_set_coalescing(vdev, SPDK_VHOST_COALESCING_DELAY_BASE_US,
+				 SPDK_VHOST_VQ_IOPS_COALESCING_THRESHOLD);
+
+	if (vhost_register_unix_socket(path, name, vdev->virtio_features, vdev->disabled_features,
+				       vdev->protocol_features)) {
+		spdk_thread_send_msg(vdev->thread, vhost_dev_thread_exit, NULL);
+		free(vdev->path);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+int
+vhost_user_dev_unregister(struct spdk_vhost_dev *vdev)
+{
+	if (!TAILQ_EMPTY(&vdev->vsessions)) {
+		SPDK_ERRLOG("Controller %s has still valid connection.\n", vdev->name);
+		return -EBUSY;
+	}
+
+	if (vdev->registered && vhost_driver_unregister(vdev->path) != 0) {
+		SPDK_ERRLOG("Could not unregister controller %s with vhost library\n"
+			    "Check if domain socket %s still exists\n",
+			    vdev->name, vdev->path);
+		return -EIO;
+	}
+
+	spdk_thread_send_msg(vdev->thread, vhost_dev_thread_exit, NULL);
+	free(vdev->path);
+
+	return 0;
+}
+
+static bool g_vhost_user_started = false;
+
+int
+vhost_user_init(void)
+{
+	size_t len;
+
+	if (g_vhost_user_started) {
+		return 0;
+	}
+
+	if (g_vhost_user_dev_dirname[0] == '\0') {
+		if (getcwd(g_vhost_user_dev_dirname, sizeof(g_vhost_user_dev_dirname) - 1) == NULL) {
+			SPDK_ERRLOG("getcwd failed (%d): %s\n", errno, spdk_strerror(errno));
+			return -1;
+		}
+
+		len = strlen(g_vhost_user_dev_dirname);
+		if (g_vhost_user_dev_dirname[len - 1] != '/') {
+			g_vhost_user_dev_dirname[len] = '/';
+			g_vhost_user_dev_dirname[len + 1] = '\0';
+		}
+	}
+
+	g_vhost_user_started = true;
+
+	return 0;
+}
+
+static void *
+vhost_user_session_shutdown(void *arg)
+{
+	struct spdk_vhost_dev *vdev = NULL;
+	struct spdk_vhost_session *vsession;
+	vhost_fini_cb vhost_cb = arg;
+
+	for (vdev = spdk_vhost_dev_next(NULL); vdev != NULL;
+	     vdev = spdk_vhost_dev_next(vdev)) {
+		spdk_vhost_lock();
+		TAILQ_FOREACH(vsession, &vdev->vsessions, tailq) {
+			if (vsession->started) {
+				_stop_session(vsession);
+			}
+		}
+		spdk_vhost_unlock();
+		vhost_driver_unregister(vdev->path);
+		vdev->registered = false;
+	}
+
+	SPDK_INFOLOG(vhost, "Exiting\n");
+	spdk_thread_send_msg(g_vhost_init_thread, vhost_cb, NULL);
+	return NULL;
+}
+
+void
+vhost_user_fini(vhost_fini_cb vhost_cb)
+{
+	pthread_t tid;
+	int rc;
+
+	if (!g_vhost_user_started) {
+		vhost_cb(NULL);
+		return;
+	}
+
+	g_vhost_user_started = false;
+
+	/* rte_vhost API for removing sockets is not asynchronous. Since it may call SPDK
+	 * ops for stopping a device or removing a connection, we need to call it from
+	 * a separate thread to avoid deadlock.
+	 */
+	rc = pthread_create(&tid, NULL, &vhost_user_session_shutdown, vhost_cb);
+	if (rc < 0) {
+		SPDK_ERRLOG("Failed to start session shutdown thread (%d): %s\n", rc, spdk_strerror(rc));
+		abort();
+	}
+	pthread_detach(tid);
+}

lib/vhost/vhost.c

@@ -47,9 +47,7 @@ bool g_packed_ring_recovery = false;
 static struct spdk_cpuset g_vhost_core_mask;
 
 /* Thread performing all vhost management operations */
-static struct spdk_thread *g_vhost_init_thread;
+struct spdk_thread *g_vhost_init_thread = NULL;
-
-static spdk_vhost_fini_cb g_fini_cpl_cb;
 
 /** Return code for the current DPDK callback */
 static int g_dpdk_response;
@@ -852,17 +850,10 @@ vhost_parse_core_mask(const char *mask, struct spdk_cpuset *cpumask)
 	return 0;
 }
 
-static void
-vhost_dev_thread_exit(void *arg1)
-{
-	spdk_thread_exit(spdk_get_thread());
-}
-
 int
 vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str,
 		   const struct spdk_vhost_dev_backend *backend)
 {
-	char path[PATH_MAX];
 	struct spdk_cpuset cpumask = {};
 	int rc;
 
@@ -883,72 +874,36 @@ vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *ma
 		return -EEXIST;
 	}
 
-	if (snprintf(path, sizeof(path), "%s%s", g_vhost_user_dev_dirname, name) >= (int)sizeof(path)) {
-		SPDK_ERRLOG("Resulting socket path for controller %s is too long: %s%s\n",
-			    name, g_vhost_user_dev_dirname,  name);
-		return -EINVAL;
-	}
-
 	vdev->name = strdup(name);
-	vdev->path = strdup(path);
+	if (vdev->name == NULL) {
-	if (vdev->name == NULL || vdev->path == NULL) {
+		return -EIO;
-		rc = -EIO;
-		goto out;
 	}
 
-	vdev->thread = spdk_thread_create(vdev->name, &cpumask);
+	rc = vhost_user_dev_register(vdev, name, &cpumask, backend);
-	if (vdev->thread == NULL) {
+	if (rc != 0) {
-		SPDK_ERRLOG("Failed to create thread for vhost controller %s.\n", name);
+		free(vdev->name);
-		rc = -EIO;
+		return rc;
-		goto out;
-	}
-
-	vdev->registered = true;
-	vdev->backend = backend;
-	TAILQ_INIT(&vdev->vsessions);
-
-	vhost_user_dev_set_coalescing(vdev, SPDK_VHOST_COALESCING_DELAY_BASE_US,
-				      SPDK_VHOST_VQ_IOPS_COALESCING_THRESHOLD);
-
-	if (vhost_register_unix_socket(path, name, vdev->virtio_features, vdev->disabled_features,
-				       vdev->protocol_features)) {
-		spdk_thread_send_msg(vdev->thread, vhost_dev_thread_exit, NULL);
-		rc = -EIO;
-		goto out;
 	}
 
 	TAILQ_INSERT_TAIL(&g_vhost_devices, vdev, tailq);
 
 	SPDK_INFOLOG(vhost, "Controller %s: new controller added\n", vdev->name);
 	return 0;
-
-out:
-	free(vdev->name);
-	free(vdev->path);
-	return rc;
 }
 
 int
 vhost_dev_unregister(struct spdk_vhost_dev *vdev)
 {
-	if (!TAILQ_EMPTY(&vdev->vsessions)) {
+	int rc;
-		SPDK_ERRLOG("Controller %s has still valid connection.\n", vdev->name);
-		return -EBUSY;
-	}
 
-	if (vdev->registered && vhost_driver_unregister(vdev->path) != 0) {
+	rc = vhost_user_dev_unregister(vdev);
-		SPDK_ERRLOG("Could not unregister controller %s with vhost library\n"
+	if (rc != 0) {
-			    "Check if domain socket %s still exists\n",
+		return rc;
-			    vdev->name, vdev->path);
-		return -EIO;
 	}
 
 	SPDK_INFOLOG(vhost, "Controller %s: removed\n", vdev->name);
 
-	spdk_thread_send_msg(vdev->thread, vhost_dev_thread_exit, NULL);
-
 	free(vdev->name);
-	free(vdev->path);
 	TAILQ_REMOVE(&g_vhost_devices, vdev, tailq);
 	return 0;
 }
@@ -1128,49 +1083,6 @@ vhost_dev_foreach_session(struct spdk_vhost_dev *vdev,
 	spdk_thread_send_msg(vdev->thread, foreach_session, ev_ctx);
 }
 
-static int
-_stop_session(struct spdk_vhost_session *vsession)
-{
-	struct spdk_vhost_dev *vdev = vsession->vdev;
-	struct spdk_vhost_virtqueue *q;
-	int rc;
-	uint16_t i;
-
-	rc = vdev->backend->stop_session(vsession);
-	if (rc != 0) {
-		SPDK_ERRLOG("Couldn't stop device with vid %d.\n", vsession->vid);
-		return rc;
-	}
-
-	for (i = 0; i < vsession->max_queues; i++) {
-		q = &vsession->virtqueue[i];
-
-		/* vring.desc and vring.desc_packed are in a union struct
-		 * so q->vring.desc can replace q->vring.desc_packed.
-		 */
-		if (q->vring.desc == NULL) {
-			continue;
-		}
-
-		/* Packed virtqueues support up to 2^15 entries each
-		 * so left one bit can be used as wrap counter.
-		 */
-		if (q->packed.packed_ring) {
-			q->last_avail_idx = q->last_avail_idx |
-					    ((uint16_t)q->packed.avail_phase << 15);
-			q->last_used_idx = q->last_used_idx |
-					   ((uint16_t)q->packed.used_phase << 15);
-		}
-
-		rte_vhost_set_vring_base(vsession->vid, i, q->last_avail_idx, q->last_used_idx);
-	}
-
-	vhost_session_mem_unregister(vsession->mem);
-	free(vsession->mem);
-
-	return 0;
-}
-
 int
 vhost_stop_device_cb(int vid)
 {
@@ -1507,25 +1419,16 @@ spdk_vhost_unlock(void)
 void
 spdk_vhost_init(spdk_vhost_init_cb init_cb)
 {
-	size_t len;
 	uint32_t i;
 	int ret = 0;
 
 	g_vhost_init_thread = spdk_get_thread();
 	assert(g_vhost_init_thread != NULL);
 
-	if (g_vhost_user_dev_dirname[0] == '\0') {
+	ret = vhost_user_init();
-		if (getcwd(g_vhost_user_dev_dirname, sizeof(g_vhost_user_dev_dirname) - 1) == NULL) {
+	if (ret != 0) {
-			SPDK_ERRLOG("getcwd failed (%d): %s\n", errno, spdk_strerror(errno));
+		init_cb(ret);
-			init_cb(-1);
+		return;
-			return;
-		}
-
-		len = strlen(g_vhost_user_dev_dirname);
-		if (g_vhost_user_dev_dirname[len - 1] != '/') {
-			g_vhost_user_dev_dirname[len] = '/';
-			g_vhost_user_dev_dirname[len + 1] = '\0';
-		}
 	}
 
 	spdk_cpuset_zero(&g_vhost_core_mask);
@@ -1535,6 +1438,8 @@ spdk_vhost_init(spdk_vhost_init_cb init_cb)
 	init_cb(ret);
 }
 
+static spdk_vhost_fini_cb g_fini_cb;
+
 static void
 vhost_fini(void *arg1)
 {
@@ -1550,52 +1455,17 @@ vhost_fini(void *arg1)
 	}
 	spdk_vhost_unlock();
 
-	g_fini_cpl_cb();
+	g_fini_cb();
-}
-
-static void *
-session_shutdown(void *arg)
-{
-	struct spdk_vhost_dev *vdev = NULL;
-	struct spdk_vhost_session *vsession;
-
-	for (vdev = spdk_vhost_dev_next(NULL); vdev != NULL;
-	     vdev = spdk_vhost_dev_next(vdev)) {
-		spdk_vhost_lock();
-		TAILQ_FOREACH(vsession, &vdev->vsessions, tailq) {
-			if (vsession->started) {
-				_stop_session(vsession);
-			}
-		}
-		spdk_vhost_unlock();
-		vhost_driver_unregister(vdev->path);
-		vdev->registered = false;
-	}
-
-	SPDK_INFOLOG(vhost, "Exiting\n");
-	spdk_thread_send_msg(g_vhost_init_thread, vhost_fini, NULL);
-	return NULL;
 }
 
 void
 spdk_vhost_fini(spdk_vhost_fini_cb fini_cb)
 {
-	pthread_t tid;
-	int rc;
-
 	assert(spdk_get_thread() == g_vhost_init_thread);
-	g_fini_cpl_cb = fini_cb;
 
-	/* rte_vhost API for removing sockets is not asynchronous. Since it may call SPDK
+	g_fini_cb = fini_cb;
-	 * ops for stopping a device or removing a connection, we need to call it from
+
-	 * a separate thread to avoid deadlock.
+	vhost_user_fini(vhost_fini);
-	 */
-	rc = pthread_create(&tid, NULL, &session_shutdown, NULL);
-	if (rc < 0) {
-		SPDK_ERRLOG("Failed to start session shutdown thread (%d): %s\n", rc, spdk_strerror(rc));
-		abort();
-	}
-	pthread_detach(tid);
 }
 
 void

lib/vhost/vhost_internal.h

@@ -47,6 +47,9 @@
 
 extern bool g_packed_ring_recovery;
 
+/* Thread performing all vhost management operations */
+extern struct spdk_thread *g_vhost_init_thread;
+
 /**
  * DPDK calls our callbacks synchronously but the work those callbacks
  * perform needs to be async. Luckily, all DPDK callbacks are called on
@@ -522,5 +525,12 @@ int vhost_user_session_set_coalescing(struct spdk_vhost_dev *vdev,
 				      struct spdk_vhost_session *vsession, void *ctx);
 int vhost_user_dev_set_coalescing(struct spdk_vhost_dev *vdev, uint32_t delay_base_us,
 				  uint32_t iops_threshold);
+int vhost_user_dev_register(struct spdk_vhost_dev *vdev, const char *name,
+			    struct spdk_cpuset *cpumask, const struct spdk_vhost_dev_backend *backend);
+int vhost_user_dev_unregister(struct spdk_vhost_dev *vdev);
+int vhost_user_init(void);
+typedef void (*vhost_fini_cb)(void *ctx);
+void vhost_user_fini(vhost_fini_cb vhost_cb);
+int _stop_session(struct spdk_vhost_session *vsession);
 
 #endif /* SPDK_VHOST_INTERNAL_H */