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

lib/bdev: ensure mutex is initialized

Browse Source 
For correct behaviour, pthread_mutex should not be locked after it has
been destroyed.

g_bdev_mgr.mutex is statically initialized. It is destroyed in
bdev_mgr_unregister_cb, but not re-initialized in spdk_bdev_initialize.
Repeated calls to initialize/unregister occur during unit tests.
Remove the destroy from bdev_mgr_unregister_cb, which seems
the simplest way of resolving the issue.

The sequence: spdk_put_io_channel(), spdk_bdev_close(),
spdk_bdev_unregister() occurs during unit tests.
spdk_bdev_unregister() destroys internal.mutex which is then
locked by a call to bdev_channel_destroy() resulting from the
earlier spdk_put_io_channel(). Move the destroy and the free of
internal.qos into bdev_destroy_cb so that they don't occur until
all of the channels have been released. Remove the no longer
required bdev_fini.

Repeat calls to spdk_bdev_unregister that occur after an unregister has
completed will lock internal.mutex which has been destroyed by the
previous unregister. This occurs during unit tests. Defer locking
internal.mutex until after the internal.status has been checked for
SPDK_BDEV_STATUS_REMOVING. This is the only place where
internal.status is set to removing and g_bdev_mgr.mutex alone is
sufficient to ensure atomicity here.

Tested with a pthreads library that contains debugging code to
check the mutex state and a modified version of bdev_io_types_test
to call get_io_channel on a different thread.

Suggested-by: Jim Harris <james.r.harris@intel.com>
Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
Change-Id: I81cc46a1b8a766700253829b19cc86c7f0eb79f2
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6217
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
Nick Connolly 2021-02-02 09:38:46 +00:00 committed by Tomasz Zawadzki
parent 1450e5bdb2
commit 77573e830e
1 changed files with 6 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
lib/bdev/bdev.c
View File

@ -1475,7 +1475,6 @@ bdev_mgr_unregister_cb(void *io_device)
g_fini_cb_arg = NULL;
g_bdev_mgr.init_complete = false;
g_bdev_mgr.module_init_complete = false;
pthread_mutex_destroy(&g_bdev_mgr.mutex);
}
static void
@ -5498,6 +5497,9 @@ bdev_destroy_cb(void *io_device)
cb_fn = bdev->internal.unregister_cb;
cb_arg = bdev->internal.unregister_ctx;
pthread_mutex_destroy(&bdev->internal.mutex);
free(bdev->internal.qos);
rc = bdev->fn_table->destruct(bdev->ctxt);
if (rc < 0) {
SPDK_ERRLOG("destruct failed\n");
@ -5507,17 +5509,6 @@ bdev_destroy_cb(void *io_device)
}
}
static void
bdev_fini(struct spdk_bdev *bdev)
{
pthread_mutex_destroy(&bdev->internal.mutex);
free(bdev->internal.qos);
spdk_io_device_unregister(__bdev_to_io_dev(bdev), bdev_destroy_cb);
}
static void
bdev_start_finished(void *arg)
{
@ -5645,9 +5636,7 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void
}
pthread_mutex_lock(&g_bdev_mgr.mutex);
pthread_mutex_lock(&bdev->internal.mutex);
if (bdev->internal.status == SPDK_BDEV_STATUS_REMOVING) {
pthread_mutex_unlock(&bdev->internal.mutex);
pthread_mutex_unlock(&g_bdev_mgr.mutex);
if (cb_fn) {
cb_fn(cb_arg, -EBUSY);
@ -5655,6 +5644,7 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void
return;
}
pthread_mutex_lock(&bdev->internal.mutex);
bdev->internal.status = SPDK_BDEV_STATUS_REMOVING;
bdev->internal.unregister_cb = cb_fn;
bdev->internal.unregister_ctx = cb_arg;
@ -5665,7 +5655,7 @@ spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void
pthread_mutex_unlock(&g_bdev_mgr.mutex);
if (rc == 0) {
bdev_fini(bdev);
spdk_io_device_unregister(__bdev_to_io_dev(bdev), bdev_destroy_cb);
}
}
@ -5897,7 +5887,7 @@ spdk_bdev_close(struct spdk_bdev_desc *desc)
pthread_mutex_unlock(&bdev->internal.mutex);
if (rc == 0) {
bdev_fini(bdev);
spdk_io_device_unregister(__bdev_to_io_dev(bdev), bdev_destroy_cb);
}
} else {
pthread_mutex_unlock(&bdev->internal.mutex);