From 9dda3d60b958f8cd2781deab37842198e052c426 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Wed, 12 Jan 2022 15:25:38 +0100 Subject: [PATCH] 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 Change-Id: Ie69ecd3b2659c805c9c0b0a0076996ef85c8fe71 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9535 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/vhost/rte_vhost_user.c | 190 +++++++++++++++++++++++++++++++++++++ lib/vhost/vhost.c | 172 ++++----------------------------- lib/vhost/vhost_internal.h | 10 ++ 3 files changed, 221 insertions(+), 151 deletions(-) diff --git a/lib/vhost/rte_vhost_user.c b/lib/vhost/rte_vhost_user.c index 059b5ace1f..583aa66edb 100644 --- a/lib/vhost/rte_vhost_user.c +++ b/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); +} diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 97eddc000d..a8baec793c 100644 --- a/lib/vhost/vhost.c +++ b/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; - -static spdk_vhost_fini_cb g_fini_cpl_cb; +struct spdk_thread *g_vhost_init_thread = NULL; /** 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 || vdev->path == NULL) { - rc = -EIO; - goto out; + if (vdev->name == NULL) { + return -EIO; } - vdev->thread = spdk_thread_create(vdev->name, &cpumask); - if (vdev->thread == NULL) { - SPDK_ERRLOG("Failed to create thread for vhost controller %s.\n", name); - rc = -EIO; - 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; + rc = vhost_user_dev_register(vdev, name, &cpumask, backend); + if (rc != 0) { + free(vdev->name); + return rc; } 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)) { - SPDK_ERRLOG("Controller %s has still valid connection.\n", vdev->name); - return -EBUSY; - } + int rc; - 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; + rc = vhost_user_dev_unregister(vdev); + if (rc != 0) { + return rc; } 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') { - 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)); - init_cb(-1); - 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'; - } + ret = vhost_user_init(); + if (ret != 0) { + init_cb(ret); + return; } 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(); -} - -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; + g_fini_cb(); } 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 - * 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, &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); + g_fini_cb = fini_cb; + + vhost_user_fini(vhost_fini); } void diff --git a/lib/vhost/vhost_internal.h b/lib/vhost/vhost_internal.h index b2068f6e9f..1dc8c04890 100644 --- a/lib/vhost/vhost_internal.h +++ b/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 */