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

bdev/nvme: Reduce conversion between spdk_bdev_io and nvme_bdev_io 



We can hold bdev_io directly in nvme_bdev_ctrlr as an outstanding reset.

We can put spdk_bdev_io_from_ctx(bio) into a parameter for a few
functions because it is used only once in a function.

Passing not spdk_bdev_io but nvme_bdev_io to bdev_nvme_verify_pi_error()
remove unnecessary substitution.

This is a little more efficient and simplifies the implementation.

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


Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent f70180d5

module/bdev/nvme/bdev_nvme.c

+21 −27
Original line number Diff line number Diff line
@@ -187,7 +187,7 @@ static int bdev_nvme_io_passthru_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qp 
				    struct spdk_nvme_cmd *cmd, void *buf, size_t nbytes, void *md_buf, size_t md_len);

static int bdev_nvme_abort(struct nvme_io_channel *nvme_ch,

			   struct nvme_bdev_io *bio, struct nvme_bdev_io *bio_to_abort);

static int bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio);

static int bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct spdk_bdev_io *bdev_io);

static int bdev_nvme_failover(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, bool remove);

static void remove_cb(void *cb_ctx, struct spdk_nvme_ctrlr *ctrlr);
@@ -255,16 +255,13 @@ static inline void 
bdev_nvme_io_complete_nvme_status(struct nvme_bdev_io *bio,

				  const struct spdk_nvme_cpl *cpl)

{

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);



	spdk_bdev_io_complete_nvme_status(bdev_io, cpl->cdw0, cpl->status.sct,

					  cpl->status.sc);

	spdk_bdev_io_complete_nvme_status(spdk_bdev_io_from_ctx(bio), cpl->cdw0,

					  cpl->status.sct, cpl->status.sc);

}



static inline void

bdev_nvme_io_complete(struct nvme_bdev_io *bio, int rc)

{

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);

	enum spdk_bdev_io_status io_status;



	if (rc == 0) {
@@ -275,7 +272,7 @@ bdev_nvme_io_complete(struct nvme_bdev_io *bio, int rc) 
		io_status = SPDK_BDEV_IO_STATUS_FAILED;

	}



	spdk_bdev_io_complete(bdev_io, io_status);

	spdk_bdev_io_complete(spdk_bdev_io_from_ctx(bio), io_status);

}



static void
@@ -489,7 +486,7 @@ bdev_nvme_abort_pending_resets(struct spdk_io_channel_iter *i) 


static void

bdev_nvme_reset_io_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr,

			    struct nvme_bdev_io *bio, int rc)

			    struct spdk_bdev_io *bdev_io, int rc)

{

	enum spdk_bdev_io_status io_status = SPDK_BDEV_IO_STATUS_SUCCESS;
@@ -497,7 +494,7 @@ bdev_nvme_reset_io_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, 
		io_status = SPDK_BDEV_IO_STATUS_FAILED;

	}



	spdk_bdev_io_complete(spdk_bdev_io_from_ctx(bio), io_status);

	spdk_bdev_io_complete(bdev_io, io_status);



	/* Make sure we clear any pending resets before returning. */

	spdk_for_each_channel(nvme_bdev_ctrlr,
@@ -511,9 +508,9 @@ static void 
_bdev_nvme_reset_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, int rc)

{

	struct nvme_bdev_ctrlr_trid *curr_trid;

	struct nvme_bdev_io *bio = nvme_bdev_ctrlr->reset_bio;

	struct spdk_bdev_io *bdev_io = nvme_bdev_ctrlr->reset_bdev_io;



	nvme_bdev_ctrlr->reset_bio = NULL;

	nvme_bdev_ctrlr->reset_bdev_io = NULL;



	if (rc) {

		SPDK_ERRLOG("Resetting controller failed.\n");
@@ -538,8 +535,8 @@ _bdev_nvme_reset_complete(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr, int rc) 


	pthread_mutex_unlock(&nvme_bdev_ctrlr->mutex);



	if (bio) {

		bdev_nvme_reset_io_complete(nvme_bdev_ctrlr, bio, rc);

	if (bdev_io) {

		bdev_nvme_reset_io_complete(nvme_bdev_ctrlr, bdev_io, rc);

	} else {

		/* Make sure we clear any pending resets before returning. */

		spdk_for_each_channel(nvme_bdev_ctrlr,
@@ -637,15 +634,14 @@ _bdev_nvme_reset(struct nvme_bdev_ctrlr *nvme_bdev_ctrlr) 
}



static int

bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio)

bdev_nvme_reset(struct nvme_io_channel *nvme_ch, struct spdk_bdev_io *bdev_io)

{

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);

	int rc;



	rc = _bdev_nvme_reset(nvme_ch->ctrlr);

	if (rc == 0) {

		assert(nvme_ch->ctrlr->reset_bio == NULL);

		nvme_ch->ctrlr->reset_bio = bio;

		assert(nvme_ch->ctrlr->reset_bdev_io == NULL);

		nvme_ch->ctrlr->reset_bdev_io = bdev_io;

	} else if (rc == -EAGAIN) {

		/*

		 * Reset call is queued only if it is from the app framework. This is on purpose so that
@@ -767,7 +763,7 @@ bdev_nvme_get_buf_cb(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io, 


	ret = bdev_nvme_readv(nvme_ns->ns,

			      qpair,

			      (struct nvme_bdev_io *)bdev_io->driver_ctx,

			      bio,

			      bdev_io->u.bdev.iovs,

			      bdev_io->u.bdev.iovcnt,

			      bdev_io->u.bdev.md_buf,
@@ -859,7 +855,7 @@ bdev_nvme_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_i 
				     bdev_io->u.bdev.num_blocks);

		break;

	case SPDK_BDEV_IO_TYPE_RESET:

		rc = bdev_nvme_reset(nvme_ch, nbdev_io);

		rc = bdev_nvme_reset(nvme_ch, bdev_io);

		break;

	case SPDK_BDEV_IO_TYPE_FLUSH:

		rc = bdev_nvme_flush(nvme_ns->ns,
@@ -2508,8 +2504,9 @@ bdev_nvme_library_fini(void) 
}



static void

bdev_nvme_verify_pi_error(struct spdk_bdev_io *bdev_io)

bdev_nvme_verify_pi_error(struct nvme_bdev_io *bio)

{

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);

	struct spdk_bdev *bdev = bdev_io->bdev;

	struct spdk_dif_ctx dif_ctx;

	struct spdk_dif_error err_blk = {};
@@ -2549,11 +2546,10 @@ static void 
bdev_nvme_no_pi_readv_done(void *ref, const struct spdk_nvme_cpl *cpl)

{

	struct nvme_bdev_io *bio = ref;

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);



	if (spdk_nvme_cpl_is_success(cpl)) {

		/* Run PI verification for read data buffer. */

		bdev_nvme_verify_pi_error(bdev_io);

		bdev_nvme_verify_pi_error(bio);

	}



	/* Return original completion status */
@@ -2603,13 +2599,12 @@ static void 
bdev_nvme_writev_done(void *ref, const struct spdk_nvme_cpl *cpl)

{

	struct nvme_bdev_io *bio = ref;

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);



	if (spdk_nvme_cpl_is_pi_error(cpl)) {

		SPDK_ERRLOG("writev completed with PI error (sct=%d, sc=%d)\n",

			    cpl->status.sct, cpl->status.sc);

		/* Run PI verification for write data buffer if PI error is detected. */

		bdev_nvme_verify_pi_error(bdev_io);

		bdev_nvme_verify_pi_error(bio);

	}



	bdev_nvme_io_complete_nvme_status(bio, cpl);
@@ -2630,7 +2625,7 @@ bdev_nvme_zone_appendv_done(void *ref, const struct spdk_nvme_cpl *cpl) 
		SPDK_ERRLOG("zone append completed with PI error (sct=%d, sc=%d)\n",

			    cpl->status.sct, cpl->status.sc);

		/* Run PI verification for zone append data buffer if PI error is detected. */

		bdev_nvme_verify_pi_error(bdev_io);

		bdev_nvme_verify_pi_error(bio);

	}



	bdev_nvme_io_complete_nvme_status(bio, cpl);
@@ -2640,13 +2635,12 @@ static void 
bdev_nvme_comparev_done(void *ref, const struct spdk_nvme_cpl *cpl)

{

	struct nvme_bdev_io *bio = ref;

	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);



	if (spdk_nvme_cpl_is_pi_error(cpl)) {

		SPDK_ERRLOG("comparev completed with PI error (sct=%d, sc=%d)\n",

			    cpl->status.sct, cpl->status.sc);

		/* Run PI verification for compare data buffer if PI error is detected. */

		bdev_nvme_verify_pi_error(bdev_io);

		bdev_nvme_verify_pi_error(bio);

	}



	bdev_nvme_io_complete_nvme_status(bio, cpl);

module/bdev/nvme/common.h

+1 −1
Original line number Diff line number Diff line
@@ -107,7 +107,7 @@ struct nvme_bdev_ctrlr { 


	struct ocssd_bdev_ctrlr			*ocssd_ctrlr;



	struct nvme_bdev_io			*reset_bio;

	struct spdk_bdev_io			*reset_bdev_io;



	/** linked list pointer for device list */

	TAILQ_ENTRY(nvme_bdev_ctrlr)		tailq;

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

+4 −7
Original line number Diff line number Diff line
@@ -1343,7 +1343,6 @@ test_pending_reset(void) 
	const int STRING_SIZE = 32;

	const char *attached_names[STRING_SIZE];

	struct spdk_bdev_io *first_bdev_io, *second_bdev_io;

	struct nvme_bdev_io *first_bio, *second_bio;

	struct spdk_io_channel *ch1, *ch2;

	struct nvme_io_channel *nvme_ch1, *nvme_ch2;

	int rc;
@@ -1354,12 +1353,10 @@ test_pending_reset(void) 
	first_bdev_io = calloc(1, sizeof(struct spdk_bdev_io) + sizeof(struct nvme_bdev_io));

	SPDK_CU_ASSERT_FATAL(first_bdev_io != NULL);

	first_bdev_io->internal.status = SPDK_BDEV_IO_STATUS_FAILED;

	first_bio = (struct nvme_bdev_io *)first_bdev_io->driver_ctx;



	second_bdev_io = calloc(1, sizeof(struct spdk_bdev_io) + sizeof(struct nvme_bdev_io));

	SPDK_CU_ASSERT_FATAL(second_bdev_io != NULL);

	second_bdev_io->internal.status = SPDK_BDEV_IO_STATUS_FAILED;

	second_bio = (struct nvme_bdev_io *)second_bdev_io->driver_ctx;



	set_thread(0);
@@ -1394,14 +1391,14 @@ test_pending_reset(void) 
	/* The first reset request is submitted on thread 1, and the second reset request

	 * is submitted on thread 0 while processing the first request.

	 */

	rc = bdev_nvme_reset(nvme_ch2, first_bio);

	rc = bdev_nvme_reset(nvme_ch2, first_bdev_io);

	CU_ASSERT(rc == 0);

	CU_ASSERT(nvme_bdev_ctrlr->resetting == true);

	CU_ASSERT(TAILQ_EMPTY(&nvme_ch2->pending_resets));



	set_thread(0);



	rc = bdev_nvme_reset(nvme_ch1, second_bio);

	rc = bdev_nvme_reset(nvme_ch1, second_bdev_io);

	CU_ASSERT(rc == 0);

	CU_ASSERT(TAILQ_FIRST(&nvme_ch1->pending_resets) == second_bdev_io);
@@ -1419,14 +1416,14 @@ test_pending_reset(void) 
	 */

	set_thread(1);



	rc = bdev_nvme_reset(nvme_ch2, first_bio);

	rc = bdev_nvme_reset(nvme_ch2, first_bdev_io);

	CU_ASSERT(rc == 0);

	CU_ASSERT(nvme_bdev_ctrlr->resetting == true);

	CU_ASSERT(TAILQ_EMPTY(&nvme_ch2->pending_resets));



	set_thread(0);



	rc = bdev_nvme_reset(nvme_ch1, second_bio);

	rc = bdev_nvme_reset(nvme_ch1, second_bdev_io);

	CU_ASSERT(rc == 0);

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