d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

bdev: defer unregister untill *all* descriptors are closed

Browse Source 
We used to wait only for those descriptors which
specified the hotremove notification callback. The
bdev could've been removed before the descriptor
was closed and the subsequent spdk_bdev_close would
simply segfault.

This patch modifies spdk_bdev_unregister to always
wait for all descriptors to be closed before actually
unregistering the bdev. This consolidates the bdev
unregister behavior for descriptors with and without
the hotremove callback.

Change-Id: I9b358209c6abd301b6fe8660e27bc6fa4ef485d6
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/450175
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-04 12:26:53 +02:00 committed by Jim Harris
parent 14032a984c
commit f93b6fb0a4
4 changed files with 40 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/spdk/bdev.h
View File

@ -259,9 +259,12 @@ 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. 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 remove_cb notification callback to be called when the bdev gets
* hotremoved. This will always be called on the same thread that
* spdk_bdev_open() was called on. It can be NULL, in which case the upper
* layer won't be notified about the bdev hotremoval. The descriptor will
* have to be manually closed to make the bdev unregister proceed.
* \param remove_ctx param for remove_cb.
* \param desc output parameter for the descriptor when operation is successful
* \return 0 if operation is successful, suitable errno value otherwise
*/

5
include/spdk/bdev_module.h
View File

@ -579,7 +579,10 @@ struct spdk_bdev_io {
int spdk_bdev_register(struct spdk_bdev *bdev);
/**
* Unregister a bdev
* Start unregistering a bdev. This will notify each currently open descriptor
* on this bdev about the hotremoval in hopes that the upper layers will stop
* using this bdev and manually close all the descriptors with spdk_bdev_close().
* The actual bdev unregistration may be deferred until all descriptors are closed.
*
* \param bdev Block device to unregister.
* \param cb_fn Callback function to be called when the unregister is complete.

4
lib/bdev/bdev.c
View File

@ -3793,9 +3793,10 @@ spdk_bdev_unregister_unsafe(struct spdk_bdev *bdev)
struct spdk_bdev_desc *desc, *tmp;
int rc = 0;
/* Notify each descriptor about hotremoval */
TAILQ_FOREACH_SAFE(desc, &bdev->internal.open_descs, link, tmp) {
rc = -EBUSY;
if (desc->remove_cb) {
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
@ -3810,6 +3811,7 @@ spdk_bdev_unregister_unsafe(struct spdk_bdev *bdev)
}
}
/* If there are no descriptors, proceed removing the bdev */
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);

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

@ -358,6 +358,33 @@ unregister_and_close(void)
spdk_bdev_close(g_desc);
poll_threads();
/* 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);
/* There is an open descriptor on the device. Unregister it,
* which can't proceed until the descriptor is closed. */
done = false;
spdk_bdev_unregister(&g_bdev.bdev, _bdev_unregistered, &done);
/* Poll the threads to allow all events to be processed */
poll_threads();
/* Make sure the bdev was not unregistered. We still have a
* descriptor open */
CU_ASSERT(done == false);
spdk_bdev_close(desc);
poll_threads();
/* The unregister should have completed */
CU_ASSERT(done == true);
/* Register the bdev again */
register_bdev(&g_bdev, "ut_bdev", &g_io_device);
remove_notify = false;
spdk_bdev_open(&g_bdev.bdev, true, _bdev_removed, &remove_notify, &desc);
CU_ASSERT(remove_notify == false);