From 283abcb9a2445183014b5bd5bc1511e837a71de6 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Sat, 20 Apr 2019 00:25:30 +0200 Subject: [PATCH] bdev: temporarily allow bdev descriptors to be closed from any thread Bdev descriptors could be closed only from the same thread that opened them. This restriction was suddenly introduced at one point without making sure all the SPDK code respects it. Vhost can still close descriptors from any arbitrary thread and fixing that would require some more effort. With this patch we remove the thread-specific assert from spdk_bdev_close() and hence allow vhost to work properly in debug builds. Vhost can still have a possible data race with bdev hotremove notification, but let's get rid of the abort() from the usual code path first. Change-Id: I6fac66a5ebc907b1c5418fff618f0b64cd9b69f4 Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/451561 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/bdev/bdev.c | 5 ++++- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 9586ef3f2..d7b61e9a1 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -3933,7 +3933,10 @@ 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()); + if (desc->thread != spdk_get_thread()) { + SPDK_ERRLOG("Descriptor %p for bdev %s closed on wrong thread (%p, expected %p)\n", + desc, bdev->name, spdk_get_thread(), desc->thread); + } pthread_mutex_lock(&bdev->internal.mutex); 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 5123cb8bf..0766af688 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -361,7 +361,7 @@ unregister_and_close(void) /* Try hotremoving a bdev with descriptors which don't provide * the notification callback */ spdk_bdev_open(&g_bdev.bdev, true, NULL, NULL, &desc); - CU_ASSERT(desc != NULL); + SPDK_CU_ASSERT_FATAL(desc != NULL); /* There is an open descriptor on the device. Unregister it, * which can't proceed until the descriptor is closed. */ @@ -387,8 +387,8 @@ unregister_and_close(void) remove_notify = false; spdk_bdev_open(&g_bdev.bdev, true, _bdev_removed, &remove_notify, &desc); + SPDK_CU_ASSERT_FATAL(desc != NULL); CU_ASSERT(remove_notify == false); - CU_ASSERT(desc != NULL); /* There is an open descriptor on the device. Unregister it, * which can't proceed until the descriptor is closed. */