From d40b665706033ebea1ee53abb44da3fbd64c4a64 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 28 Sep 2021 16:54:49 +0900 Subject: [PATCH] bdev/nvme: admin_passthru() submits to the first found unfailed ctrlr bdev_nvme_admin_passthru() chooses the first ctrlr which is not failed. Signed-off-by: Shuhei Matsumoto Change-Id: If41a1d1e1bde4bddfa92e5a385509daa3f0ce4de Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9525 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- module/bdev/nvme/bdev_nvme.c | 27 ++-- .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 116 ++++++++++++++++++ 2 files changed, 133 insertions(+), 10 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 7f954cc2c..30755dc78 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -4305,18 +4305,23 @@ bdev_nvme_admin_passthru(struct nvme_bdev_channel *nbdev_ch, struct nvme_bdev_io { struct nvme_io_path *io_path; struct nvme_ctrlr *nvme_ctrlr; + struct spdk_nvme_ctrlr *ctrlr = NULL; uint32_t max_xfer_size; - /* Admin commands are submitted only to the first nvme_ctrlr for now. - * - * TODO: This limitation will be removed in the following patches. - */ - io_path = STAILQ_FIRST(&nbdev_ch->io_path_list); - assert(io_path != NULL); + /* Choose the first ctrlr which is not failed. */ + STAILQ_FOREACH(io_path, &nbdev_ch->io_path_list, stailq) { + nvme_ctrlr = nvme_ctrlr_channel_get_ctrlr(io_path->ctrlr_ch); + if (!spdk_nvme_ctrlr_is_failed(nvme_ctrlr->ctrlr)) { + ctrlr = nvme_ctrlr->ctrlr; + break; + } + } - nvme_ctrlr = nvme_ctrlr_channel_get_ctrlr(io_path->ctrlr_ch); + if (ctrlr == NULL) { + return -ENXIO; + } - max_xfer_size = spdk_nvme_ctrlr_get_max_xfer_size(nvme_ctrlr->ctrlr); + max_xfer_size = spdk_nvme_ctrlr_get_max_xfer_size(ctrlr); if (nbytes > max_xfer_size) { SPDK_ERRLOG("nbytes is greater than MDTS %" PRIu32 ".\n", max_xfer_size); @@ -4325,8 +4330,8 @@ bdev_nvme_admin_passthru(struct nvme_bdev_channel *nbdev_ch, struct nvme_bdev_io bio->orig_thread = spdk_get_thread(); - return spdk_nvme_ctrlr_cmd_admin_raw(nvme_ctrlr->ctrlr, cmd, buf, - (uint32_t)nbytes, bdev_nvme_admin_passthru_done, bio); + return spdk_nvme_ctrlr_cmd_admin_raw(ctrlr, cmd, buf, (uint32_t)nbytes, + bdev_nvme_admin_passthru_done, bio); } static int @@ -4391,6 +4396,8 @@ bdev_nvme_abort(struct nvme_bdev_channel *nbdev_ch, struct nvme_bdev_io *bio, struct nvme_ctrlr *nvme_ctrlr; int rc = 0; + bio->io_path = NULL; + bio->orig_thread = spdk_get_thread(); /* Even admin commands, they were submitted to only nvme_ctrlrs which were diff --git a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c index a726779b2..831f45354 100644 --- a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c +++ b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c @@ -758,6 +758,12 @@ spdk_nvme_ctrlr_fail(struct spdk_nvme_ctrlr *ctrlr) ctrlr->is_failed = true; } +bool +spdk_nvme_ctrlr_is_failed(struct spdk_nvme_ctrlr *ctrlr) +{ + return ctrlr->is_failed; +} + #define UT_ANA_DESC_SIZE (sizeof(struct spdk_nvme_ana_group_descriptor) + \ sizeof(uint32_t)) static void @@ -3361,6 +3367,115 @@ test_add_multi_io_paths_to_nbdev_ch(void) CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); } +static void +test_admin_path(void) +{ + struct nvme_path_id path1 = {}, path2 = {}; + struct spdk_nvme_ctrlr *ctrlr1, *ctrlr2; + struct nvme_bdev_ctrlr *nbdev_ctrlr; + const int STRING_SIZE = 32; + const char *attached_names[STRING_SIZE]; + struct nvme_bdev *bdev; + struct spdk_io_channel *ch; + struct spdk_bdev_io *bdev_io; + int rc; + + memset(attached_names, 0, sizeof(char *) * STRING_SIZE); + ut_init_trid(&path1.trid); + ut_init_trid2(&path2.trid); + g_ut_attach_ctrlr_status = 0; + g_ut_attach_bdev_count = 1; + + set_thread(0); + + ctrlr1 = ut_attach_ctrlr(&path1.trid, 1, true, true); + SPDK_CU_ASSERT_FATAL(ctrlr1 != NULL); + + memset(&ctrlr1->ns[0].uuid, 1, sizeof(struct spdk_uuid)); + + rc = bdev_nvme_create(&path1.trid, "nvme0", attached_names, STRING_SIZE, 0, + attach_ctrlr_done, NULL, NULL, true); + CU_ASSERT(rc == 0); + + spdk_delay_us(1000); + poll_threads(); + + spdk_delay_us(g_opts.nvme_adminq_poll_period_us); + poll_threads(); + + ctrlr2 = ut_attach_ctrlr(&path2.trid, 1, true, true); + SPDK_CU_ASSERT_FATAL(ctrlr2 != NULL); + + memset(&ctrlr2->ns[0].uuid, 1, sizeof(struct spdk_uuid)); + + rc = bdev_nvme_create(&path2.trid, "nvme0", attached_names, STRING_SIZE, 0, + attach_ctrlr_done, NULL, NULL, true); + CU_ASSERT(rc == 0); + + spdk_delay_us(1000); + poll_threads(); + + spdk_delay_us(g_opts.nvme_adminq_poll_period_us); + poll_threads(); + + nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); + + bdev = nvme_bdev_ctrlr_get_bdev(nbdev_ctrlr, 1); + SPDK_CU_ASSERT_FATAL(bdev != NULL); + + ch = spdk_get_io_channel(bdev); + SPDK_CU_ASSERT_FATAL(ch != NULL); + + bdev_io = ut_alloc_bdev_io(SPDK_BDEV_IO_TYPE_NVME_ADMIN, bdev, ch); + bdev_io->u.nvme_passthru.cmd.opc = SPDK_NVME_OPC_GET_FEATURES; + + /* ctrlr1 is failed but ctrlr2 is not failed. admin command is + * submitted to ctrlr2. + */ + ctrlr1->is_failed = true; + bdev_io->internal.in_submit_request = true; + + bdev_nvme_submit_request(ch, bdev_io); + + CU_ASSERT(ctrlr1->adminq.num_outstanding_reqs == 0); + CU_ASSERT(ctrlr2->adminq.num_outstanding_reqs == 1); + CU_ASSERT(bdev_io->internal.in_submit_request == true); + + spdk_delay_us(g_opts.nvme_adminq_poll_period_us); + poll_threads(); + + CU_ASSERT(ctrlr2->adminq.num_outstanding_reqs == 0); + CU_ASSERT(bdev_io->internal.in_submit_request == false); + CU_ASSERT(bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS); + + /* both ctrlr1 and ctrlr2 are failed. admin command is failed to submit. */ + ctrlr2->is_failed = true; + bdev_io->internal.in_submit_request = true; + + bdev_nvme_submit_request(ch, bdev_io); + + CU_ASSERT(ctrlr1->adminq.num_outstanding_reqs == 0); + CU_ASSERT(ctrlr2->adminq.num_outstanding_reqs == 0); + CU_ASSERT(bdev_io->internal.in_submit_request == false); + CU_ASSERT(bdev_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED); + + free(bdev_io); + + spdk_put_io_channel(ch); + + poll_threads(); + + rc = bdev_nvme_delete("nvme0", &g_any_path); + CU_ASSERT(rc == 0); + + poll_threads(); + spdk_delay_us(1000); + poll_threads(); + + CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); +} + int main(int argc, const char **argv) { @@ -3391,6 +3506,7 @@ main(int argc, const char **argv) CU_ADD_TEST(suite, test_create_bdev_ctrlr); CU_ADD_TEST(suite, test_add_multi_ns_to_bdev); CU_ADD_TEST(suite, test_add_multi_io_paths_to_nbdev_ch); + CU_ADD_TEST(suite, test_admin_path); CU_basic_set_mode(CU_BRM_VERBOSE);