diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 8beb2ce82..1f1601648 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -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; diff --git a/lib/nvme/nvme_qpair.c b/lib/nvme/nvme_qpair.c index 3b87f3418..26db4874c 100644 --- a/lib/nvme/nvme_qpair.c +++ b/lib/nvme/nvme_qpair.c @@ -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. - */ - tr = nvme_malloc("nvme_tr", sizeof(*tr), nvme_align32pow2(sizeof(*tr)), &phys_addr); - if (tr == NULL) { - nvme_printf(ctrlr, "nvme_tr failed\n"); - goto fail; - } - nvme_qpair_construct_tracker(tr, i, phys_addr); - LIST_INSERT_HEAD(&qpair->free_tr, tr, list); - } - - qpair->act_tr = calloc(num_trackers, sizeof(struct nvme_tracker *)); - if (qpair->act_tr == NULL) { - nvme_printf(ctrlr, "alloc nvme_act_tr failed\n"); + /* + * 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. + */ + 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); + } + 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); } /** diff --git a/test/lib/nvme/unit/nvme_qpair_c/nvme_qpair_ut.c b/test/lib/nvme/unit/nvme_qpair_c/nvme_qpair_ut.c index 50807e834..674f04f6e 100644 --- a/test/lib/nvme/unit/nvme_qpair_c/nvme_qpair_ut.c +++ b/test/lib/nvme/unit/nvme_qpair_c/nvme_qpair_ut.c @@ -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, ®s); - 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 = ®s; @@ -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)