Commit 696ad465 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Remove the failover_in_progress flag from struct nvme_ctrlr 



The failover_in_progress flag is used to decide the return value of
bdev_nvme_failover().

bdev_nvme_delete() calls bdev_nvme_failover() with remove=true to remove
nvme_ctrlr->active_path_id. However bdev_nvme_failover() returns zero
if nvme_ctrlr->failover_in_progress is true. bdev_nvme_failover() may
return zero even if it does not remove nvme_ctrlr->active_path_id.

The following will be better.

bdev_nvme_failover() returns -EBUSY if nvme_ctrlr->resetting is true,
and the caller repeats calling bdev_nvme_failover() until the target trid
becomes alternative path or bdev_nvme_failover() returns zero.

To do that, the failover_in_progress flag is not necessary any more.

Removing the failover_in_progress will also simplify the following
patches to unify ctrlr reset and failover.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I57ab944beb1d06ea4def144c81c69705860de35f
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10441


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 74f18d6a

module/bdev/nvme/bdev_nvme.c

+3 −12
Original line number Diff line number Diff line
@@ -1283,7 +1283,6 @@ _bdev_nvme_reset_complete(struct spdk_io_channel_iter *i, int status) 


	pthread_mutex_lock(&nvme_ctrlr->mutex);

	nvme_ctrlr->resetting = false;

	nvme_ctrlr->failover_in_progress = false;



	path_id = TAILQ_FIRST(&nvme_ctrlr->trids);

	assert(path_id != NULL);
@@ -1580,7 +1579,7 @@ static int 
bdev_nvme_failover_start(struct nvme_ctrlr *nvme_ctrlr, bool remove)

{

	struct nvme_path_id *path_id = NULL, *next_path = NULL;

	int rc;

	int rc __attribute__((unused));



	pthread_mutex_lock(&nvme_ctrlr->mutex);

	if (nvme_ctrlr->destruct) {
@@ -1595,14 +1594,9 @@ bdev_nvme_failover_start(struct nvme_ctrlr *nvme_ctrlr, bool remove) 
	next_path = TAILQ_NEXT(path_id, link);



	if (nvme_ctrlr->resetting) {

		if (next_path && !nvme_ctrlr->failover_in_progress) {

			rc = -EBUSY;

		} else {

			rc = -EALREADY;

		}

		pthread_mutex_unlock(&nvme_ctrlr->mutex);

		SPDK_NOTICELOG("Unable to perform reset, already in progress.\n");

		return rc;

		return -EBUSY;

	}



	nvme_ctrlr->resetting = true;
@@ -1614,7 +1608,6 @@ bdev_nvme_failover_start(struct nvme_ctrlr *nvme_ctrlr, bool remove) 
		SPDK_NOTICELOG("Start failover from %s:%s to %s:%s\n", path_id->trid.traddr,

			       path_id->trid.trsvcid,	next_path->trid.traddr, next_path->trid.trsvcid);



		nvme_ctrlr->failover_in_progress = true;

		spdk_nvme_ctrlr_fail(nvme_ctrlr->ctrlr);

		nvme_ctrlr->active_path_id = next_path;

		rc = spdk_nvme_ctrlr_set_trid(nvme_ctrlr->ctrlr, &next_path->trid);
@@ -1642,11 +1635,9 @@ bdev_nvme_failover(struct nvme_ctrlr *nvme_ctrlr, bool remove) 
	rc = bdev_nvme_failover_start(nvme_ctrlr, remove);

	if (rc == 0) {

		spdk_thread_send_msg(nvme_ctrlr->thread, _bdev_nvme_reset, nvme_ctrlr);

	} else if (rc != -EALREADY) {

		return rc;

	}



	return 0;

	return rc;

}



static int bdev_nvme_unmap(struct nvme_bdev_io *bio, uint64_t offset_blocks,

module/bdev/nvme/bdev_nvme.h

+0 −1
Original line number Diff line number Diff line
@@ -104,7 +104,6 @@ struct nvme_ctrlr { 
	int					ref;



	uint32_t				resetting : 1;

	uint32_t				failover_in_progress : 1;

	uint32_t				destruct : 1;

	uint32_t				ana_log_page_updating : 1;

	/**

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

+4 −21
Original line number Diff line number Diff line
@@ -1451,18 +1451,10 @@ test_failover_ctrlr(void) 
	nvme_ctrlr->resetting = true;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == 0);



	/* Case 3: failover is in progress. */

	nvme_ctrlr->failover_in_progress = true;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == 0);

	CU_ASSERT(curr_trid->is_failed == false);

	CU_ASSERT(rc == -EBUSY);



	/* Case 4: reset completes successfully. */

	/* Case 3: reset completes successfully. */

	nvme_ctrlr->resetting = false;

	nvme_ctrlr->failover_in_progress = false;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == 0);
@@ -1492,27 +1484,19 @@ test_failover_ctrlr(void) 
	/* Failover starts from thread 1. */

	set_thread(1);



	/* Case 5: reset is in progress. */

	/* Case 4: reset is in progress. */

	nvme_ctrlr->resetting = true;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == -EBUSY);



	/* Case 5: failover is in progress. */

	nvme_ctrlr->failover_in_progress = true;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == 0);



	/* Case 6: failover completes successfully. */

	/* Case 5: failover completes successfully. */

	nvme_ctrlr->resetting = false;

	nvme_ctrlr->failover_in_progress = false;



	rc = bdev_nvme_failover(nvme_ctrlr, false);

	CU_ASSERT(rc == 0);



	CU_ASSERT(nvme_ctrlr->resetting == true);

	CU_ASSERT(nvme_ctrlr->failover_in_progress == true);



	next_trid = TAILQ_FIRST(&nvme_ctrlr->trids);

	SPDK_CU_ASSERT_FATAL(next_trid != NULL);
@@ -1523,7 +1507,6 @@ test_failover_ctrlr(void) 
	poll_threads();



	CU_ASSERT(nvme_ctrlr->resetting == false);

	CU_ASSERT(nvme_ctrlr->failover_in_progress == false);



	spdk_put_io_channel(ch2);