Commit 327d1c98 authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

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: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I9577901266975447f9acfe53475221113f02fea3
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15510


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent 081b190b

lib/vhost/vhost.c

+14 −3
Original line number Diff line number Diff line
@@ -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

lib/vhost/vhost_scsi.c

+4 −6
Original line number Diff line number Diff line
@@ -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 *