From 4058ffecd7f35b61820b37c3e3ac4475739ceaca Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Sun, 31 Oct 2021 19:38:18 +0900 Subject: [PATCH] bdev/nvme: Redirect the reset ctrlr operation into nvme_ctrlr->thread In the following patches, we want to retry reconnect if reconnect failed in a reset ctrlr sequence but we want to delay the retry. While we wait the delayed retry, we want to quiesce ctrlr completely. As part of quiesce ctrlr operations, we want to pause adminq poller but we need to do it on the nvme_ctrlr->thread. If a reset ctrlr sequence runs on the nvme_ctrlr->thread, we can avoid redirecting the pending destruct request at completion too. So we redirect the reset ctrlr sequence into the nvme_ctrlr->thread. Signed-off-by: Shuhei Matsumoto Change-Id: I538b962e2a7b5cf00ebbac2a1e888482ddeeee61 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10075 Community-CI: Broadcom CI Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 40 +++++++++++-------- .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 28 ++++++------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 802b0830e..f6395c14d 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -515,10 +515,8 @@ nvme_ctrlr_unregister_cb(void *io_device) } static void -nvme_ctrlr_unregister(void *ctx) +nvme_ctrlr_unregister(struct nvme_ctrlr *nvme_ctrlr) { - struct nvme_ctrlr *nvme_ctrlr = ctx; - spdk_io_device_unregister(nvme_ctrlr, nvme_ctrlr_unregister_cb); } @@ -1217,6 +1215,8 @@ _bdev_nvme_reset_complete(struct spdk_io_channel_iter *i, int status) void *reset_cb_arg = nvme_ctrlr->reset_cb_arg; bool complete_pending_destruct = false; + assert(nvme_ctrlr->thread == spdk_get_thread()); + nvme_ctrlr->reset_cb_fn = NULL; nvme_ctrlr->reset_cb_arg = NULL; @@ -1249,8 +1249,7 @@ _bdev_nvme_reset_complete(struct spdk_io_channel_iter *i, int status) } if (complete_pending_destruct) { - spdk_thread_send_msg(nvme_ctrlr->thread, nvme_ctrlr_unregister, - nvme_ctrlr); + nvme_ctrlr_unregister(nvme_ctrlr); } } @@ -1343,6 +1342,23 @@ bdev_nvme_reset_destroy_qpair(struct spdk_io_channel_iter *i) spdk_for_each_channel_continue(i, 0); } +static void +_bdev_nvme_reset(void *ctx) +{ + struct nvme_ctrlr *nvme_ctrlr = ctx; + + assert(nvme_ctrlr->resetting == true); + assert(nvme_ctrlr->thread == spdk_get_thread()); + + spdk_nvme_ctrlr_prepare_for_reset(nvme_ctrlr->ctrlr); + + /* First, delete all NVMe I/O queue pairs. */ + spdk_for_each_channel(nvme_ctrlr, + bdev_nvme_reset_destroy_qpair, + NULL, + bdev_nvme_reset_ctrlr); +} + static int bdev_nvme_reset(struct nvme_ctrlr *nvme_ctrlr) { @@ -1360,14 +1376,8 @@ bdev_nvme_reset(struct nvme_ctrlr *nvme_ctrlr) nvme_ctrlr->resetting = true; pthread_mutex_unlock(&nvme_ctrlr->mutex); - spdk_nvme_ctrlr_prepare_for_reset(nvme_ctrlr->ctrlr); - - /* First, delete all NVMe I/O queue pairs. */ - spdk_for_each_channel(nvme_ctrlr, - bdev_nvme_reset_destroy_qpair, - NULL, - bdev_nvme_reset_ctrlr); + spdk_thread_send_msg(nvme_ctrlr->thread, _bdev_nvme_reset, nvme_ctrlr); return 0; } @@ -1563,11 +1573,7 @@ bdev_nvme_failover(struct nvme_ctrlr *nvme_ctrlr, bool remove) rc = bdev_nvme_failover_start(nvme_ctrlr, remove); if (rc == 0) { - /* First, delete all NVMe I/O queue pairs. */ - spdk_for_each_channel(nvme_ctrlr, - bdev_nvme_reset_destroy_qpair, - NULL, - bdev_nvme_reset_ctrlr); + spdk_thread_send_msg(nvme_ctrlr->thread, _bdev_nvme_reset, nvme_ctrlr); } else if (rc != -EALREADY) { return rc; } 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 b4e6c3a39..c049193e0 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 @@ -1290,7 +1290,7 @@ test_reset_ctrlr(void) CU_ASSERT(ctrlr_ch1->qpair != NULL); CU_ASSERT(ctrlr_ch2->qpair != NULL); - poll_thread_times(0, 1); + poll_thread_times(0, 3); CU_ASSERT(ctrlr_ch1->qpair == NULL); CU_ASSERT(ctrlr_ch2->qpair != NULL); @@ -1299,7 +1299,7 @@ test_reset_ctrlr(void) CU_ASSERT(ctrlr_ch2->qpair == NULL); CU_ASSERT(ctrlr.is_failed == true); - poll_thread_times(1, 1); + poll_thread_times(0, 1); CU_ASSERT(ctrlr.is_failed == false); poll_thread_times(0, 1); @@ -1312,13 +1312,11 @@ test_reset_ctrlr(void) CU_ASSERT(nvme_ctrlr->resetting == true); CU_ASSERT(curr_trid->is_failed == true); + poll_thread_times(0, 2); + CU_ASSERT(nvme_ctrlr->resetting == true); poll_thread_times(1, 1); CU_ASSERT(nvme_ctrlr->resetting == true); poll_thread_times(0, 1); - CU_ASSERT(nvme_ctrlr->resetting == true); - poll_thread_times(1, 1); - CU_ASSERT(nvme_ctrlr->resetting == true); - poll_thread_times(1, 1); CU_ASSERT(nvme_ctrlr->resetting == false); CU_ASSERT(curr_trid->is_failed == false); @@ -2829,13 +2827,13 @@ test_reconnect_qpair(void) CU_ASSERT(ctrlr_ch2->qpair == NULL); CU_ASSERT(nvme_ctrlr->resetting == true); - poll_thread_times(0, 1); + poll_thread_times(0, 2); poll_thread_times(1, 1); CU_ASSERT(ctrlr_ch1->qpair == NULL); CU_ASSERT(ctrlr_ch2->qpair == NULL); CU_ASSERT(ctrlr->is_failed == true); - poll_thread_times(1, 1); + poll_thread_times(0, 1); CU_ASSERT(ctrlr->is_failed == false); poll_thread_times(0, 1); @@ -2844,10 +2842,9 @@ test_reconnect_qpair(void) CU_ASSERT(ctrlr_ch2->qpair != NULL); CU_ASSERT(nvme_ctrlr->resetting == true); + poll_thread_times(0, 2); poll_thread_times(1, 1); poll_thread_times(0, 1); - poll_thread_times(1, 1); - poll_thread_times(1, 1); CU_ASSERT(nvme_ctrlr->resetting == false); poll_threads(); @@ -2864,16 +2861,15 @@ test_reconnect_qpair(void) CU_ASSERT(ctrlr_ch2->qpair == NULL); CU_ASSERT(nvme_ctrlr->resetting == true); - poll_thread_times(0, 1); + poll_thread_times(0, 2); poll_thread_times(1, 1); CU_ASSERT(ctrlr_ch1->qpair == NULL); CU_ASSERT(ctrlr_ch2->qpair == NULL); CU_ASSERT(ctrlr->is_failed == true); + poll_thread_times(0, 2); poll_thread_times(1, 1); poll_thread_times(0, 1); - poll_thread_times(1, 1); - poll_thread_times(1, 1); CU_ASSERT(ctrlr->is_failed == true); CU_ASSERT(nvme_ctrlr->resetting == false); CU_ASSERT(ctrlr_ch1->qpair == NULL); @@ -3654,7 +3650,7 @@ test_reset_bdev_ctrlr(void) CU_ASSERT(nvme_ctrlr1->resetting == true); CU_ASSERT(nvme_ctrlr1->reset_cb_arg == first_bio); - poll_thread_times(0, 1); + poll_thread_times(0, 2); CU_ASSERT(io_path11->ctrlr_ch->qpair == NULL); CU_ASSERT(io_path21->ctrlr_ch->qpair != NULL); @@ -3686,7 +3682,7 @@ test_reset_bdev_ctrlr(void) CU_ASSERT(first_bio->io_path == io_path12); CU_ASSERT(nvme_ctrlr2->resetting == true); - poll_thread_times(0, 1); + poll_thread_times(0, 2); CU_ASSERT(io_path12->ctrlr_ch->qpair == NULL); CU_ASSERT(io_path22->ctrlr_ch->qpair != NULL); @@ -3927,7 +3923,7 @@ test_retry_io_if_ctrlr_is_resetting(void) ctrlr_ch->qpair->is_connected = false; ctrlr->is_failed = true; - poll_thread_times(0, 3); + poll_thread_times(0, 4); CU_ASSERT(ctrlr_ch->qpair == NULL); CU_ASSERT(nvme_ctrlr->resetting == true);