From 327d1c988d20caf4d21d663b66a3bdfce4498261 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 17 Nov 2022 05:43:41 +0000 Subject: [PATCH] vhost: defer vhost_dev_unregister until scsi tgts removed Currently when a vhost-scsi controller is removed, it calls spdk_vhost_scsi_dev_remove_tgt on all remaining targets, and then immediately calls vhost_dev_unregister. But this path goes into vhost_user_dev_unregister which immediately returns with error if there are any pending async operations - and there are since scsi_dev_remove_tgt is asynchronous. So instead add the vhost_dev_unregister call to remove_scsi_tgt, so that the unregister only happens after the last ref goes away. This requires changing vhost_fini() to no longer assume that spdk_vhost_dev_remove() will immediately unregister the device, since it now happens asynchronously. Previously vhost_fini() was making this assumption erroneously - it would call g_fini_cb without actually checking that the devices had been unregistered. Because of that incorrect assumption, we need to do both the vhost and vhost-scsi changes in the same patch. Signed-off-by: Jim Harris Change-Id: I9577901266975447f9acfe53475221113f02fea3 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15510 Tested-by: SPDK CI Jenkins Reviewed-by: Tomasz Zawadzki Reviewed-by: Changpeng Liu --- lib/vhost/vhost.c | 17 ++++++++++++++--- lib/vhost/vhost_scsi.c | 10 ++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 8b64e10d5..d94228aa8 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -23,6 +23,8 @@ static pthread_mutex_t g_vhost_mutex = PTHREAD_MUTEX_INITIALIZER; static TAILQ_HEAD(, spdk_virtio_blk_transport) g_virtio_blk_transports = TAILQ_HEAD_INITIALIZER( g_virtio_blk_transports); +static spdk_vhost_fini_cb g_fini_cb; + struct spdk_vhost_dev * spdk_vhost_dev_next(struct spdk_vhost_dev *vdev) { @@ -170,6 +172,11 @@ vhost_dev_unregister(struct spdk_vhost_dev *vdev) free(vdev->name); TAILQ_REMOVE(&g_vhost_devices, vdev, tailq); + + if (TAILQ_EMPTY(&g_vhost_devices) && g_fini_cb != NULL) { + g_fini_cb(); + } + return 0; } @@ -237,14 +244,18 @@ spdk_vhost_scsi_init(spdk_vhost_init_cb init_cb) init_cb(ret); } -static spdk_vhost_fini_cb g_fini_cb; - static void vhost_fini(void) { struct spdk_vhost_dev *vdev, *tmp; spdk_vhost_lock(); + if (spdk_vhost_dev_next(NULL) == NULL) { + spdk_vhost_unlock(); + g_fini_cb(); + return; + } + vdev = spdk_vhost_dev_next(NULL); while (vdev != NULL) { tmp = spdk_vhost_dev_next(vdev); @@ -254,7 +265,7 @@ vhost_fini(void) } spdk_vhost_unlock(); - g_fini_cb(); + /* g_fini_cb will get called when last device is unregistered. */ } void diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index fa4e46a6e..b9eb96806 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -187,6 +187,7 @@ remove_scsi_tgt(struct spdk_vhost_scsi_dev *svdev, SPDK_INFOLOG(vhost, "removed target 'Target %u'\n", scsi_tgt_num); if (--svdev->ref == 0 && svdev->registered == false) { + vhost_dev_unregister(&svdev->vdev); free(svdev); } } @@ -878,7 +879,7 @@ vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) { struct spdk_vhost_scsi_dev *svdev = to_scsi_dev(vdev); struct spdk_vhost_user_dev *user_dev = vdev->ctxt; - int rc, i; + int rc = 0, i; if (user_dev->pending_async_op_num) { return -EBUSY; @@ -900,17 +901,14 @@ vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) } } - rc = vhost_dev_unregister(vdev); - if (rc != 0) { - return rc; - } svdev->registered = false; if (svdev->ref == 0) { + rc = vhost_dev_unregister(vdev); free(svdev); } - return 0; + return rc; } struct spdk_scsi_dev *