Commit 563f69eb authored Jun 26, 2021 by Shuhei Matsumoto Committed by Tomasz Zawadzki Jul 05, 2021

bdev: spdk_bdev_get_by_name() hold mutex itself while traversing bdev name tree



spdk_bdev_register() and spdk_bdev_add_alias() had not held mutex when
adding bdev name or alias to global bdev name tree. This bug caused unexpected
error when traversing global bdev name tree.

The next patch will fix the bug. This patch is a preparation for the fix.

spdk_bdev_get_by_name() had not held mutex while traversing bdev
name tree. The major callers to spdk_bdev_get_by_name() had held mutex
when calling it. However, this was not clear.

Factor out the internal of spdk_bdev_get_by_name() into a helper
function bdev_get_by_name() and then change spdk_bdev_get_by_name()
to lock and unlock when calling bdev_get_by_name().

Then replace spdk_bdev_get_by_name() call in spdk_bdev_alias_add() and
bdev_register() by bdev_get_by_name() call.

spdk_bdev_get_by_name() call in spdk_bdev_examine() is not changed.
This is called only from JSON RPC and not related with the bug. So
we want to fix only unlocked access to global bdev name tree.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I25f07694e569eec10dba6c3c8543f6ce77412fe8
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8523


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>

parent 680388d4

lib/bdev/bdev.c

+17 −5

Original line number	Diff line number	Diff line
		@@ -475,8 +475,8 @@ spdk_bdev_set_opts(struct spdk_bdev_opts *opts)
		return 0;
		}

		struct spdk_bdev *
		spdk_bdev_get_by_name(const char *bdev_name)
		static struct spdk_bdev *
		bdev_get_by_name(const char *bdev_name)
		{
		struct spdk_bdev_name find;
		struct spdk_bdev_name *res;
		@@ -490,6 +490,18 @@ spdk_bdev_get_by_name(const char *bdev_name)
		return NULL;
		}

		struct spdk_bdev *
		spdk_bdev_get_by_name(const char *bdev_name)
		{
		struct spdk_bdev *bdev;

		pthread_mutex_lock(&g_bdev_mgr.mutex);
		bdev = bdev_get_by_name(bdev_name);
		pthread_mutex_unlock(&g_bdev_mgr.mutex);

		return bdev;
		}

		struct spdk_bdev_wait_for_examine_ctx {
		struct spdk_poller *poller;
		spdk_bdev_wait_for_examine_cb cb_fn;
		@@ -3265,7 +3277,7 @@ spdk_bdev_alias_add(struct spdk_bdev bdev, const char alias)
		return -EINVAL;
		}

		if (spdk_bdev_get_by_name(alias)) {
		if (bdev_get_by_name(alias)) {
		SPDK_ERRLOG("Bdev name/alias: %s already exists\n", alias);
		return -EEXIST;
		}
		@@ -5557,7 +5569,7 @@ bdev_register(struct spdk_bdev *bdev)
		return -EINVAL;
		}

		if (spdk_bdev_get_by_name(bdev->name)) {
		if (bdev_get_by_name(bdev->name)) {
		SPDK_ERRLOG("Bdev name:%s already exists\n", bdev->name);
		return -EEXIST;
		}
		@@ -5872,7 +5884,7 @@ spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t event

		pthread_mutex_lock(&g_bdev_mgr.mutex);

		bdev = spdk_bdev_get_by_name(bdev_name);
		bdev = bdev_get_by_name(bdev_name);

		if (bdev == NULL) {
		SPDK_NOTICELOG("Currently unable to find bdev with name: %s\n", bdev_name);

Admin message