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 <james.r.harris@intel.com>
Change-Id: I9577901266975447f9acfe53475221113f02fea3
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15510
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This commit is contained in:
Jim Harris 2022-11-17 05:43:41 +00:00 committed by Tomasz Zawadzki
parent 081b190b5f
commit 327d1c988d
2 changed files with 18 additions and 9 deletions

View File

@ -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( static TAILQ_HEAD(, spdk_virtio_blk_transport) g_virtio_blk_transports = TAILQ_HEAD_INITIALIZER(
g_virtio_blk_transports); g_virtio_blk_transports);
static spdk_vhost_fini_cb g_fini_cb;
struct spdk_vhost_dev * struct spdk_vhost_dev *
spdk_vhost_dev_next(struct spdk_vhost_dev *vdev) spdk_vhost_dev_next(struct spdk_vhost_dev *vdev)
{ {
@ -170,6 +172,11 @@ vhost_dev_unregister(struct spdk_vhost_dev *vdev)
free(vdev->name); free(vdev->name);
TAILQ_REMOVE(&g_vhost_devices, vdev, tailq); TAILQ_REMOVE(&g_vhost_devices, vdev, tailq);
if (TAILQ_EMPTY(&g_vhost_devices) && g_fini_cb != NULL) {
g_fini_cb();
}
return 0; return 0;
} }
@ -237,14 +244,18 @@ spdk_vhost_scsi_init(spdk_vhost_init_cb init_cb)
init_cb(ret); init_cb(ret);
} }
static spdk_vhost_fini_cb g_fini_cb;
static void static void
vhost_fini(void) vhost_fini(void)
{ {
struct spdk_vhost_dev *vdev, *tmp; struct spdk_vhost_dev *vdev, *tmp;
spdk_vhost_lock(); spdk_vhost_lock();
if (spdk_vhost_dev_next(NULL) == NULL) {
spdk_vhost_unlock();
g_fini_cb();
return;
}
vdev = spdk_vhost_dev_next(NULL); vdev = spdk_vhost_dev_next(NULL);
while (vdev != NULL) { while (vdev != NULL) {
tmp = spdk_vhost_dev_next(vdev); tmp = spdk_vhost_dev_next(vdev);
@ -254,7 +265,7 @@ vhost_fini(void)
} }
spdk_vhost_unlock(); spdk_vhost_unlock();
g_fini_cb(); /* g_fini_cb will get called when last device is unregistered. */
} }
void void

View File

@ -187,6 +187,7 @@ remove_scsi_tgt(struct spdk_vhost_scsi_dev *svdev,
SPDK_INFOLOG(vhost, "removed target 'Target %u'\n", scsi_tgt_num); SPDK_INFOLOG(vhost, "removed target 'Target %u'\n", scsi_tgt_num);
if (--svdev->ref == 0 && svdev->registered == false) { if (--svdev->ref == 0 && svdev->registered == false) {
vhost_dev_unregister(&svdev->vdev);
free(svdev); 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_scsi_dev *svdev = to_scsi_dev(vdev);
struct spdk_vhost_user_dev *user_dev = vdev->ctxt; struct spdk_vhost_user_dev *user_dev = vdev->ctxt;
int rc, i; int rc = 0, i;
if (user_dev->pending_async_op_num) { if (user_dev->pending_async_op_num) {
return -EBUSY; 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; svdev->registered = false;
if (svdev->ref == 0) { if (svdev->ref == 0) {
rc = vhost_dev_unregister(vdev);
free(svdev); free(svdev);
} }
return 0; return rc;
} }
struct spdk_scsi_dev * struct spdk_scsi_dev *