From dbdf54f1c9f39195d597c3b0825f4685c2e950fd Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Wed, 4 Aug 2021 08:56:37 -0400 Subject: [PATCH] bdev/lvol: unload empty lvs during fini_start This patch makes use of async_fini_start flag to make fini_start asynchronous. During this time all lvol stores which have no open lvols are unloaded. This is required, since lvs holds claim on the underlying bdev. Fixes #1630 Signed-off-by: Tomasz Zawadzki Change-Id: If443cb087324d08a4a70df71c7afd930ab654f90 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9095 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu Reviewed-by: Konrad Sztyber Tested-by: SPDK CI Jenkins --- module/bdev/lvol/vbdev_lvol.c | 38 +++++++++++++++++++ .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 26 ++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/module/bdev/lvol/vbdev_lvol.c b/module/bdev/lvol/vbdev_lvol.c index 652fce8d8..45f5231b0 100644 --- a/module/bdev/lvol/vbdev_lvol.c +++ b/module/bdev/lvol/vbdev_lvol.c @@ -53,6 +53,7 @@ static struct spdk_bdev_module g_lvol_if = { .name = "lvol", .module_init = vbdev_lvs_init, .fini_start = vbdev_lvs_fini_start, + .async_fini_start = true, .examine_disk = vbdev_lvs_examine, .get_ctx_size = vbdev_lvs_get_ctx_size, @@ -1230,10 +1231,47 @@ vbdev_lvs_init(void) return 0; } +static void vbdev_lvs_fini_start_iter(struct lvol_store_bdev *lvs_bdev); + +static void +vbdev_lvs_fini_start_unload_cb(void *cb_arg, int lvserrno) +{ + struct lvol_store_bdev *lvs_bdev = cb_arg; + struct lvol_store_bdev *next_lvs_bdev = vbdev_lvol_store_next(lvs_bdev); + + if (lvserrno != 0) { + SPDK_INFOLOG(vbdev_lvol, "Lvol store removed with error: %d.\n", lvserrno); + } + + TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores); + free(lvs_bdev); + + vbdev_lvs_fini_start_iter(next_lvs_bdev); +} + +static void +vbdev_lvs_fini_start_iter(struct lvol_store_bdev *lvs_bdev) +{ + struct spdk_lvol_store *lvs; + + while (lvs_bdev != NULL) { + lvs = lvs_bdev->lvs; + + if (_vbdev_lvs_are_lvols_closed(lvs)) { + spdk_lvs_unload(lvs, vbdev_lvs_fini_start_unload_cb, lvs_bdev); + return; + } + lvs_bdev = vbdev_lvol_store_next(lvs_bdev); + } + + spdk_bdev_module_fini_start_done(); +} + static void vbdev_lvs_fini_start(void) { g_shutdown_started = true; + vbdev_lvs_fini_start_iter(vbdev_lvol_store_first()); } static int diff --git a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c index 9deec5901..ba1db673e 100644 --- a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c +++ b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c @@ -61,6 +61,8 @@ bool g_examine_done = false; bool g_bdev_alias_already_exists = false; bool g_lvs_with_name_already_exists = false; +DEFINE_STUB_V(spdk_bdev_module_fini_start_done, (void)); + const struct spdk_bdev_aliases_list * spdk_bdev_get_aliases(const struct spdk_bdev *bdev) { @@ -1042,7 +1044,29 @@ ut_bdev_finish(void) int sz = 10; int rc; - /* Test creating lvs with two lvols. Delete first lvol explicitly, + /* Scenario 1 + * Test unload of lvs with no lvols during bdev finish. */ + + rc = vbdev_lvs_create("bdev", "lvs", 0, LVS_CLEAR_WITH_UNMAP, + lvol_store_op_with_handle_complete, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_lvserrno == 0); + SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); + lvs = g_lvol_store; + + /* Start bdev finish */ + vbdev_lvs_fini_start(); + CU_ASSERT(g_shutdown_started == true); + + /* During shutdown, lvs with no lvols should be unloaded */ + CU_ASSERT(g_lvol_store == NULL); + CU_ASSERT(TAILQ_EMPTY(&g_spdk_lvol_pairs)); + + /* Revert module state back to normal */ + g_shutdown_started = false; + + /* Scenario 2 + * Test creating lvs with two lvols. Delete first lvol explicitly, * then start bdev finish. This should unload the remaining lvol and * lvol store. */