From 2ee6ab36f9a0e38f0e47e9dab3db40a6ea72cfd5 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 21 Jul 2021 00:36:17 +0900 Subject: [PATCH] bdev/nvme: bdev_nvme_reset() follow spdk_nvme_ctrlr_reset() about return value Previously bdev_nvme_reset() returned -EBUSY if ctrlr is being destructed and returned -EAGAIN if ctrlr is being reset. These did not match what spdk_nvme_ctrlr_reset() returned. Reset operation will be more important than current when multipath is supported and reset operation is made asynchronous. Hence change bdev_nvme_reset() to follow spdk_nvme_ctrlr_reset(). bdev_nvme_reset() returns -ENXIO if ctrlr is being destructed and returns -EBUSY if ctrlr is being reset. Additionally change the return value of bdev_nvme_failover() accordingly. After the change bdev_nvme_failover() returns -ENXIO if being destructed and returns -EBUSY if ctrlr is being reset. Signed-off-by: Shuhei Matsumoto Change-Id: Ie2c6f8601050b1043d83de9cf01490751784e4e5 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8859 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Konrad Sztyber Reviewed-by: Paul Luse Reviewed-by: Ben Walker Reviewed-by: Krzysztof Karas Reviewed-by: Aleksey Marchuk Reviewed-by: Jim Harris --- module/bdev/nvme/bdev_nvme.c | 14 +++++++------- module/bdev/nvme/bdev_nvme.h | 4 ++-- test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 90e7953e1..aef522983 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -856,13 +856,13 @@ bdev_nvme_reset(struct nvme_ctrlr *nvme_ctrlr) pthread_mutex_lock(&nvme_ctrlr->mutex); if (nvme_ctrlr->destruct) { pthread_mutex_unlock(&nvme_ctrlr->mutex); - return -EBUSY; + return -ENXIO; } if (nvme_ctrlr->resetting) { pthread_mutex_unlock(&nvme_ctrlr->mutex); SPDK_NOTICELOG("Unable to perform reset, already in progress.\n"); - return -EAGAIN; + return -EBUSY; } nvme_ctrlr->resetting = true; @@ -912,7 +912,7 @@ bdev_nvme_reset_io(struct nvme_bdev_channel *nbdev_ch, struct nvme_bdev_io *bio) assert(ctrlr_ch->ctrlr->reset_cb_arg == NULL); ctrlr_ch->ctrlr->reset_cb_fn = bdev_nvme_reset_io_complete; ctrlr_ch->ctrlr->reset_cb_arg = bio; - } else if (rc == -EAGAIN) { + } else if (rc == -EBUSY) { /* * Reset call is queued only if it is from the app framework. This is on purpose so that * we don't interfere with the app framework reset strategy. i.e. we are deferring to the @@ -937,7 +937,7 @@ bdev_nvme_failover_start(struct nvme_ctrlr *nvme_ctrlr, bool remove) if (nvme_ctrlr->destruct) { pthread_mutex_unlock(&nvme_ctrlr->mutex); /* Don't bother resetting if the controller is in the process of being destructed. */ - return -EBUSY; + return -ENXIO; } curr_trid = TAILQ_FIRST(&nvme_ctrlr->trids); @@ -947,9 +947,9 @@ bdev_nvme_failover_start(struct nvme_ctrlr *nvme_ctrlr, bool remove) if (nvme_ctrlr->resetting) { if (next_trid && !nvme_ctrlr->failover_in_progress) { - rc = -EAGAIN; - } else { rc = -EBUSY; + } else { + rc = -EALREADY; } pthread_mutex_unlock(&nvme_ctrlr->mutex); SPDK_NOTICELOG("Unable to perform reset, already in progress.\n"); @@ -997,7 +997,7 @@ bdev_nvme_failover(struct nvme_ctrlr *nvme_ctrlr, bool remove) bdev_nvme_reset_destroy_qpair, NULL, bdev_nvme_reset_ctrlr); - } else if (rc != -EBUSY) { + } else if (rc != -EALREADY) { return rc; } diff --git a/module/bdev/nvme/bdev_nvme.h b/module/bdev/nvme/bdev_nvme.h index 450155863..d5f3398aa 100644 --- a/module/bdev/nvme/bdev_nvme.h +++ b/module/bdev/nvme/bdev_nvme.h @@ -229,8 +229,8 @@ int bdev_nvme_delete(const char *name, const struct spdk_nvme_transport_id *trid * \param cb_fn Function to be called back after reset completes * \param cb_arg Argument for callback function * \return zero on success. Negated errno on the following error conditions: - * -EBUSY: controller is being destroyed. - * -EAGAIN: controller is already being reset. + * -ENXIO: controller is being destroyed. + * -EBUSY: controller is already being reset. */ int bdev_nvme_reset_rpc(struct nvme_ctrlr *nvme_ctrlr, bdev_nvme_reset_cb cb_fn, void *cb_arg); 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 c583f57e6..246dfce99 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 @@ -1221,14 +1221,14 @@ test_reset_ctrlr(void) nvme_ctrlr->destruct = true; rc = bdev_nvme_reset(nvme_ctrlr); - CU_ASSERT(rc == -EBUSY); + CU_ASSERT(rc == -ENXIO); /* Case 2: reset is in progress. */ nvme_ctrlr->destruct = false; nvme_ctrlr->resetting = true; rc = bdev_nvme_reset(nvme_ctrlr); - CU_ASSERT(rc == -EAGAIN); + CU_ASSERT(rc == -EBUSY); /* Case 3: reset completes successfully. */ nvme_ctrlr->resetting = false; @@ -1336,7 +1336,7 @@ test_race_between_reset_and_destruct_ctrlr(void) /* New reset request is rejected. */ rc = bdev_nvme_reset(nvme_ctrlr); - CU_ASSERT(rc == -EBUSY); + CU_ASSERT(rc == -ENXIO); /* Additional polling called spdk_io_device_unregister() to ctrlr, * However there are two channels and destruct is not completed yet. @@ -1399,7 +1399,7 @@ test_failover_ctrlr(void) nvme_ctrlr->destruct = true; rc = bdev_nvme_failover(nvme_ctrlr, false); - CU_ASSERT(rc == 0); + CU_ASSERT(rc == -ENXIO); CU_ASSERT(curr_trid->is_failed == false); /* Case 2: reset is in progress. */ @@ -1452,7 +1452,7 @@ test_failover_ctrlr(void) nvme_ctrlr->resetting = true; rc = bdev_nvme_failover(nvme_ctrlr, false); - CU_ASSERT(rc == -EAGAIN); + CU_ASSERT(rc == -EBUSY); /* Case 5: failover is in progress. */ nvme_ctrlr->failover_in_progress = true;