Commit 1dd74730 authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvme: create tracker pool in a single allocation



Replace the previous code that allocated each tracker individually with
one large allocation per queue pair.

struct nvme_tracker is now explicitly padded to reach exactly 4096 bytes
to allow normal array indexing to work correctly while maintaining the
alignment requirement that ensures each tracker's PRP list does not
cross a page boundary.

This also allows removal of the act_tr array, since the tr array can be
indexed directly now, and each tracker can store its own active state.

Change-Id: Ia7c51735b96594d12f7f478cefcc4aedc84207ad
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 3272320c
Loading
Loading
Loading
Loading
+16 −5
Original line number Diff line number Diff line
@@ -245,17 +245,25 @@ struct nvme_tracker {
	struct nvme_request		*req;
	uint16_t			cid;

	uint16_t			rsvd1: 15;
	uint16_t			active: 1;

	uint32_t			rsvd2;

	uint64_t			prp_sgl_bus_addr;

	union {
		uint64_t			prp[NVME_MAX_PRP_LIST_ENTRIES];
		struct spdk_nvme_sgl_descriptor	sgl[NVME_MAX_SGL_DESCRIPTORS];
	} u;

	uint64_t			rsvd3;
};
/*
 * struct nvme_tracker must fit in 4K (min page size) so that the alignment math in
 * nvme_qpair_construct() works correctly to ensure prp[] does not cross a page boundary.
 * struct nvme_tracker must be exactly 4K so that the prp[] array does not cross a page boundary
 * and so that there is no padding required to meet alignment requirements.
 */
SPDK_STATIC_ASSERT(sizeof(struct nvme_tracker) <= 4096, "nvme_tracker too large");
SPDK_STATIC_ASSERT(sizeof(struct nvme_tracker) == 4096, "nvme_tracker is not 4K");


struct spdk_nvme_qpair {
@@ -275,9 +283,12 @@ struct spdk_nvme_qpair {
	LIST_HEAD(, nvme_tracker)	free_tr;
	LIST_HEAD(, nvme_tracker)	outstanding_tr;

	STAILQ_HEAD(, nvme_request)	queued_req;
	/**
	 * Array of trackers indexed by command ID.
	 */
	struct nvme_tracker		*tr;

	struct nvme_tracker		**act_tr;
	STAILQ_HEAD(, nvme_request)	queued_req;

	uint16_t			id;

+21 −29
Original line number Diff line number Diff line
@@ -291,6 +291,7 @@ nvme_qpair_construct_tracker(struct nvme_tracker *tr, uint16_t cid, uint64_t phy
{
	tr->prp_sgl_bus_addr = phys_addr + offsetof(struct nvme_tracker, u.prp);
	tr->cid = cid;
	tr->active = false;
}

static void
@@ -299,7 +300,7 @@ nvme_qpair_submit_tracker(struct spdk_nvme_qpair *qpair, struct nvme_tracker *tr
	struct nvme_request	*req;

	req = tr->req;
	qpair->act_tr[tr->cid] = tr;
	qpair->tr[tr->cid].active = true;

	/* Copy the command from the tracker to the submission queue. */
	nvme_copy_command(&qpair->cmd[qpair->sq_tail], &req->cmd);
@@ -332,7 +333,7 @@ nvme_qpair_complete_tracker(struct spdk_nvme_qpair *qpair, struct nvme_tracker *
		nvme_qpair_print_completion(qpair, cpl);
	}

	qpair->act_tr[cpl->cid] = NULL;
	qpair->tr[cpl->cid].active = false;

	nvme_assert(cpl->cid == req->cmd.cid, ("cpl cid does not match cmd cid\n"));

@@ -493,9 +494,9 @@ spdk_nvme_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_
		if (cpl->status.p != qpair->phase)
			break;

		tr = qpair->act_tr[cpl->cid];
		tr = &qpair->tr[cpl->cid];

		if (tr != NULL) {
		if (tr->active) {
			nvme_qpair_complete_tracker(qpair, tr, cpl, true);
		} else {
			nvme_printf(qpair->ctrlr,
@@ -566,26 +567,25 @@ nvme_qpair_construct(struct spdk_nvme_qpair *qpair, uint16_t id,
	LIST_INIT(&qpair->outstanding_tr);
	STAILQ_INIT(&qpair->queued_req);

	for (i = 0; i < num_trackers; i++) {
	/*
		 * Round alignment up to next power of 2.  This ensures the PRP
		 *  list embedded in the nvme_tracker object will not span a
		 *  4KB boundary.
	 * Reserve space for all of the trackers in a single allocation.
	 *   struct nvme_tracker must be padded so that its size is already a power of 2.
	 *   This ensures the PRP list embedded in the nvme_tracker object will not span a
	 *   4KB boundary, while allowing access to trackers in tr[] via normal array indexing.
	 */
		tr = nvme_malloc("nvme_tr", sizeof(*tr), nvme_align32pow2(sizeof(*tr)), &phys_addr);
		if (tr == NULL) {
	qpair->tr = nvme_malloc("nvme_tr", num_trackers * sizeof(*tr), sizeof(*tr), &phys_addr);
	if (qpair->tr == NULL) {
		nvme_printf(ctrlr, "nvme_tr failed\n");
		goto fail;
	}

	for (i = 0; i < num_trackers; i++) {
		tr = &qpair->tr[i];
		nvme_qpair_construct_tracker(tr, i, phys_addr);
		LIST_INSERT_HEAD(&qpair->free_tr, tr, list);
		phys_addr += sizeof(struct nvme_tracker);
	}

	qpair->act_tr = calloc(num_trackers, sizeof(struct nvme_tracker *));
	if (qpair->act_tr == NULL) {
		nvme_printf(ctrlr, "alloc nvme_act_tr failed\n");
		goto fail;
	}
	nvme_qpair_reset(qpair);
	return 0;
fail:
@@ -621,8 +621,6 @@ _nvme_admin_qpair_destroy(struct spdk_nvme_qpair *qpair)
void
nvme_qpair_destroy(struct spdk_nvme_qpair *qpair)
{
	struct nvme_tracker	*tr;

	if (nvme_qpair_is_admin_queue(qpair)) {
		_nvme_admin_qpair_destroy(qpair);
	}
@@ -630,14 +628,8 @@ nvme_qpair_destroy(struct spdk_nvme_qpair *qpair)
		nvme_free(qpair->cmd);
	if (qpair->cpl)
		nvme_free(qpair->cpl);
	if (qpair->act_tr)
		free(qpair->act_tr);

	while (!LIST_EMPTY(&qpair->free_tr)) {
		tr = LIST_FIRST(&qpair->free_tr);
		LIST_REMOVE(tr, list);
		nvme_free(tr);
	}
	if (qpair->tr)
		nvme_free(qpair->tr);
}

/**
+10 −15
Original line number Diff line number Diff line
@@ -251,18 +251,17 @@ ut_insert_cq_entry(struct spdk_nvme_qpair *qpair, uint32_t slot)
	nvme_alloc_request(&req);
	SPDK_CU_ASSERT_FATAL(req != NULL);
	memset(req, 0, sizeof(*req));
	req->cmd.cid = slot;

	tr = LIST_FIRST(&qpair->free_tr);
	LIST_REMOVE(tr, list); /* remove tr from free_tr */
	LIST_INSERT_HEAD(&qpair->outstanding_tr, tr, list);
	tr->cid = slot;
	req->cmd.cid = tr->cid;
	tr->req = req;
	qpair->act_tr[tr->cid] = tr;
	qpair->tr[tr->cid].active = true;

	cpl = &qpair->cpl[slot];
	cpl->status.p = qpair->phase;
	cpl->cid = slot;
	cpl->cid = tr->cid;
}

static void
@@ -303,7 +302,7 @@ test3(void)
	 * Extract its command ID to retrieve its tracker.
	 */
	cid = qpair.cmd[0].cid;
	tr = qpair.act_tr[cid];
	tr = &qpair.tr[cid];
	SPDK_CU_ASSERT_FATAL(tr != NULL);

	/*
@@ -382,7 +381,6 @@ test_sgl_req(void)

	sgl_tr = LIST_FIRST(&qpair.outstanding_tr);
	LIST_REMOVE(sgl_tr, list);
	free(sgl_tr);
	cleanup_submit_request_test(&qpair);
	nvme_free_request(req);

@@ -431,7 +429,6 @@ test_sgl_req(void)
		}

		LIST_REMOVE(sgl_tr, list);
		free(sgl_tr);
	}
	cleanup_submit_request_test(&qpair);
	nvme_free_request(req);
@@ -487,15 +484,15 @@ static void test_nvme_qpair_fail(void)
	struct spdk_nvme_ctrlr		ctrlr = {};
	struct spdk_nvme_registers	regs = {};
	struct nvme_tracker		*tr_temp;
	uint64_t			phys_addr = 0;

	prepare_submit_request_test(&qpair, &ctrlr, &regs);

	tr_temp = nvme_malloc("nvme_tracker", sizeof(struct nvme_tracker),
			      64, &phys_addr);
	tr_temp = LIST_FIRST(&qpair.free_tr);
	SPDK_CU_ASSERT_FATAL(tr_temp != NULL);
	LIST_REMOVE(tr_temp, list);
	tr_temp->req = nvme_allocate_request_null(expected_failure_callback, NULL);
	SPDK_CU_ASSERT_FATAL(tr_temp->req != NULL);
	tr_temp->req->cmd.cid = tr_temp->cid;

	LIST_INSERT_HEAD(&qpair.outstanding_tr, tr_temp, list);
	nvme_qpair_fail(&qpair);
@@ -563,7 +560,6 @@ static void test_nvme_qpair_destroy(void)
	struct spdk_nvme_ctrlr		ctrlr = {};
	struct spdk_nvme_registers	regs = {};
	struct nvme_tracker		*tr_temp;
	uint64_t			phys_addr = 0;

	memset(&ctrlr, 0, sizeof(ctrlr));
	ctrlr.regs = &regs;
@@ -572,22 +568,21 @@ static void test_nvme_qpair_destroy(void)

	nvme_qpair_construct(&qpair, 1, 128, 32, &ctrlr);
	nvme_qpair_destroy(&qpair);
	CU_ASSERT(LIST_EMPTY(&qpair.free_tr));


	nvme_qpair_construct(&qpair, 0, 128, 32, &ctrlr);
	tr_temp = nvme_malloc("nvme_tracker", sizeof(struct nvme_tracker),
			      64, &phys_addr);
	tr_temp = LIST_FIRST(&qpair.free_tr);
	SPDK_CU_ASSERT_FATAL(tr_temp != NULL);
	LIST_REMOVE(tr_temp, list);
	tr_temp->req = nvme_allocate_request_null(expected_failure_callback, NULL);
	SPDK_CU_ASSERT_FATAL(tr_temp->req != NULL);

	tr_temp->req->cmd.opc = SPDK_NVME_OPC_ASYNC_EVENT_REQUEST;
	tr_temp->req->cmd.cid = tr_temp->cid;
	LIST_INSERT_HEAD(&qpair.outstanding_tr, tr_temp, list);

	nvme_qpair_destroy(&qpair);
	CU_ASSERT(LIST_EMPTY(&qpair.outstanding_tr));
	CU_ASSERT(LIST_EMPTY(&qpair.free_tr));
}

static void test_nvme_completion_is_retry(void)