From c899854d0371a7cdb3e2fd8c07ccf3d1f0b8089a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 2 Aug 2018 11:20:56 -0700 Subject: [PATCH] bdev: add new fini_start notification callback for modules When an SPDK application shuts down, the bdev layer will automatically unregister all of the bdevs to ensure they are properly quiesced and cleaned up. Some modules may want to perform different operations when a bdev is destructed during normal runtime vs. shutdown. For example, for lvol, when the last lvol is cleaned up, it should unload the lvolstore, release and close the bdev that contains the lvolstore. You never want to do this during normal runtime though - it is perfectly valid to have an lvolstore that contains no lvols. RAID and future bdev modules such as multipath have similar use cases. So add a new bdev module callback named "fini_start". If a module specifies a function pointer for this callback, the bdev layer will call it before it starts the bdev unregistrations. This enables some future patches to the bdev layer such that it will always unregister block devices that are not claimed (i.e. logical volumes) before block devices that are claimed (i.e. the bdev containing an lvolstore). Signed-off-by: Jim Harris Change-Id: I6e87f5c2b27f16731ea5def858f26e882a29495a Reviewed-on: https://review.gerrithub.io/421175 Tested-by: SPDK CI Jenkins Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Changpeng Liu Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto --- include/spdk/bdev_module.h | 11 ++++++++++- lib/bdev/bdev.c | 8 ++++++++ test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 10 ++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/include/spdk/bdev_module.h b/include/spdk/bdev_module.h index 317dd64ad..074cb6438 100644 --- a/include/spdk/bdev_module.h +++ b/include/spdk/bdev_module.h @@ -67,9 +67,18 @@ struct spdk_bdev_module { */ void (*init_complete)(void); + /** + * Optional callback for modules that require notification of when + * the bdev subsystem is starting the fini process. + * + * Modules are not required to define this function. + */ + void (*fini_start)(void); + /** * Finish function for the module. Called by the spdk application - * before the spdk application exits to perform any necessary cleanup. + * after all bdevs for all modules have been unregistered. This allows + * the module to do any final cleanup before the SPDK application exits. * * Modules are not required to define this function. */ diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index c8b28a2a9..20ba9a731 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -932,6 +932,8 @@ _spdk_bdev_finish_unregister_bdevs_iter(void *cb_arg, int bdeverrno) void spdk_bdev_finish(spdk_bdev_fini_cb cb_fn, void *cb_arg) { + struct spdk_bdev_module *m; + assert(cb_fn != NULL); g_fini_thread = spdk_get_thread(); @@ -939,6 +941,12 @@ spdk_bdev_finish(spdk_bdev_fini_cb cb_fn, void *cb_arg) g_fini_cb_fn = cb_fn; g_fini_cb_arg = cb_arg; + TAILQ_FOREACH(m, &g_bdev_mgr.bdev_modules, internal.tailq) { + if (m->fini_start) { + m->fini_start(); + } + } + _spdk_bdev_finish_unregister_bdevs_iter(NULL, 0); } diff --git a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c index 7644f45a3..680e1c95e 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -71,6 +71,7 @@ bool g_teardown_done = false; bool g_get_io_channel = true; bool g_create_ch = true; bool g_init_complete_called = false; +bool g_fini_start_called = true; static int stub_create_ch(void *io_device, void *ctx_buf) @@ -190,11 +191,18 @@ init_complete(void) g_init_complete_called = true; } +static void +fini_start(void) +{ + g_fini_start_called = true; +} + struct spdk_bdev_module bdev_ut_if = { .name = "bdev_ut", .module_init = module_init, .module_fini = module_fini, .init_complete = init_complete, + .fini_start = fini_start, }; SPDK_BDEV_MODULE_REGISTER(&bdev_ut_if) @@ -302,7 +310,9 @@ basic(void) CU_ASSERT(g_ut_threads[0].ch != NULL); spdk_put_io_channel(g_ut_threads[0].ch); + g_fini_start_called = false; teardown_test(); + CU_ASSERT(g_fini_start_called == true); } static void