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

bdev/nvme: Remove remove parameter from bdev_nvme_failover_ctrlr() 



The remove parameter of bdev_nvme_failover_ctrlr() is always set to
false. _bdev_nvme_delete() sets the remove parameter to true but it
calls bdev_nvme_failover_ctrlr_unsafe().

For clarification and simplification, remove the remove parameter from
bdev_nvme_failover_ctrlr().

Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: I5373ba0bb1aeac896579d2effbec7a410782d7a1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/20701


Reviewed-by: default avatarRichael <richael.zhuang@arm.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarMateusz Kozlowski <mateusz.kozlowski@solidigm.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Community-CI: Mellanox Build Bot
parent a2c540e3

module/bdev/nvme/bdev_nvme.c

+6 −6
Original line number Diff line number Diff line
@@ -190,7 +190,7 @@ static void bdev_nvme_abort(struct nvme_bdev_channel *nbdev_ch, 
			    struct nvme_bdev_io *bio, struct nvme_bdev_io *bio_to_abort);

static void bdev_nvme_reset_io(struct nvme_bdev_channel *nbdev_ch, struct nvme_bdev_io *bio);

static int bdev_nvme_reset_ctrlr(struct nvme_ctrlr *nvme_ctrlr);

static int bdev_nvme_failover_ctrlr(struct nvme_ctrlr *nvme_ctrlr, bool remove);

static int bdev_nvme_failover_ctrlr(struct nvme_ctrlr *nvme_ctrlr);

static void remove_cb(void *cb_ctx, struct spdk_nvme_ctrlr *ctrlr);

static int nvme_ctrlr_read_ana_log_page(struct nvme_ctrlr *nvme_ctrlr);
@@ -1577,7 +1577,7 @@ bdev_nvme_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_ 
		} else {

			/* qpair was disconnected unexpectedly. Reset controller for recovery. */

			SPDK_NOTICELOG("qpair %p was disconnected and freed. reset controller.\n", qpair);

			bdev_nvme_failover_ctrlr(nvme_qpair->ctrlr, false);

			bdev_nvme_failover_ctrlr(nvme_qpair->ctrlr);

		}

	} else {

		/* In this case, ctrlr_channel is already deleted. */
@@ -1664,7 +1664,7 @@ bdev_nvme_poll_adminq(void *arg) 
							    g_opts.nvme_adminq_poll_period_us);

			disconnected_cb(nvme_ctrlr);

		} else {

			bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

			bdev_nvme_failover_ctrlr(nvme_ctrlr);

		}

	} else if (spdk_nvme_ctrlr_get_admin_qp_failure_reason(nvme_ctrlr->ctrlr) !=

		   SPDK_NVME_QPAIR_FAILURE_NONE) {
@@ -2053,7 +2053,7 @@ _bdev_nvme_reset_ctrlr_complete(struct spdk_io_channel_iter *i, int status) 
		nvme_ctrlr_disconnect(nvme_ctrlr, bdev_nvme_start_reconnect_delay_timer);

		break;

	case OP_FAILOVER:

		bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

		bdev_nvme_failover_ctrlr(nvme_ctrlr);

		break;

	default:

		break;
@@ -2851,12 +2851,12 @@ bdev_nvme_failover_ctrlr_unsafe(struct nvme_ctrlr *nvme_ctrlr, bool remove) 
}



static int

bdev_nvme_failover_ctrlr(struct nvme_ctrlr *nvme_ctrlr, bool remove)

bdev_nvme_failover_ctrlr(struct nvme_ctrlr *nvme_ctrlr)

{

	int rc;



	pthread_mutex_lock(&nvme_ctrlr->mutex);

	rc = bdev_nvme_failover_ctrlr_unsafe(nvme_ctrlr, remove);

	rc = bdev_nvme_failover_ctrlr_unsafe(nvme_ctrlr, false);

	pthread_mutex_unlock(&nvme_ctrlr->mutex);



	if (rc == 0) {

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

+8 −8
Original line number Diff line number Diff line
@@ -1656,7 +1656,7 @@ test_failover_ctrlr(void) 
	/* Case 1: ctrlr is already being destructed. */

	nvme_ctrlr->destruct = true;



	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == -ENXIO);

	CU_ASSERT(curr_trid->last_failed_tsc == 0);
@@ -1664,13 +1664,13 @@ test_failover_ctrlr(void) 
	nvme_ctrlr->destruct = false;

	nvme_ctrlr->resetting = true;



	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == -EINPROGRESS);



	/* Case 3: reset completes successfully. */

	nvme_ctrlr->resetting = false;



	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == 0);



	CU_ASSERT(nvme_ctrlr->resetting == true);
@@ -1703,13 +1703,13 @@ test_failover_ctrlr(void) 
	/* Case 4: reset is in progress. */

	nvme_ctrlr->resetting = true;



	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == -EINPROGRESS);



	/* Case 5: failover completes successfully. */

	nvme_ctrlr->resetting = false;



	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == 0);



	CU_ASSERT(nvme_ctrlr->resetting == true);
@@ -5498,8 +5498,8 @@ test_retry_failover_ctrlr(void) 
	/* If we remove trid1 while reconnect is scheduled, trid1 is removed and path_id is

	 * switched to trid2 but reset is not started.

	 */

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, true);

	CU_ASSERT(rc == 0);

	rc = bdev_nvme_failover_ctrlr_unsafe(nvme_ctrlr, true);

	CU_ASSERT(rc == -EALREADY);



	CU_ASSERT(ut_get_path_id_by_trid(nvme_ctrlr, &trid1) == NULL);

	CU_ASSERT(path_id2 == nvme_ctrlr->active_path_id);
@@ -6683,7 +6683,7 @@ test_race_between_reset_and_disconnected(void) 
	 *

	 * Simulate fabric connect command timeout by calling bdev_nvme_failover_ctrlr().

	 */

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr, false);

	rc = bdev_nvme_failover_ctrlr(nvme_ctrlr);

	CU_ASSERT(rc == -EINPROGRESS);

	CU_ASSERT(nvme_ctrlr->resetting == true);

	CU_ASSERT(nvme_ctrlr->pending_failover == true);