Commit 99d2e2ac authored by Ben Walker's avatar Ben Walker Committed by Konrad Sztyber
Browse files

bdev/nvme: Replace spdk_bdev_io module_link with per-io ctx retry_link 



We're removing module_link from spdk_bdev_io, so add a list element to
this module's per-io area.

Change-Id: Icd679e509d25e7bd1eb10d663a4e21b945e391f9
Signed-off-by: default avatarBen Walker <ben@nvidia.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24363


Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Community-CI: Mellanox Build Bot
parent dcd73c6c

module/bdev/nvme/bdev_nvme.c

+26 −37
Original line number Diff line number Diff line
@@ -97,6 +97,9 @@ struct nvme_bdev_io { 


	/* Current tsc at submit time. */

	uint64_t submit_tsc;



	/* Used to put nvme_bdev_io into the list */

	TAILQ_ENTRY(nvme_bdev_io) retry_link;

};



struct nvme_probe_skip_entry {
@@ -697,11 +700,9 @@ static void 
bdev_nvme_clear_retry_io_path(struct nvme_bdev_channel *nbdev_ch,

			      struct nvme_io_path *io_path)

{

	struct spdk_bdev_io *bdev_io;

	struct nvme_bdev_io *bio;



	TAILQ_FOREACH(bdev_io, &nbdev_ch->retry_io_list, module_link) {

		bio = (struct nvme_bdev_io *)bdev_io->driver_ctx;

	TAILQ_FOREACH(bio, &nbdev_ch->retry_io_list, retry_link) {

		if (bio->io_path == io_path) {

			bio->io_path = NULL;

		}
@@ -1119,29 +1120,25 @@ static int 
bdev_nvme_retry_ios(void *arg)

{

	struct nvme_bdev_channel *nbdev_ch = arg;

	struct spdk_bdev_io *bdev_io, *tmp_bdev_io;

	struct nvme_bdev_io *bio;

	struct nvme_bdev_io *bio, *tmp_bio;

	uint64_t now, delay_us;



	now = spdk_get_ticks();



	TAILQ_FOREACH_SAFE(bdev_io, &nbdev_ch->retry_io_list, module_link, tmp_bdev_io) {

		bio = (struct nvme_bdev_io *)bdev_io->driver_ctx;

	TAILQ_FOREACH_SAFE(bio, &nbdev_ch->retry_io_list, retry_link, tmp_bio) {

		if (bio->retry_ticks > now) {

			break;

		}



		TAILQ_REMOVE(&nbdev_ch->retry_io_list, bdev_io, module_link);

		TAILQ_REMOVE(&nbdev_ch->retry_io_list, bio, retry_link);



		bdev_nvme_retry_io(nbdev_ch, bdev_io);

		bdev_nvme_retry_io(nbdev_ch, spdk_bdev_io_from_ctx(bio));

	}



	spdk_poller_unregister(&nbdev_ch->retry_io_poller);



	bdev_io = TAILQ_FIRST(&nbdev_ch->retry_io_list);

	if (bdev_io != NULL) {

		bio = (struct nvme_bdev_io *)bdev_io->driver_ctx;



	bio = TAILQ_FIRST(&nbdev_ch->retry_io_list);

	if (bio != NULL) {

		delay_us = (bio->retry_ticks - now) * SPDK_SEC_TO_USEC / spdk_get_ticks_hz();



		nbdev_ch->retry_io_poller = SPDK_POLLER_REGISTER(bdev_nvme_retry_ios, nbdev_ch,
@@ -1155,24 +1152,20 @@ static void 
bdev_nvme_queue_retry_io(struct nvme_bdev_channel *nbdev_ch,

			 struct nvme_bdev_io *bio, uint64_t delay_ms)

{

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);

	struct spdk_bdev_io *tmp_bdev_io;

	struct nvme_bdev_io *tmp_bio;



	bio->retry_ticks = spdk_get_ticks() + delay_ms * spdk_get_ticks_hz() / 1000ULL;



	TAILQ_FOREACH_REVERSE(tmp_bdev_io, &nbdev_ch->retry_io_list, retry_io_head, module_link) {

		tmp_bio = (struct nvme_bdev_io *)tmp_bdev_io->driver_ctx;



	TAILQ_FOREACH_REVERSE(tmp_bio, &nbdev_ch->retry_io_list, retry_io_head, retry_link) {

		if (tmp_bio->retry_ticks <= bio->retry_ticks) {

			TAILQ_INSERT_AFTER(&nbdev_ch->retry_io_list, tmp_bdev_io, bdev_io,

					   module_link);

			TAILQ_INSERT_AFTER(&nbdev_ch->retry_io_list, tmp_bio, bio,

					   retry_link);

			return;

		}

	}



	/* No earlier I/Os were found. This I/O must be the new head. */

	TAILQ_INSERT_HEAD(&nbdev_ch->retry_io_list, bdev_io, module_link);

	TAILQ_INSERT_HEAD(&nbdev_ch->retry_io_list, bio, retry_link);



	spdk_poller_unregister(&nbdev_ch->retry_io_poller);
@@ -1183,11 +1176,11 @@ bdev_nvme_queue_retry_io(struct nvme_bdev_channel *nbdev_ch, 
static void

bdev_nvme_abort_retry_ios(struct nvme_bdev_channel *nbdev_ch)

{

	struct spdk_bdev_io *bdev_io, *tmp_io;

	struct nvme_bdev_io *bio, *tmp_bio;



	TAILQ_FOREACH_SAFE(bdev_io, &nbdev_ch->retry_io_list, module_link, tmp_io) {

		TAILQ_REMOVE(&nbdev_ch->retry_io_list, bdev_io, module_link);

		__bdev_nvme_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_ABORTED, NULL);

	TAILQ_FOREACH_SAFE(bio, &nbdev_ch->retry_io_list, retry_link, tmp_bio) {

		TAILQ_REMOVE(&nbdev_ch->retry_io_list, bio, retry_link);

		__bdev_nvme_io_complete(spdk_bdev_io_from_ctx(bio), SPDK_BDEV_IO_STATUS_ABORTED, NULL);

	}



	spdk_poller_unregister(&nbdev_ch->retry_io_poller);
@@ -1197,12 +1190,12 @@ static int 
bdev_nvme_abort_retry_io(struct nvme_bdev_channel *nbdev_ch,

			 struct nvme_bdev_io *bio_to_abort)

{

	struct spdk_bdev_io *bdev_io_to_abort;

	struct nvme_bdev_io *bio;



	TAILQ_FOREACH(bdev_io_to_abort, &nbdev_ch->retry_io_list, module_link) {

		if ((struct nvme_bdev_io *)bdev_io_to_abort->driver_ctx == bio_to_abort) {

			TAILQ_REMOVE(&nbdev_ch->retry_io_list, bdev_io_to_abort, module_link);

			__bdev_nvme_io_complete(bdev_io_to_abort, SPDK_BDEV_IO_STATUS_ABORTED, NULL);

	TAILQ_FOREACH(bio, &nbdev_ch->retry_io_list, retry_link) {

		if (bio == bio_to_abort) {

			TAILQ_REMOVE(&nbdev_ch->retry_io_list, bio, retry_link);

			__bdev_nvme_io_complete(spdk_bdev_io_from_ctx(bio), SPDK_BDEV_IO_STATUS_ABORTED, NULL);

			return 0;

		}

	}
@@ -1813,7 +1806,6 @@ bdev_nvme_complete_pending_resets(struct spdk_io_channel_iter *i) 
	struct spdk_io_channel *_ch = spdk_io_channel_iter_get_channel(i);

	struct nvme_ctrlr_channel *ctrlr_ch = spdk_io_channel_get_ctx(_ch);

	int rc = 0;

	struct spdk_bdev_io *bdev_io;

	struct nvme_bdev_io *bio;



	if (spdk_io_channel_iter_get_ctx(i) != NULL) {
@@ -1821,10 +1813,9 @@ bdev_nvme_complete_pending_resets(struct spdk_io_channel_iter *i) 
	}



	while (!TAILQ_EMPTY(&ctrlr_ch->pending_resets)) {

		bdev_io = TAILQ_FIRST(&ctrlr_ch->pending_resets);

		TAILQ_REMOVE(&ctrlr_ch->pending_resets, bdev_io, module_link);

		bio = TAILQ_FIRST(&ctrlr_ch->pending_resets);

		TAILQ_REMOVE(&ctrlr_ch->pending_resets, bio, retry_link);



		bio = (struct nvme_bdev_io *)bdev_io->driver_ctx;

		bdev_nvme_reset_io_continue(bio, rc);

	}
@@ -2832,7 +2823,6 @@ static int 
_bdev_nvme_reset_io(struct nvme_io_path *io_path, struct nvme_bdev_io *bio)

{

	struct nvme_ctrlr_channel *ctrlr_ch;

	struct spdk_bdev_io *bdev_io;

	int rc;



	rc = nvme_ctrlr_op(io_path->qpair->ctrlr, NVME_CTRLR_OP_RESET,
@@ -2852,8 +2842,7 @@ _bdev_nvme_reset_io(struct nvme_io_path *io_path, struct nvme_bdev_io *bio) 
		 * we don't interfere with the app framework reset strategy. i.e. we are deferring to the

		 * upper level. If they are in the middle of a reset, we won't try to schedule another one.

		 */

		bdev_io = spdk_bdev_io_from_ctx(bio);

		TAILQ_INSERT_TAIL(&ctrlr_ch->pending_resets, bdev_io, module_link);

		TAILQ_INSERT_TAIL(&ctrlr_ch->pending_resets, bio, retry_link);

	}



	return 0;

module/bdev/nvme/bdev_nvme.h

+2 −2
Original line number Diff line number Diff line
@@ -204,7 +204,7 @@ struct nvme_qpair { 


struct nvme_ctrlr_channel {

	struct nvme_qpair		*qpair;

	TAILQ_HEAD(, spdk_bdev_io)	pending_resets;

	TAILQ_HEAD(, nvme_bdev_io)	pending_resets;



	struct spdk_io_channel_iter	*reset_iter;

	struct spdk_poller		*connect_poller;
@@ -230,7 +230,7 @@ struct nvme_bdev_channel { 
	uint32_t				rr_min_io;

	uint32_t				rr_counter;

	STAILQ_HEAD(, nvme_io_path)		io_path_list;

	TAILQ_HEAD(retry_io_head, spdk_bdev_io)	retry_io_list;

	TAILQ_HEAD(retry_io_head, nvme_bdev_io)	retry_io_list;

	struct spdk_poller			*retry_io_poller;

};

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

+22 −19
Original line number Diff line number Diff line
@@ -1968,7 +1968,7 @@ test_pending_reset(void) 
	set_thread(0);



	bdev_nvme_submit_request(ch1, second_bdev_io);

	CU_ASSERT(TAILQ_FIRST(&ctrlr_ch1->pending_resets) == second_bdev_io);

	CU_ASSERT(spdk_bdev_io_from_ctx(TAILQ_FIRST(&ctrlr_ch1->pending_resets)) == second_bdev_io);



	poll_threads();

	spdk_delay_us(g_opts.nvme_adminq_poll_period_us);
@@ -1993,7 +1993,7 @@ test_pending_reset(void) 
	set_thread(0);



	bdev_nvme_submit_request(ch1, second_bdev_io);

	CU_ASSERT(TAILQ_FIRST(&ctrlr_ch1->pending_resets) == second_bdev_io);

	CU_ASSERT(spdk_bdev_io_from_ctx(TAILQ_FIRST(&ctrlr_ch1->pending_resets)) == second_bdev_io);



	ctrlr->fail_reset = true;
@@ -2856,7 +2856,7 @@ test_abort(void) 
	bdev_nvme_submit_request(ch1, write_io);



	CU_ASSERT(write_io->internal.in_submit_request == true);

	CU_ASSERT(write_io == TAILQ_FIRST(&nbdev_ch1->retry_io_list));

	CU_ASSERT(write_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch1->retry_io_list)));



	/* Aborting the queued write request should succeed immediately. */

	abort_io->internal.ch = (struct spdk_bdev_channel *)ch1;
@@ -4237,7 +4237,8 @@ test_reset_bdev_ctrlr(void) 


	CU_ASSERT(nvme_ctrlr1->resetting == true);

	CU_ASSERT(nvme_ctrlr1->ctrlr_op_cb_arg == first_bio);

	CU_ASSERT(TAILQ_FIRST(&io_path21->qpair->ctrlr_ch->pending_resets) == second_bdev_io);

	CU_ASSERT(TAILQ_FIRST(&io_path21->qpair->ctrlr_ch->pending_resets) ==

		  (struct nvme_bdev_io *)second_bdev_io->driver_ctx);



	poll_threads();

	spdk_delay_us(g_opts.nvme_adminq_poll_period_us);
@@ -4433,7 +4434,7 @@ test_retry_io_if_ana_state_is_updating(void) 


	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io1->internal.in_submit_request == true);

	CU_ASSERT(bdev_io1 == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io1 == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	/* ANA state became accessible while I/O was queued. */

	nvme_ns->ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE;
@@ -4587,7 +4588,7 @@ test_retry_io_for_io_path_error(void) 


	CU_ASSERT(nvme_qpair1->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	poll_threads();
@@ -4648,7 +4649,7 @@ test_retry_io_for_io_path_error(void) 
	CU_ASSERT(nvme_qpair1->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(nvme_qpair2->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	spdk_nvme_ctrlr_free_io_qpair(nvme_qpair1->qpair);

	nvme_qpair1->qpair = NULL;
@@ -4815,7 +4816,7 @@ test_retry_io_count(void) 


	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	poll_threads();
@@ -4846,7 +4847,7 @@ test_retry_io_count(void) 


	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	poll_threads();
@@ -5077,7 +5078,7 @@ test_retry_io_for_ana_error(void) 


	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));

	/* I/O should be retried immediately. */

	CU_ASSERT(bio->retry_ticks == now);

	CU_ASSERT(nvme_ns->ana_state_updating == true);
@@ -5088,7 +5089,7 @@ test_retry_io_for_ana_error(void) 
	/* Namespace is inaccessible, and hence I/O should be queued again. */

	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));

	/* I/O should be retried after a second if no I/O path was found but

	 * any I/O path may become available.

	 */
@@ -5266,8 +5267,10 @@ test_retry_io_if_ctrlr_is_resetting(void) 


	CU_ASSERT(bdev_io1->internal.in_submit_request == true);

	CU_ASSERT(bdev_io2->internal.in_submit_request == true);

	CU_ASSERT(bdev_io1 == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io2 == TAILQ_NEXT(bdev_io1, module_link));

	CU_ASSERT(bdev_io1 == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));

	CU_ASSERT(bdev_io2 == spdk_bdev_io_from_ctx(

			  TAILQ_NEXT((struct nvme_bdev_io *)bdev_io1->driver_ctx,

				     retry_link)));



	poll_threads();

	spdk_delay_us(g_opts.nvme_adminq_poll_period_us);
@@ -5283,7 +5286,7 @@ test_retry_io_if_ctrlr_is_resetting(void) 
	CU_ASSERT(nvme_qpair->qpair->num_outstanding_reqs == 1);

	CU_ASSERT(bdev_io1->internal.in_submit_request == true);

	CU_ASSERT(bdev_io2->internal.in_submit_request == true);

	CU_ASSERT(bdev_io2 == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io2 == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	poll_threads();
@@ -5291,7 +5294,7 @@ test_retry_io_if_ctrlr_is_resetting(void) 
	CU_ASSERT(bdev_io1->internal.in_submit_request == false);

	CU_ASSERT(bdev_io1->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS);

	CU_ASSERT(bdev_io2->internal.in_submit_request == true);

	CU_ASSERT(bdev_io2 == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io2 == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	spdk_delay_us(1);
@@ -5719,7 +5722,7 @@ test_fail_path(void) 
	bdev_nvme_submit_request(ch, bdev_io);



	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	/* After a second, the I/O should be still queued and the ctrlr should be

	 * still recovering.
@@ -5728,7 +5731,7 @@ test_fail_path(void) 
	poll_threads();



	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	CU_ASSERT(nvme_ctrlr->resetting == false);

	CU_ASSERT(ctrlr->is_failed == false);
@@ -6593,7 +6596,7 @@ test_retry_io_to_same_path(void) 


	CU_ASSERT(io_path2->qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	/* The 2nd I/O should keep caching io_path2. */

	CU_ASSERT(bio->io_path == io_path2);
@@ -6621,7 +6624,7 @@ test_retry_io_to_same_path(void) 


	CU_ASSERT(io_path2->qpair->qpair->num_outstanding_reqs == 0);

	CU_ASSERT(bdev_io->internal.in_submit_request == true);

	CU_ASSERT(bdev_io == TAILQ_FIRST(&nbdev_ch->retry_io_list));

	CU_ASSERT(bdev_io == spdk_bdev_io_from_ctx(TAILQ_FIRST(&nbdev_ch->retry_io_list)));



	/* The 2nd I/O should keep caching io_path2. */

	CU_ASSERT(bio->io_path == io_path2);