From 26c5cc5259eee76b4da3b8fe34090aba4a8f68e2 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Fri, 19 Mar 2021 06:51:01 +0900 Subject: [PATCH] bdev/nvme: Remove ctx parameter from _bdev_nvme_reset() Separate bdev_nvme_reset() and _bdev_nvme_reset() by making bdev_nvme_reset() call _bdev_nvme_reset_start(), and then remove the ctx parameter from _bdev_nvme_reset(). This clarifies the next patch and reduces the size of the next patch. Signed-off-by: Shuhei Matsumoto Change-Id: I76b0f2f5b83445845a313203e594dca0be150bc3 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6949 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 24 ++++++++++++------- .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 10 ++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index eb90954f1..fbff171f4 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -531,7 +531,7 @@ _bdev_nvme_reset_start(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr) } static int -_bdev_nvme_reset(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, void *ctx) +_bdev_nvme_reset(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr) { int rc; @@ -540,7 +540,7 @@ _bdev_nvme_reset(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, void *ctx) /* First, delete all NVMe I/O queue pairs. */ spdk_for_each_channel(nvme_bdev_ctrlr, _bdev_nvme_reset_destroy_qpair, - ctx, + NULL, _bdev_nvme_reset_ctrlr); } @@ -553,11 +553,16 @@ bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio) struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio); int rc; - rc = _bdev_nvme_reset(nvme_ch->ctrlr, bio); - if (rc == -EBUSY) { + rc = _bdev_nvme_reset_start(nvme_ch->ctrlr); + if (rc == 0) { + /* First, delete all NVMe I/O queue pairs. */ + spdk_for_each_channel(nvme_ch->ctrlr, + _bdev_nvme_reset_destroy_qpair, + bio, + _bdev_nvme_reset_ctrlr); + } else if (rc == -EBUSY) { /* Don't bother resetting if the controller is in the process of being destructed. */ spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_FAILED); - return 0; } else if (rc == -EAGAIN) { /* * Reset call is queued only if it is from the app framework. This is on purpose so that @@ -565,10 +570,11 @@ bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio) * upper level. If they are in the middle of a reset, we won't try to schedule another one. */ TAILQ_INSERT_TAIL(&nvme_ch->pending_resets, bdev_io, module_link); - return 0; } else { return rc; } + + return 0; } static int @@ -1316,7 +1322,7 @@ nvme_abort_cpl(void *ctx, const struct spdk_nvme_cpl *cpl) if (spdk_nvme_cpl_is_error(cpl)) { SPDK_WARNLOG("Abort failed. Resetting controller.\n"); - _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + _bdev_nvme_reset(nvme_bdev_ctrlr); } } @@ -1341,7 +1347,7 @@ timeout_cb(void *cb_arg, struct spdk_nvme_ctrlr *ctrlr, csts = spdk_nvme_ctrlr_get_regs_csts(ctrlr); if (csts.bits.cfs) { SPDK_ERRLOG("Controller Fatal Status, reset required\n"); - _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + _bdev_nvme_reset(nvme_bdev_ctrlr); return; } } @@ -1360,7 +1366,7 @@ timeout_cb(void *cb_arg, struct spdk_nvme_ctrlr *ctrlr, /* FALLTHROUGH */ case SPDK_BDEV_NVME_TIMEOUT_ACTION_RESET: - _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + _bdev_nvme_reset(nvme_bdev_ctrlr); break; case SPDK_BDEV_NVME_TIMEOUT_ACTION_NONE: SPDK_DEBUGLOG(bdev_nvme, "No action for nvme controller timeout.\n"); 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 aba5c414d..01ea80bb9 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 @@ -944,14 +944,14 @@ test_reset_ctrlr(void) /* Case 1: ctrlr is already being destructed. */ nvme_bdev_ctrlr->destruct = true; - rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + rc = _bdev_nvme_reset(nvme_bdev_ctrlr); CU_ASSERT(rc == -EBUSY); /* Case 2: reset is in progress. */ nvme_bdev_ctrlr->destruct = false; nvme_bdev_ctrlr->resetting = true; - rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + rc = _bdev_nvme_reset(nvme_bdev_ctrlr); CU_ASSERT(rc == -EAGAIN); /* Case 3: reset completes successfully. */ @@ -959,7 +959,7 @@ test_reset_ctrlr(void) curr_trid->is_failed = true; ctrlr.is_failed = true; - rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + rc = _bdev_nvme_reset(nvme_bdev_ctrlr); CU_ASSERT(rc == 0); CU_ASSERT(nvme_bdev_ctrlr->resetting == true); CU_ASSERT(nvme_ch1->qpair != NULL); @@ -1036,7 +1036,7 @@ test_race_between_reset_and_destruct_ctrlr(void) /* Reset starts from thread 1. */ set_thread(1); - rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + rc = _bdev_nvme_reset(nvme_bdev_ctrlr); CU_ASSERT(rc == 0); CU_ASSERT(nvme_bdev_ctrlr->resetting == true); @@ -1057,7 +1057,7 @@ test_race_between_reset_and_destruct_ctrlr(void) CU_ASSERT(nvme_bdev_ctrlr->resetting == false); /* New reset request is rejected. */ - rc = _bdev_nvme_reset(nvme_bdev_ctrlr, NULL); + rc = _bdev_nvme_reset(nvme_bdev_ctrlr); CU_ASSERT(rc == -EBUSY); /* Additional polling called spdk_io_device_unregister() to ctrlr,