From 3e7394af6aee8eeb175340a5088a9957e2a30254 Mon Sep 17 00:00:00 2001 From: Mike Gerdts Date: Mon, 19 Dec 2022 09:21:10 -0600 Subject: [PATCH] bdev: remove bdev_register_examine_thread deprecation Starting in SPDK 23.01, calling spdk_bdev_register() and spdk_bdev_examine() from a thread other than the app thread was deprecated. This commit removes the deprecation and as such calling these functions from a thread other than the app thread is an error. As a side effect of this commit, all bdev module examine_config() and examine_disk() callbacks will be called on the app thread. Change-Id: Idaae06608101e2a513d9312ac5544ffe94effe4a Signed-off-by: Mike Gerdts Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15826 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto --- deprecation.md | 14 ------ lib/bdev/bdev.c | 15 +++--- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 70 +++++++++++--------------- 3 files changed, 37 insertions(+), 62 deletions(-) diff --git a/deprecation.md b/deprecation.md index 4b7b26592..3f9c0d19b 100644 --- a/deprecation.md +++ b/deprecation.md @@ -58,20 +58,6 @@ and will be removed in SPDK 23.05. Deprecated `spdk_nvme_ctrlr_prepare_for_reset` API, which will be removed in SPDK 22.01. For PCIe transport, `spdk_nvme_ctrlr_disconnect` should be used before freeing I/O qpairs. -### bdev - -#### `bdev_register_examine_thread` - -Deprecated calling `spdk_bdev_register()` and `spdk_bdev_examine()` from a thread other than the -app thread. See `spdk_thread_get_app_thread()`. Starting in SPDK 23.05, calling -`spdk_bdev_register()` or `spdk_bdev_examine()` from a thread other than the app thread will return -an error. - -With the removal of this deprecation, calls to vbdev modules' `examine_disk()` and -`examine_config()` callbacks will happen only on the app thread. This means that vbdev module -maintainers will not need to make any changes to examine callbacks that call `spdk_bdev_register()` -on the same thread as the examine callback uses. - ### gpt #### `old_gpt_guid` diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index b16c1b3a8..56f2c98a0 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -73,9 +73,6 @@ int __itt_init_ittlib(const char *, __itt_group_id); static void log_already_claimed(enum spdk_log_level level, const int line, const char *func, const char *detail, struct spdk_bdev *bdev); -SPDK_LOG_DEPRECATION_REGISTER(bdev_register_examine_thread, - "bdev register and examine on non-app thread", "SPDK 23.05", 0); - SPDK_LOG_DEPRECATION_REGISTER(vtune_support, "Intel(R) VTune integration", "SPDK 23.05", 0); static const char *qos_rpc_type[] = {"rw_ios_per_sec", @@ -770,9 +767,12 @@ spdk_bdev_examine(const char *name) { struct spdk_bdev *bdev; struct spdk_bdev_examine_item *item; + struct spdk_thread *thread = spdk_get_thread(); - if (spdk_unlikely(spdk_thread_get_app_thread() != spdk_get_thread())) { - SPDK_LOG_DEPRECATED(bdev_register_examine_thread); + if (spdk_unlikely(spdk_thread_get_app_thread() != thread)) { + SPDK_ERRLOG("Cannot examine bdev %s on thread %p (%s)\n", name, thread, + thread ? spdk_thread_get_name(thread) : "null"); + return -EINVAL; } if (g_bdev_opts.bdev_auto_examine) { @@ -7370,10 +7370,13 @@ int spdk_bdev_register(struct spdk_bdev *bdev) { struct spdk_bdev_desc *desc; + struct spdk_thread *thread = spdk_get_thread(); int rc; if (spdk_unlikely(spdk_thread_get_app_thread() != spdk_get_thread())) { - SPDK_LOG_DEPRECATED(bdev_register_examine_thread); + SPDK_ERRLOG("Cannot examine bdev %s on thread %p (%s)\n", bdev->name, thread, + thread ? spdk_thread_get_name(thread) : "null"); + return -EINVAL; } rc = bdev_register(bdev); 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 56ef58488..e78716973 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -2354,19 +2354,6 @@ bdev_init_wt_cb(void *done, int rc) } -static uint64_t -get_wrong_thread_hits(void) -{ - static uint64_t previous = 0; - uint64_t ret, current; - - current = spdk_deprecation_get_hits(_deprecated_bdev_register_examine_thread); - ret = current - previous; - previous = current; - - return ret; -} - static int wrong_thread_setup(void) { @@ -2380,18 +2367,13 @@ wrong_thread_setup(void) set_thread(1); - /* Ignore return, just setting the base for the next time it is called. */ - get_wrong_thread_hits(); - return 0; } static int wrong_thread_teardown(void) { - int rc; - - rc = get_wrong_thread_hits(); + int rc = 0; set_thread(0); @@ -2412,12 +2394,20 @@ wrong_thread_teardown(void) return rc; } +static void +_bdev_unregistered_wt(void *ctx, int rc) +{ + struct spdk_thread **threadp = ctx; + + *threadp = spdk_get_thread(); +} + static void spdk_bdev_register_wt(void) { struct spdk_bdev bdev = { 0 }; int rc; - bool done; + struct spdk_thread *unreg_thread; bdev.name = "wt_bdev"; bdev.fn_table = &fn_table; @@ -2427,26 +2417,29 @@ spdk_bdev_register_wt(void) /* Can register only on app thread */ rc = spdk_bdev_register(&bdev); - CU_ASSERT(rc == 0); - CU_ASSERT(get_wrong_thread_hits() == 1); + CU_ASSERT(rc == -EINVAL); /* Can unregister on any thread */ - done = false; - spdk_bdev_unregister(&bdev, _bdev_unregistered, &done); - poll_threads(); - CU_ASSERT(get_wrong_thread_hits() == 0); - CU_ASSERT(done); - - /* Can unregister by name on any thread */ + set_thread(0); rc = spdk_bdev_register(&bdev); CU_ASSERT(rc == 0); - CU_ASSERT(get_wrong_thread_hits() == 1); - done = false; - rc = spdk_bdev_unregister_by_name(bdev.name, bdev.module, _bdev_unregistered, &done); + set_thread(1); + unreg_thread = NULL; + spdk_bdev_unregister(&bdev, _bdev_unregistered_wt, &unreg_thread); + poll_threads(); + CU_ASSERT(unreg_thread == spdk_get_thread()); + + /* Can unregister by name on any thread */ + set_thread(0); + rc = spdk_bdev_register(&bdev); + CU_ASSERT(rc == 0); + set_thread(1); + unreg_thread = NULL; + rc = spdk_bdev_unregister_by_name(bdev.name, bdev.module, _bdev_unregistered_wt, + &unreg_thread); CU_ASSERT(rc == 0); poll_threads(); - CU_ASSERT(get_wrong_thread_hits() == 0); - CU_ASSERT(done); + CU_ASSERT(unreg_thread == spdk_get_thread()); } static void @@ -2473,12 +2466,9 @@ spdk_bdev_examine_wt(void) /* Can examine only on the app thread */ rc = spdk_bdev_examine("ut_bdev_wt"); - CU_ASSERT(rc == 0); - poll_threads(); - CU_ASSERT(get_wrong_thread_hits() == 1); + CU_ASSERT(rc == -EINVAL); unregister_bdev(&g_bdev); CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") == NULL); - CU_ASSERT(get_wrong_thread_hits() == 0); /* Can wait for examine on app thread, callback called on app thread. */ set_thread(0); @@ -2488,11 +2478,9 @@ spdk_bdev_examine_wt(void) rc = spdk_bdev_wait_for_examine(wait_for_examine_cb, &thread); CU_ASSERT(rc == 0); poll_threads(); - CU_ASSERT(get_wrong_thread_hits() == 0); CU_ASSERT(thread == spdk_get_thread()); unregister_bdev(&g_bdev); CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") == NULL); - CU_ASSERT(get_wrong_thread_hits() == 0); /* Can wait for examine on non-app thread, callback called on same thread. */ set_thread(0); @@ -2502,11 +2490,9 @@ spdk_bdev_examine_wt(void) rc = spdk_bdev_wait_for_examine(wait_for_examine_cb, &thread); CU_ASSERT(rc == 0); poll_threads(); - CU_ASSERT(get_wrong_thread_hits() == 0); CU_ASSERT(thread == spdk_get_thread()); unregister_bdev(&g_bdev); CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") == NULL); - CU_ASSERT(get_wrong_thread_hits() == 0); unregister_bdev(&g_bdev); g_bdev_opts.bdev_auto_examine = save_auto_examine;