ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bdev: temporarily allow bdev descriptors to be closed from any thread

Browse Source 
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 <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/451561
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This commit is contained in:
Darek Stojaczyk 2019-04-20 00:25:30 +02:00 committed by Jim Harris
parent f93b6fb0a4
commit 283abcb9a2
2 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/bdev/bdev.c
View File

@ -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_DEBUGLOG(SPDK_LOG_BDEV, "Closing descriptor %p for bdev %s on thread %p\n", desc, bdev->name,
spdk_get_thread()); 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); pthread_mutex_lock(&bdev->internal.mutex);

4
test/unit/lib/bdev/mt/bdev.c/bdev_ut.c
View File

@ -361,7 +361,7 @@ unregister_and_close(void)
/* Try hotremoving a bdev with descriptors which don't provide /* Try hotremoving a bdev with descriptors which don't provide
* the notification callback */ * the notification callback */
spdk_bdev_open(&g_bdev.bdev, true, NULL, NULL, &desc); 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, /* There is an open descriptor on the device. Unregister it,
* which can't proceed until the descriptor is closed. */ * which can't proceed until the descriptor is closed. */
@ -387,8 +387,8 @@ unregister_and_close(void)
remove_notify = false; remove_notify = false;
spdk_bdev_open(&g_bdev.bdev, true, _bdev_removed, &remove_notify, &desc); 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(remove_notify == false);
CU_ASSERT(desc != NULL);
/* There is an open descriptor on the device. Unregister it, /* There is an open descriptor on the device. Unregister it,
* which can't proceed until the descriptor is closed. */ * which can't proceed until the descriptor is closed. */