From 898739fbac0e1980c13b270a0484c5a1d0ff0e60 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Wed, 5 Sep 2018 16:14:47 -0700 Subject: [PATCH] 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: Ben Walker Reviewed-on: https://review.gerrithub.io/424738 Chandler-Test-Pool: SPDK Automated Test System Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: GangCao Reviewed-by: Changpeng Liu --- include/spdk/bdev.h | 6 +++++- lib/bdev/bdev.c | 15 +++++++++++++-- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 2 ++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/include/spdk/bdev.h b/include/spdk/bdev.h index c2852ea47..f75e80394 100644 --- a/include/spdk/bdev.h +++ b/include/spdk/bdev.h @@ -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); diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index b49f2661b..fe62e36e0 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -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); diff --git a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c index 5630908ce..39532c2d0 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -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;