Commit 2ee6ab36 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

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: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ie2c6f8601050b1043d83de9cf01490751784e4e5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8859


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@gmail.com>
Reviewed-by: default avatarPaul Luse <paul.e.luse@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarKrzysztof Karas <krzysztof.karas@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 6d2caa65

module/bdev/nvme/bdev_nvme.c

+7 −7
Original line number Diff line number Diff line
@@ -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;

	}

module/bdev/nvme/bdev_nvme.h

+2 −2
Original line number Diff line number Diff line
@@ -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);

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+5 −5
Original line number Diff line number Diff line
@@ -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;