diff --git a/deprecation.md b/deprecation.md index 88adb2df5..fe9fbeab0 100644 --- a/deprecation.md +++ b/deprecation.md @@ -21,7 +21,21 @@ The tags can be matched with the level 4 headers below. ### nvme -#### `nvme_ctrlr_prepare_for_reset`` +#### `nvme_ctrlr_prepare_for_reset` 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. diff --git a/include/spdk/bdev.h b/include/spdk/bdev.h index 7242385b4..5f1559b36 100644 --- a/include/spdk/bdev.h +++ b/include/spdk/bdev.h @@ -252,6 +252,8 @@ int spdk_bdev_wait_for_examine(spdk_bdev_wait_for_examine_cb cb_fn, void *cb_arg /** * Examine a block device explicitly * + * This function must be called from the SPDK app thread. + * * \param name the name or alias of the block device * \return 0 if block device was examined successfully, suitable errno value otherwise */ diff --git a/include/spdk/bdev_module.h b/include/spdk/bdev_module.h index 275815a65..58c1b17a5 100644 --- a/include/spdk/bdev_module.h +++ b/include/spdk/bdev_module.h @@ -849,6 +849,8 @@ struct spdk_bdev_io { /** * Register a new bdev. * + * This function must be called from the SPDK app thread. + * * \param bdev Block device to register. * * \return 0 on success. diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 908ef8cd1..58f2666cd 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -61,6 +61,9 @@ int __itt_init_ittlib(const char *, __itt_group_id); */ #define SPDK_BDEV_MAX_CHILDREN_COPY_REQS (8) +SPDK_LOG_DEPRECATION_REGISTER(bdev_register_examine_thread, + "bdev register and examine on non-app thread", "SPDK 23.05", 0); + static const char *qos_rpc_type[] = {"rw_ios_per_sec", "rw_mbytes_per_sec", "r_mbytes_per_sec", "w_mbytes_per_sec" }; @@ -677,6 +680,10 @@ spdk_bdev_examine(const char *name) struct spdk_bdev *bdev; struct spdk_bdev_examine_item *item; + if (spdk_unlikely(spdk_thread_get_app_thread() != spdk_get_thread())) { + SPDK_LOG_DEPRECATED(bdev_register_examine_thread); + } + if (g_bdev_opts.bdev_auto_examine) { SPDK_ERRLOG("Manual examine is not allowed if auto examine is enabled"); return -EINVAL; @@ -7214,6 +7221,10 @@ spdk_bdev_register(struct spdk_bdev *bdev) struct spdk_bdev_desc *desc; int rc; + if (spdk_unlikely(spdk_thread_get_app_thread() != spdk_get_thread())) { + SPDK_LOG_DEPRECATED(bdev_register_examine_thread); + } + rc = bdev_register(bdev); if (rc != 0) { return rc; 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 3763e7edd..65a435f0f 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: BSD-3-Clause * Copyright (C) 2017 Intel Corporation. * All rights reserved. + * Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved. */ #include "spdk_cunit.h" @@ -2293,16 +2294,182 @@ unregister_during_reset(void) teardown_test(); } +static void +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) +{ + allocate_cores(1); + allocate_threads(2); + set_thread(0); + + spdk_bdev_initialize(bdev_init_wt_cb, NULL); + spdk_io_device_register(&g_io_device, stub_create_ch, stub_destroy_ch, + sizeof(struct ut_bdev_channel), NULL); + + 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(); + + set_thread(0); + + g_teardown_done = false; + spdk_io_device_unregister(&g_io_device, NULL); + spdk_bdev_finish(finish_cb, NULL); + poll_threads(); + memset(&g_bdev, 0, sizeof(g_bdev)); + if (!g_teardown_done) { + fprintf(stderr, "%s:%d %s: teardown not done\n", __FILE__, __LINE__, __func__); + rc = -1; + } + g_teardown_done = false; + + free_threads(); + free_cores(); + + return rc; +} + +static void +spdk_bdev_register_wt(void) +{ + struct spdk_bdev bdev = { 0 }; + int rc; + bool done; + + bdev.name = "wt_bdev"; + bdev.fn_table = &fn_table; + bdev.module = &bdev_ut_if; + bdev.blocklen = 4096; + bdev.blockcnt = 1024; + + /* Can register only on app thread */ + rc = spdk_bdev_register(&bdev); + CU_ASSERT(rc == 0); + CU_ASSERT(get_wrong_thread_hits() == 1); + + /* 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 */ + 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); + CU_ASSERT(rc == 0); + poll_threads(); + CU_ASSERT(get_wrong_thread_hits() == 0); + CU_ASSERT(done); +} + +static void +wait_for_examine_cb(void *arg) +{ + struct spdk_thread **thread = arg; + + *thread = spdk_get_thread(); +} + +static void +spdk_bdev_examine_wt(void) +{ + int rc; + bool save_auto_examine = g_bdev_opts.bdev_auto_examine; + struct spdk_thread *thread; + + g_bdev_opts.bdev_auto_examine = false; + + set_thread(0); + register_bdev(&g_bdev, "ut_bdev_wt", &g_io_device); + CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") != NULL); + set_thread(1); + + /* 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); + 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); + register_bdev(&g_bdev, "ut_bdev_wt", &g_io_device); + CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") != NULL); + thread = NULL; + 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); + register_bdev(&g_bdev, "ut_bdev_wt", &g_io_device); + CU_ASSERT(spdk_bdev_get_by_name("ut_bdev_wt") != NULL); + thread = NULL; + 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; +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; + CU_pSuite suite_wt = NULL; unsigned int num_failures; CU_set_error_action(CUEA_ABORT); CU_initialize_registry(); suite = CU_add_suite("bdev", NULL, NULL); + suite_wt = CU_add_suite("bdev_wrong_thread", wrong_thread_setup, wrong_thread_teardown); CU_ADD_TEST(suite, basic); CU_ADD_TEST(suite, unregister_and_close); @@ -2323,6 +2490,8 @@ main(int argc, char **argv) CU_ADD_TEST(suite, bdev_set_io_timeout_mt); CU_ADD_TEST(suite, lock_lba_range_then_submit_io); CU_ADD_TEST(suite, unregister_during_reset); + CU_ADD_TEST(suite_wt, spdk_bdev_register_wt); + CU_ADD_TEST(suite_wt, spdk_bdev_examine_wt); CU_basic_set_mode(CU_BRM_VERBOSE); CU_basic_run_tests();