Commit 0fe8cd17 authored by Pawel Wodkowski's avatar Pawel Wodkowski Committed by Jim Harris
Browse files

bdev: don't allow multiple unregister calls 



Unregister calls are not guarded. Fix this by chekcing status before
doing unregister.

Change-Id: I593e27efdae17f6d89362fd8e4edccf2af2b7281
Signed-off-by: default avatarPawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/445894


Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 5a051a6c

lib/bdev/bdev.c

+54 −31
Original line number Diff line number Diff line
@@ -3775,33 +3775,18 @@ _remove_notify(void *arg) 
	}

}



void

spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg)

/* Must be called while holding bdev->internal.mutex.

 * returns: 0 - bdev removed and ready to be destructed.

 *          -EBUSY - bdev can't be destructed yet.  */

static int

spdk_bdev_unregister_unsafe(struct spdk_bdev *bdev)

{

	struct spdk_bdev_desc	*desc, *tmp;

	bool			do_destruct = true;

	struct spdk_thread	*thread;



	SPDK_DEBUGLOG(SPDK_LOG_BDEV, "Removing bdev %s from list\n", bdev->name);



	thread = spdk_get_thread();

	if (!thread) {

		/* The user called this from a non-SPDK thread. */

		if (cb_fn != NULL) {

			cb_fn(cb_arg, -ENOTSUP);

		}

		return;

	}



	pthread_mutex_lock(&bdev->internal.mutex);



	bdev->internal.status = SPDK_BDEV_STATUS_REMOVING;

	bdev->internal.unregister_cb = cb_fn;

	bdev->internal.unregister_ctx = cb_arg;

	int			rc = 0;



	TAILQ_FOREACH_SAFE(desc, &bdev->internal.open_descs, link, tmp) {

		if (desc->remove_cb) {

			do_destruct = false;

			rc = -EBUSY;

			/*

			 * Defer invocation of the remove_cb to a separate message that will

			 *  run later on its thread.  This ensures this context unwinds and
@@ -3816,16 +3801,52 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void 
		}

	}



	if (!do_destruct) {

	if (rc == 0) {

		TAILQ_REMOVE(&g_bdev_mgr.bdevs, bdev, internal.link);

		SPDK_DEBUGLOG(SPDK_LOG_BDEV, "Removing bdev %s from list done\n", bdev->name);

	}



	return rc;

}



void

spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg)

{

	struct spdk_thread	*thread;

	int			rc;



	SPDK_DEBUGLOG(SPDK_LOG_BDEV, "Removing bdev %s from list\n", bdev->name);



	thread = spdk_get_thread();

	if (!thread) {

		/* The user called this from a non-SPDK thread. */

		if (cb_fn != NULL) {

			cb_fn(cb_arg, -ENOTSUP);

		}

		return;

	}



	pthread_mutex_lock(&bdev->internal.mutex);

	if (bdev->internal.status == SPDK_BDEV_STATUS_REMOVING) {

		pthread_mutex_unlock(&bdev->internal.mutex);

		if (cb_fn) {

			cb_fn(cb_arg, -EBUSY);

		}

		return;

	}



	TAILQ_REMOVE(&g_bdev_mgr.bdevs, bdev, internal.link);

	bdev->internal.status = SPDK_BDEV_STATUS_REMOVING;

	bdev->internal.unregister_cb = cb_fn;

	bdev->internal.unregister_ctx = cb_arg;



	/* Call under lock. */

	rc = spdk_bdev_unregister_unsafe(bdev);

	pthread_mutex_unlock(&bdev->internal.mutex);



	if (rc == 0) {

		spdk_bdev_fini(bdev);

	}

}



int

spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_cb,
@@ -3895,7 +3916,7 @@ void 
spdk_bdev_close(struct spdk_bdev_desc *desc)

{

	struct spdk_bdev *bdev = desc->bdev;

	bool do_unregister = false;

	int rc;



	SPDK_DEBUGLOG(SPDK_LOG_BDEV, "Closing descriptor %p for bdev %s on thread %p\n", desc, bdev->name,

		      spdk_get_thread());
@@ -3928,12 +3949,14 @@ spdk_bdev_close(struct spdk_bdev_desc *desc) 
	spdk_bdev_set_qd_sampling_period(bdev, 0);



	if (bdev->internal.status == SPDK_BDEV_STATUS_REMOVING && TAILQ_EMPTY(&bdev->internal.open_descs)) {

		do_unregister = true;

	}

		rc = spdk_bdev_unregister_unsafe(bdev);

		pthread_mutex_unlock(&bdev->internal.mutex);



	if (do_unregister == true) {

		spdk_bdev_unregister(bdev, bdev->internal.unregister_cb, bdev->internal.unregister_ctx);

		if (rc == 0) {

			spdk_bdev_fini(bdev);

		}

	} else {

		pthread_mutex_unlock(&bdev->internal.mutex);

	}

}