From 428b17a0a87f40717c58816cc9741efecd0fb0d1 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Fri, 1 Apr 2022 15:49:11 +0900 Subject: [PATCH] bdev: Add spdk_for_each_bdev/bdev_leaf for clean up and further improvements To execute a callback function for each registered bdev or unclaimed bdev, add new public APIs, spdk_for_each_bdev() and spdk_for_each_bdev_leaf(). These functions are safe for race conditions by opening before and closing after executing the provided callback function. Signed-off-by: Shuhei Matsumoto Change-Id: I59b702ffec7b4fc5e9779de5a3a75d44922b829b Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12088 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- CHANGELOG.md | 3 ++ doc/bdev_pg.md | 2 +- include/spdk/bdev.h | 38 +++++++++++++++ lib/bdev/bdev.c | 76 +++++++++++++++++++++++++++++ lib/bdev/spdk_bdev.map | 2 + test/unit/lib/bdev/bdev.c/bdev_ut.c | 59 ++++++++++++++++++++++ 6 files changed, 179 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 765f9afa3..f8be42cd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ Removed deprecated spdk_bdev_module_finish_done(). Use spdk_bdev_module_fini_don A new API `spdk_bdev_unregister_by_name` was added to handle race conditions correctly. +New APIs, `spdk_for_each_bdev` and `spdk_for_each_bdev_leaf`, were added to provide iteration +safe for race conditions. + ### idxd A new parameter `flags` was added to all low level submission and preparation diff --git a/doc/bdev_pg.md b/doc/bdev_pg.md index 67261c2c6..e3df6da88 100644 --- a/doc/bdev_pg.md +++ b/doc/bdev_pg.md @@ -63,7 +63,7 @@ to tear down the bdev library, call spdk_bdev_finish(). All block devices have a simple string name. At any time, a pointer to the device object can be obtained by calling spdk_bdev_get_by_name(), or the entire set of bdevs may be iterated using spdk_bdev_first() and spdk_bdev_next() and -their variants. +their variants or spdk_for_each_bdev() and its variant. Some block devices may also be given aliases, which are also string names. Aliases behave like symlinks - they can be used interchangeably with the real diff --git a/include/spdk/bdev.h b/include/spdk/bdev.h index c927873d3..84da4cdcd 100644 --- a/include/spdk/bdev.h +++ b/include/spdk/bdev.h @@ -378,6 +378,44 @@ int spdk_bdev_open_ext(const char *bdev_name, bool write, spdk_bdev_event_cb_t e */ void spdk_bdev_close(struct spdk_bdev_desc *desc); +/** + * Callback function for spdk_for_each_bdev() and spdk_for_each_bdev_leaf(). + * + * \param ctx Context passed to the callback. + * \param bdev Block device the callback handles. + */ +typedef int (*spdk_for_each_bdev_fn)(void *ctx, struct spdk_bdev *bdev); + +/** + * Call the provided callback function for every registered block device. + * If fn returns negated errno, spdk_for_each_bdev() terminates iteration. + * + * spdk_for_each_bdev() opens before and closes after executing the provided + * callback function for each bdev internally. + * + * \param ctx Context passed to the callback function. + * \param fn Callback function for each block device. + * + * \return 0 if operation is sucessful, or suitable errno value one of the + * callback returned otherwise. + */ +int spdk_for_each_bdev(void *ctx, spdk_for_each_bdev_fn fn); + +/** + * Call the provided callback function for every block device without virtual + * block devices on top. + * + * spdk_for_each_bdev_leaf() opens before and closes after executing the provided + * callback function for each unclaimed bdev internally. + * + * \param ctx Context passed to the callback function. + * \param fn Callback funciton for each block device without virtual block devices on top. + * + * \return 0 if operation is successful, or suitable errno value one of the + * callback returned otherwise. + */ +int spdk_for_each_bdev_leaf(void *ctx, spdk_for_each_bdev_fn fn); + /** * Get the bdev associated with a bdev descriptor. * diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 1e7482aa5..8f6baebf4 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -6572,6 +6572,82 @@ spdk_bdev_desc_get_bdev(struct spdk_bdev_desc *desc) return desc->bdev; } +int +spdk_for_each_bdev(void *ctx, spdk_for_each_bdev_fn fn) +{ + struct spdk_bdev *bdev, *tmp; + struct spdk_bdev_desc *desc; + int rc = 0; + + assert(fn != NULL); + + pthread_mutex_lock(&g_bdev_mgr.mutex); + bdev = spdk_bdev_first(); + while (bdev != NULL) { + rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, &desc); + if (rc != 0) { + break; + } + rc = bdev_open(bdev, false, desc); + if (rc != 0) { + bdev_desc_free(desc); + break; + } + pthread_mutex_unlock(&g_bdev_mgr.mutex); + + rc = fn(ctx, bdev); + + pthread_mutex_lock(&g_bdev_mgr.mutex); + tmp = spdk_bdev_next(bdev); + bdev_close(bdev, desc); + if (rc != 0) { + break; + } + bdev = tmp; + } + pthread_mutex_unlock(&g_bdev_mgr.mutex); + + return rc; +} + +int +spdk_for_each_bdev_leaf(void *ctx, spdk_for_each_bdev_fn fn) +{ + struct spdk_bdev *bdev, *tmp; + struct spdk_bdev_desc *desc; + int rc = 0; + + assert(fn != NULL); + + pthread_mutex_lock(&g_bdev_mgr.mutex); + bdev = spdk_bdev_first_leaf(); + while (bdev != NULL) { + rc = bdev_desc_alloc(bdev, _tmp_bdev_event_cb, NULL, &desc); + if (rc != 0) { + break; + } + rc = bdev_open(bdev, false, desc); + if (rc != 0) { + bdev_desc_free(desc); + break; + } + pthread_mutex_unlock(&g_bdev_mgr.mutex); + + rc = fn(ctx, bdev); + + pthread_mutex_lock(&g_bdev_mgr.mutex); + tmp = spdk_bdev_next_leaf(bdev); + bdev_close(bdev, desc); + if (rc != 0) { + break; + } + bdev = tmp; + } + pthread_mutex_unlock(&g_bdev_mgr.mutex); + + return rc; +} + void spdk_bdev_io_get_iovec(struct spdk_bdev_io *bdev_io, struct iovec **iovp, int *iovcntp) { diff --git a/lib/bdev/spdk_bdev.map b/lib/bdev/spdk_bdev.map index d88d282f2..203a392be 100644 --- a/lib/bdev/spdk_bdev.map +++ b/lib/bdev/spdk_bdev.map @@ -15,6 +15,8 @@ spdk_bdev_next; spdk_bdev_first_leaf; spdk_bdev_next_leaf; + spdk_for_each_bdev; + spdk_for_each_bdev_leaf; spdk_bdev_open_ext; spdk_bdev_close; spdk_bdev_desc_get_bdev; diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 7a83d9eaa..46b40e647 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -5256,6 +5256,64 @@ bdev_unregister_by_name(void) free_bdev(bdev); } +static int +count_bdevs(void *ctx, struct spdk_bdev *bdev) +{ + int *count = ctx; + + (*count)++; + + return 0; +} + +static void +for_each_bdev_test(void) +{ + struct spdk_bdev *bdev[8]; + int rc, count; + + bdev[0] = allocate_bdev("bdev0"); + + bdev[1] = allocate_bdev("bdev1"); + rc = spdk_bdev_module_claim_bdev(bdev[1], NULL, &bdev_ut_if); + CU_ASSERT(rc == 0); + + bdev[2] = allocate_bdev("bdev2"); + + bdev[3] = allocate_bdev("bdev3"); + rc = spdk_bdev_module_claim_bdev(bdev[3], NULL, &bdev_ut_if); + CU_ASSERT(rc == 0); + + bdev[4] = allocate_bdev("bdev4"); + + bdev[5] = allocate_bdev("bdev5"); + rc = spdk_bdev_module_claim_bdev(bdev[5], NULL, &bdev_ut_if); + CU_ASSERT(rc == 0); + + bdev[6] = allocate_bdev("bdev6"); + + bdev[7] = allocate_bdev("bdev7"); + + count = 0; + rc = spdk_for_each_bdev(&count, count_bdevs); + CU_ASSERT(rc == 0); + CU_ASSERT(count == 8); + + count = 0; + rc = spdk_for_each_bdev_leaf(&count, count_bdevs); + CU_ASSERT(rc == 0); + CU_ASSERT(count == 5); + + free_bdev(bdev[0]); + free_bdev(bdev[1]); + free_bdev(bdev[2]); + free_bdev(bdev[3]); + free_bdev(bdev[4]); + free_bdev(bdev[5]); + free_bdev(bdev[6]); + free_bdev(bdev[7]); +} + int main(int argc, char **argv) { @@ -5306,6 +5364,7 @@ main(int argc, char **argv) CU_ADD_TEST(suite, bdev_writev_readv_ext); CU_ADD_TEST(suite, bdev_register_uuid_alias); CU_ADD_TEST(suite, bdev_unregister_by_name); + CU_ADD_TEST(suite, for_each_bdev_test); allocate_cores(1); allocate_threads(1);