Commit b10fbdf5 authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

nvme: store fabrics connect data ptr in status structure



Since the connect will be completed asynchronously, we
need to keep the pointer around so we can access (and
free it!) later when the command completes.

Also change the code to poll on the status using the
new nvme_wait_for_completion_poll(), as prep for upcoming
patches.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I28add8f967fd000afed1e50e491a16ea9da16c22
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8603


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent b2bc01cd
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -244,6 +244,7 @@ nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl)

	if (status->timed_out) {
		/* There is no routine waiting for the completion of this request, free allocated memory */
		spdk_free(status->dma_data);
		free(status);
		return;
	}
+15 −6
Original line number Diff line number Diff line
@@ -416,6 +416,8 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries)
		return -ENOMEM;
	}

	status->dma_data = nvmf_data;

	memset(&cmd, 0, sizeof(cmd));
	cmd.opcode = SPDK_NVME_OPC_FABRIC;
	cmd.fctype = SPDK_NVMF_FABRIC_COMMAND_CONNECT;
@@ -441,16 +443,23 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries)
					nvme_completion_poll_cb, status);
	if (rc < 0) {
		SPDK_ERRLOG("Failed to allocate/submit FABRIC_CONNECT command, rc %d\n", rc);
		spdk_free(nvmf_data);
		spdk_free(status->dma_data);
		free(status);
		return rc;
	}

	/* If we time out, the qpair will abort the request upon destruction. */
	rc = nvme_wait_for_completion_timeout(qpair, status, ctrlr->opts.fabrics_connect_timeout_us);
	if (rc) {
	if (ctrlr->opts.fabrics_connect_timeout_us > 0) {
		status->timeout_tsc = spdk_get_ticks() + ctrlr->opts.fabrics_connect_timeout_us *
				      spdk_get_ticks_hz() / SPDK_SEC_TO_USEC;
	}

	/* Wait until the command completes or times out */
	while (nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL) == -EAGAIN) {}

	if (status->timed_out || spdk_nvme_cpl_is_error(&status->cpl)) {
		SPDK_ERRLOG("Connect command failed, rc %d, trtype:%s adrfam:%s traddr:%s trsvcid:%s subnqn:%s\n",
			    rc,
			    status->timed_out ? -ECANCELED : -EIO,
			    spdk_nvme_transport_id_trtype_str(ctrlr->trid.trtype),
			    spdk_nvme_transport_id_adrfam_str(ctrlr->trid.adrfam),
			    ctrlr->trid.traddr,
@@ -460,8 +469,8 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries)
			SPDK_ERRLOG("Connect command completed with error: sct %d, sc %d\n", status->cpl.status.sct,
				    status->cpl.status.sc);
		}
		spdk_free(nvmf_data);
		if (!status->timed_out) {
			spdk_free(status->dma_data);
			free(status);
		}
		return -EIO;
@@ -473,7 +482,7 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries)
		SPDK_DEBUGLOG(nvme, "CNTLID 0x%04" PRIx16 "\n", ctrlr->cntlid);
	}

	spdk_free(nvmf_data);
	spdk_free(status->dma_data);
	free(status);
	return 0;
}
+5 −0
Original line number Diff line number Diff line
@@ -383,6 +383,11 @@ struct nvme_request {
struct nvme_completion_poll_status {
	struct spdk_nvme_cpl	cpl;
	uint64_t		timeout_tsc;
	/**
	 * DMA buffer retained throughout the duration of the command.  It'll be released
	 * automatically if the command times out, otherwise the user is responsible for freeing it.
	 */
	void			*dma_data;
	bool			done;
	/* This flag indicates that the request has been timed out and the memory
	   must be freed in a completion callback */
+37 −10
Original line number Diff line number Diff line
@@ -42,8 +42,6 @@ pid_t g_spdk_nvme_pid;
struct spdk_nvmf_fabric_prop_set_cmd g_ut_cmd = {};
struct spdk_nvmf_fabric_prop_get_rsp g_ut_response = {};

DEFINE_STUB_V(nvme_completion_poll_cb, (void *arg, const struct spdk_nvme_cpl *cpl));

DEFINE_STUB_V(spdk_nvme_ctrlr_get_default_ctrlr_opts,
	      (struct spdk_nvme_ctrlr_opts *opts, size_t opts_size));

@@ -82,6 +80,7 @@ DEFINE_STUB(spdk_nvme_transport_id_adrfam_str, const char *,
DEFINE_STUB(nvme_ctrlr_process_init, int, (struct spdk_nvme_ctrlr *ctrlr), 0);

static struct spdk_nvmf_fabric_connect_data g_nvmf_data;
static struct nvme_request *g_request;

int
spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr,
@@ -100,15 +99,30 @@ spdk_nvme_ctrlr_cmd_io_raw(struct spdk_nvme_ctrlr *ctrlr,

	memcpy(&req->cmd, cmd, sizeof(req->cmd));
	memcpy(&g_nvmf_data, buf, sizeof(g_nvmf_data));
	g_request = req;

	return 0;
}

DEFINE_RETURN_MOCK(nvme_wait_for_completion_timeout, int);
void
nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl)
{
	struct nvme_completion_poll_status *status = arg;

	if (status->timed_out) {
		spdk_free(status->dma_data);
		free(status);
	}

	g_request = NULL;
}

static bool g_nvme_wait_for_completion_timeout;

int
nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair,
nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair,
		struct nvme_completion_poll_status *status,
				 uint64_t timeout_in_usecs)
		pthread_mutex_t *robust_mutex)
{
	struct spdk_nvmf_fabric_connect_rsp *rsp = (void *)&status->cpl;

@@ -116,8 +130,7 @@ nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair,
		rsp->status_code_specific.success.cntlid = 1;
	}

	status->timed_out = false;
	HANDLE_RETURN_MOCK(nvme_wait_for_completion_timeout);
	status->timed_out = g_nvme_wait_for_completion_timeout;

	return 0;
}
@@ -218,6 +231,19 @@ spdk_nvme_ctrlr_cmd_admin_raw(struct spdk_nvme_ctrlr *ctrlr,
	return 0;
}

static void
abort_request(struct nvme_request *request)
{
	struct spdk_nvme_cpl cpl = {
		.status = {
			.sct = SPDK_NVME_SCT_GENERIC,
			.sc = SPDK_NVME_SC_ABORTED_SQ_DELETION,
		}
	};

	request->cb_fn(request->cb_arg, &cpl);
}

static void
test_nvme_fabric_prop_set_cmd(void)
{
@@ -403,11 +429,12 @@ test_nvme_fabric_qpair_connect(void)

	/* Wait_for completion timeout */
	STAILQ_INSERT_HEAD(&qpair.free_req, &req, stailq);
	MOCK_SET(nvme_wait_for_completion_timeout, 1);
	g_nvme_wait_for_completion_timeout = true;

	rc = nvme_fabric_qpair_connect(&qpair, 1);
	CU_ASSERT(rc == -EIO);
	MOCK_CLEAR(nvme_wait_for_completion_timeout);
	g_nvme_wait_for_completion_timeout = false;
	abort_request(g_request);

	/* Input parameters invalid */
	rc = nvme_fabric_qpair_connect(&qpair, 0);