From 64d76e50cc587a460b327cead12c533a1fc3eead Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Sun, 17 Mar 2019 00:50:51 +0100 Subject: [PATCH] vhost: call session_event_done() always under the global vhost lock In the next patch we will put much more responsibility on spdk_vhost_session_event_done(), so here we make sure it's always called under the global vhost mutex. Specifically, spdk_vhost_session_event_done() will set vsession->lcore, which any other thread might try to concurrently access via spdk_vhost_dev_foreach_session(). Change-Id: I7a5fde4be4e8bdfdbbb24ac955af964f516bdb68 Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/448227 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/vhost/vhost.c | 9 +++++++++ lib/vhost/vhost_blk.c | 5 +++++ lib/vhost/vhost_internal.h | 8 +++++--- lib/vhost/vhost_nvme.c | 5 +++++ lib/vhost/vhost_scsi.c | 4 ++++ 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 55f56d5da..a0506fbc9 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -886,9 +886,18 @@ spdk_vhost_event_cb(void *arg1, void *arg2) { struct spdk_vhost_session_fn_ctx *ctx = arg1; struct spdk_vhost_session *vsession; + struct spdk_event *ev; + + if (pthread_mutex_trylock(&g_spdk_vhost_mutex) != 0) { + ev = spdk_event_allocate(spdk_env_get_current_core(), + spdk_vhost_event_cb, arg1, arg2); + spdk_event_call(ev); + return; + } vsession = spdk_vhost_session_find_by_id(ctx->vdev, ctx->vsession_id); ctx->cb_fn(ctx->vdev, vsession, NULL); + pthread_mutex_unlock(&g_spdk_vhost_mutex); } static void spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev, diff --git a/lib/vhost/vhost_blk.c b/lib/vhost/vhost_blk.c index 3f1cb2b40..7a73aa1fd 100644 --- a/lib/vhost/vhost_blk.c +++ b/lib/vhost/vhost_blk.c @@ -754,6 +754,10 @@ destroy_session_poller_cb(void *arg) return -1; } + if (spdk_vhost_trylock() != 0) { + return -1; + } + for (i = 0; i < vsession->max_queues; i++) { vsession->virtqueue[i].next_event_time = 0; spdk_vhost_vq_used_signal(vsession, &vsession->virtqueue[i]); @@ -770,6 +774,7 @@ destroy_session_poller_cb(void *arg) spdk_poller_unregister(&bvsession->stop_poller); spdk_vhost_session_event_done(vsession, 0); + spdk_vhost_unlock(); return -1; } diff --git a/lib/vhost/vhost_internal.h b/lib/vhost/vhost_internal.h index c2a30809a..a62c0db19 100644 --- a/lib/vhost/vhost_internal.h +++ b/lib/vhost/vhost_internal.h @@ -321,9 +321,9 @@ void spdk_vhost_dev_foreach_session(struct spdk_vhost_dev *dev, * Call the provided function on the session's lcore and block until * spdk_vhost_session_event_done() is called. * - * As an optimization, this function will unlock the vhost mutex - * while it's waiting, which makes it prone to data races. - * Practically, it is only useful for session start/stop and still + * This must be called under the global vhost mutex, which this function + * will unlock for the time it's waiting. This makes it prone to data races, + * so practically it is only useful for session start/stop and still * has to be used with extra caution. * * \param vsession vhost session @@ -340,6 +340,8 @@ int spdk_vhost_session_send_event(struct spdk_vhost_session *vsession, /** * Finish a blocking spdk_vhost_session_send_event() call. * + * Must be called under the global vhost mutex. + * * \param vsession vhost session * \param response return code */ diff --git a/lib/vhost/vhost_nvme.c b/lib/vhost/vhost_nvme.c index ea86d7766..5097785b0 100644 --- a/lib/vhost/vhost_nvme.c +++ b/lib/vhost/vhost_nvme.c @@ -1164,6 +1164,10 @@ destroy_device_poller_cb(void *arg) /* FIXME wait for pending I/Os to complete */ + if (spdk_vhost_trylock() != 0) { + return -1; + } + for (i = 0; i < nvme->num_ns; i++) { ns_dev = &nvme->ns[i]; if (ns_dev->bdev_io_channel) { @@ -1184,6 +1188,7 @@ destroy_device_poller_cb(void *arg) spdk_poller_unregister(&nvme->stop_poller); spdk_vhost_session_event_done(nvme->vsession, 0); + spdk_vhost_unlock(); return -1; } diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index e9d5971c7..6d87f7119 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -1365,6 +1365,9 @@ destroy_session_poller_cb(void *arg) return -1; } + if (spdk_vhost_trylock() != 0) { + return -1; + } for (i = 0; i < vsession->max_queues; i++) { spdk_vhost_vq_used_signal(vsession, &vsession->virtqueue[i]); @@ -1386,6 +1389,7 @@ destroy_session_poller_cb(void *arg) spdk_poller_unregister(&svsession->stop_poller); spdk_vhost_session_event_done(vsession, 0); + spdk_vhost_unlock(); return -1; }