Commit 898739fb authored by Ben Walker's avatar Ben Walker
Browse files

bdev: Enforce that spdk_bdev_close() is called on same thread as open() 



spdk_bdev_close() must be called on the same thread as
spdk_bdev_open(). Further, the remove callback on the
descriptor will also be run on the same thread as
spdk_bdev_open().

Change-Id: I949d6dd67de1e63d39f06944d473e4aa7134111b
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/424738


Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarGangCao <gang.cao@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
parent c5920554

include/spdk/bdev.h

+5 −1
Original line number Diff line number Diff line
@@ -234,7 +234,8 @@ struct spdk_bdev *spdk_bdev_next_leaf(struct spdk_bdev *prev); 
 *

 * \param bdev Block device to open.

 * \param write true is read/write access requested, false if read-only

 * \param remove_cb callback function for hot remove the device.

 * \param remove_cb callback function for hot remove the device. Will

 * always be called on the same thread that spdk_bdev_open() was called on.

 * \param remove_ctx param for hot removal callback function.

 * \param desc output parameter for the descriptor when operation is successful

 * \return 0 if operation is successful, suitable errno value otherwise
@@ -245,6 +246,9 @@ int spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t rem 
/**

 * Close a previously opened block device.

 *

 * Must be called on the same thread that the spdk_bdev_open()

 * was performed on.

 *

 * \param desc Block device descriptor to close.

 */

void spdk_bdev_close(struct spdk_bdev_desc *desc);

lib/bdev/bdev.c

+13 −2
Original line number Diff line number Diff line
@@ -259,6 +259,7 @@ struct spdk_bdev_channel { 


struct spdk_bdev_desc {

	struct spdk_bdev		*bdev;

	struct spdk_thread		*thread;

	spdk_bdev_remove_cb_t		remove_cb;

	void				*remove_ctx;

	bool				remove_scheduled;
@@ -3205,14 +3206,14 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void 
			do_destruct = false;

			/*

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

			 *  run later on this thread.  This ensures this context unwinds and

			 *  run later on its thread.  This ensures this context unwinds and

			 *  we don't recursively unregister this bdev again if the remove_cb

			 *  immediately closes its descriptor.

			 */

			if (!desc->remove_scheduled) {

				/* Avoid scheduling removal of the same descriptor multiple times. */

				desc->remove_scheduled = true;

				spdk_thread_send_msg(thread, _remove_notify, desc);

				spdk_thread_send_msg(desc->thread, _remove_notify, desc);

			}

		}

	}
@@ -3233,6 +3234,13 @@ spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_ 
	       void *remove_ctx, struct spdk_bdev_desc **_desc)

{

	struct spdk_bdev_desc *desc;

	struct spdk_thread *thread;



	thread = spdk_get_thread();

	if (!thread) {

		SPDK_ERRLOG("Cannot open bdev from non-SPDK thread.\n");

		return -ENOTSUP;

	}



	desc = calloc(1, sizeof(*desc));

	if (desc == NULL) {
@@ -3256,6 +3264,7 @@ spdk_bdev_open(struct spdk_bdev *bdev, bool write, spdk_bdev_remove_cb_t remove_ 
	TAILQ_INSERT_TAIL(&bdev->internal.open_descs, desc, link);



	desc->bdev = bdev;

	desc->thread = thread;

	desc->remove_cb = remove_cb;

	desc->remove_ctx = remove_ctx;

	desc->write = write;
@@ -3275,6 +3284,8 @@ spdk_bdev_close(struct spdk_bdev_desc *desc) 
	SPDK_DEBUGLOG(SPDK_LOG_BDEV, "Closing descriptor %p for bdev %s on thread %p\n", desc, bdev->name,

		      spdk_get_thread());



	assert(desc->thread == spdk_get_thread());



	pthread_mutex_lock(&bdev->internal.mutex);



	TAILQ_REMOVE(&bdev->internal.open_descs, desc, link);

test/unit/lib/bdev/mt/bdev.c/bdev_ut.c

+2 −0
Original line number Diff line number Diff line
@@ -255,6 +255,7 @@ setup_test(void) 
	bool done = false;



	allocate_threads(BDEV_UT_NUM_THREADS);

	set_thread(0);

	spdk_bdev_initialize(bdev_init_cb, &done);

	spdk_io_device_register(&g_io_device, stub_create_ch, stub_destroy_ch,

				sizeof(struct ut_bdev_channel), NULL);
@@ -271,6 +272,7 @@ finish_cb(void *cb_arg) 
static void

teardown_test(void)

{

	set_thread(0);

	g_teardown_done = false;

	spdk_bdev_close(g_desc);

	g_desc = NULL;