Commit c4e81408 authored by Pawel Baldysiak's avatar Pawel Baldysiak Committed by Tomasz Zawadzki
Browse files

nvmf: Fix counting oustanding IOs for passthru subsystem 



Nvmf is counting in-flight IOs in per-subsys array,
where NSID-1 is used as index.

Susbystem in passthru mode overwrites NSID of IO command
with NSID of underlying device.

Currently - if susbsytem has passthru enabled -
IO counter is increased for original NSID, but since
the completion is returned with underlying NSID,
it decresed for the wrong one.
It happens if nvmf's NSID is not matching underlying bdev's NSID.
For example - if there are two NS in one passthru subsys,
or single NS is added with not matching NSID.

Add recovering of original NSID to the request in the completion path,
so the in-flight IO counter is inc/dec for same NSID.
Add assert to make spoting such issue easier in debug build.

Change-Id: Ia007ead694592b62af9e60b5065e103806355721
Signed-off-by: default avatarPawel Baldysiak <pawel.baldysiak@dell.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/25832


Reviewed-by: default avatarAnkit Kumar <ankit.kumar@samsung.com>
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Reviewed-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-by: default avatarChangpeng Liu <changpeliu@tencent.com>
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
parent 1abef745

lib/nvmf/ctrlr.c

+13 −2
Original line number Diff line number Diff line
@@ -4585,6 +4585,7 @@ nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) 


	if (ctrlr->subsys->passthrough) {

		assert(ns->passthru_nsid > 0);

		req->orig_nsid = req->cmd->nvme_cmd.nsid;

		req->cmd->nvme_cmd.nsid = ns->passthru_nsid;



		return nvmf_bdev_ctrlr_nvme_passthru_io(bdev, desc, ch, req);
@@ -4635,6 +4636,7 @@ nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) 
				goto invalid_opcode;

			}

			if (ns->passthru_nsid) {

				req->orig_nsid = req->cmd->nvme_cmd.nsid;

				req->cmd->nvme_cmd.nsid = ns->passthru_nsid;

			}

			return nvmf_bdev_ctrlr_nvme_passthru_io(bdev, desc, ch, req);
@@ -4691,10 +4693,9 @@ _nvmf_request_complete(void *ctx) 
	rsp->sqid = 0;

	rsp->status.p = 0;

	rsp->cid = req->cmd->nvme_cmd.cid;

	nsid = req->cmd->nvme_cmd.nsid;

	opcode = req->cmd->nvmf_cmd.opcode;



	qpair = req->qpair;



	if (spdk_likely(qpair->ctrlr)) {

		sgroup = &qpair->group->sgroups[qpair->ctrlr->subsys->id];

		assert(sgroup != NULL);
@@ -4703,6 +4704,13 @@ _nvmf_request_complete(void *ctx) 
			qpair->group->stat.completed_nvme_io++;

		}



		/* If we changed nvme_cmd.nsid to match the passthrough nsid, we need to

		 * restore it here for accounting purposes.

		 */

		if (qpair->ctrlr->subsys->passthrough && req->orig_nsid) {

			req->cmd->nvme_cmd.nsid = req->orig_nsid;

		}



		/*

		 * Set the crd value.

		 * If the the IO has any error, and dnr (DoNotRetry) is not 1,
@@ -4717,6 +4725,8 @@ _nvmf_request_complete(void *ctx) 
		sgroup = nvmf_subsystem_pg_from_connect_cmd(req);

	}



	nsid = req->cmd->nvme_cmd.nsid;



	if (SPDK_DEBUGLOG_FLAG_ENABLED("nvmf")) {

		spdk_nvme_print_completion(qpair->qid, rsp);

	}
@@ -4762,6 +4772,7 @@ _nvmf_request_complete(void *ctx) 


				/* NOTE: This implicitly also checks for 0, since 0 - 1 wraps around to UINT32_MAX. */

				if (spdk_likely(nsid - 1 < sgroup->num_ns)) {

					assert(sgroup->ns_info[nsid - 1].io_outstanding != 0);

					sgroup->ns_info[nsid - 1].io_outstanding--;

				}

			}