From 68d3bb2de406f1e6152122c162565f489c1fa84f Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Wed, 25 Mar 2020 04:41:12 -0400 Subject: [PATCH] nvme: save separate metadata size to nvme request Previously the SPDK NVMe driver always set PSDT to 01b for hardware SGLs which is aligned to the Linux NVMe driver, for this case the metadata length is not required when filling the NVMe command fields. There is no alignment nor granularity requirement for Data Blocks for PSDT 01b case. And if the drive reported that it needs dword alignment with SGL, for this case, when using spearate metadata, it needs a length parameter to fill the SGL descriptor. Change-Id: I56ffaada775fe66de7637dae15b509ee9556e80a Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1351 Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- lib/nvme/nvme_ctrlr_cmd.c | 13 +++++++++++-- lib/nvme/nvme_internal.h | 7 +++++-- lib/nvme/nvme_ns_cmd.c | 3 ++- lib/nvme/nvme_ns_ocssd_cmd.c | 3 ++- test/unit/lib/nvme/nvme.c/nvme_ut.c | 4 ++-- test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c | 5 +++++ 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/nvme/nvme_ctrlr_cmd.c b/lib/nvme/nvme_ctrlr_cmd.c index 9c9b0c3c6..968f20f38 100644 --- a/lib/nvme/nvme_ctrlr_cmd.c +++ b/lib/nvme/nvme_ctrlr_cmd.c @@ -47,7 +47,7 @@ spdk_nvme_ctrlr_io_cmd_raw_no_payload_build(struct spdk_nvme_ctrlr *ctrlr, } memset(&payload, 0, sizeof(payload)); - req = nvme_allocate_request(qpair, &payload, 0, cb_fn, cb_arg); + req = nvme_allocate_request(qpair, &payload, 0, 0, cb_fn, cb_arg); if (req == NULL) { return -ENOMEM; @@ -87,10 +87,19 @@ spdk_nvme_ctrlr_cmd_io_raw_with_md(struct spdk_nvme_ctrlr *ctrlr, { struct nvme_request *req; struct nvme_payload payload; + uint32_t md_len = 0; payload = NVME_PAYLOAD_CONTIG(buf, md_buf); - req = nvme_allocate_request(qpair, &payload, len, cb_fn, cb_arg); + /* Caculate metadata length */ + if (md_buf) { + struct spdk_nvme_ns *ns = &ctrlr->ns[cmd->nsid - 1]; + + assert(ns->sector_size != 0); + md_len = len / ns->sector_size * ns->md_size; + } + + req = nvme_allocate_request(qpair, &payload, len, md_len, cb_fn, cb_arg); if (req == NULL) { return -ENOMEM; } diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index a4f79e5e5..40f5e847c 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -295,6 +295,8 @@ struct nvme_request { pid_t pid; struct spdk_nvme_cpl cpl; + uint32_t md_size; + /** * The following members should not be reordered with members * above. These members are only needed when splitting @@ -901,7 +903,7 @@ int nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entrie static inline struct nvme_request * nvme_allocate_request(struct spdk_nvme_qpair *qpair, - const struct nvme_payload *payload, uint32_t payload_size, + const struct nvme_payload *payload, uint32_t payload_size, uint32_t md_size, spdk_nvme_cmd_cb cb_fn, void *cb_arg) { struct nvme_request *req; @@ -930,6 +932,7 @@ nvme_allocate_request(struct spdk_nvme_qpair *qpair, req->cb_arg = cb_arg; req->payload = *payload; req->payload_size = payload_size; + req->md_size = md_size; req->pid = g_spdk_nvme_pid; req->submit_tick = 0; @@ -945,7 +948,7 @@ nvme_allocate_request_contig(struct spdk_nvme_qpair *qpair, payload = NVME_PAYLOAD_CONTIG(buffer, NULL); - return nvme_allocate_request(qpair, &payload, payload_size, cb_fn, cb_arg); + return nvme_allocate_request(qpair, &payload, payload_size, 0, cb_fn, cb_arg); } static inline struct nvme_request * diff --git a/lib/nvme/nvme_ns_cmd.c b/lib/nvme/nvme_ns_cmd.c index 2cb4ad1ed..d57b9be02 100644 --- a/lib/nvme/nvme_ns_cmd.c +++ b/lib/nvme/nvme_ns_cmd.c @@ -402,7 +402,8 @@ _nvme_ns_cmd_rw(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, sector_size -= 8; } - req = nvme_allocate_request(qpair, payload, lba_count * sector_size, cb_fn, cb_arg); + req = nvme_allocate_request(qpair, payload, lba_count * sector_size, lba_count * ns->md_size, + cb_fn, cb_arg); if (req == NULL) { return NULL; } diff --git a/lib/nvme/nvme_ns_ocssd_cmd.c b/lib/nvme/nvme_ns_ocssd_cmd.c index 92a589ab6..f60aa6789 100644 --- a/lib/nvme/nvme_ns_ocssd_cmd.c +++ b/lib/nvme/nvme_ns_ocssd_cmd.c @@ -103,7 +103,8 @@ _nvme_ocssd_ns_cmd_vector_rw_with_md(struct spdk_nvme_ns *ns, payload = NVME_PAYLOAD_CONTIG(buffer, metadata); - req = nvme_allocate_request(qpair, &payload, num_lbas * ns->sector_size, cb_fn, cb_arg); + req = nvme_allocate_request(qpair, &payload, num_lbas * ns->sector_size, num_lbas * ns->md_size, + cb_fn, cb_arg); if (req == NULL) { return -ENOMEM; } diff --git a/test/unit/lib/nvme/nvme.c/nvme_ut.c b/test/unit/lib/nvme/nvme.c/nvme_ut.c index 784c08d18..e6542818e 100644 --- a/test/unit/lib/nvme/nvme.c/nvme_ut.c +++ b/test/unit/lib/nvme/nvme.c/nvme_ut.c @@ -662,13 +662,13 @@ test_nvme_allocate_request(void) STAILQ_INIT(&qpair.queued_req); /* Test trying to allocate a request when no requests are available */ - req = nvme_allocate_request(&qpair, &payload, payload_struct_size, + req = nvme_allocate_request(&qpair, &payload, payload_struct_size, 0, cb_fn, cb_arg); CU_ASSERT(req == NULL); /* put a dummy on the queue, and then allocate one */ STAILQ_INSERT_HEAD(&qpair.free_req, &dummy_req, stailq); - req = nvme_allocate_request(&qpair, &payload, payload_struct_size, + req = nvme_allocate_request(&qpair, &payload, payload_struct_size, 0, cb_fn, cb_arg); /* all the req elements should now match the passed in parameters */ diff --git a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c index d6d5c1c64..fc73e309a 100644 --- a/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c @@ -1250,6 +1250,7 @@ test_nvme_ns_cmd_write_with_md(void) SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); CU_ASSERT(g_request->payload.md == metadata); + CU_ASSERT(g_request->md_size == 256 * 128); CU_ASSERT(g_request->payload_size == 256 * 512); nvme_free_request(g_request); @@ -1383,6 +1384,7 @@ test_nvme_ns_cmd_write_with_md(void) SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); CU_ASSERT(g_request->payload.md == metadata); + CU_ASSERT(g_request->md_size == 256 * 8); CU_ASSERT(g_request->payload_size == 256 * 512); nvme_free_request(g_request); @@ -1414,12 +1416,14 @@ test_nvme_ns_cmd_write_with_md(void) CU_ASSERT(child0->payload_offset == 0); CU_ASSERT(child0->payload_size == 256 * 512); CU_ASSERT(child0->md_offset == 0); + CU_ASSERT(child0->md_size == 256 * 8); child1 = TAILQ_NEXT(child0, child_tailq); SPDK_CU_ASSERT_FATAL(child1 != NULL); CU_ASSERT(child1->payload_offset == 256 * 512); CU_ASSERT(child1->payload_size == 128 * 512); CU_ASSERT(child1->md_offset == 256 * 8); + CU_ASSERT(child1->md_size == 128 * 8); nvme_request_free_children(g_request); nvme_free_request(g_request); @@ -1466,6 +1470,7 @@ test_nvme_ns_cmd_read_with_md(void) SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); CU_ASSERT(g_request->payload.md == metadata); + CU_ASSERT(g_request->md_size == 256 * md_size); CU_ASSERT(g_request->payload_size == 256 * 512); nvme_free_request(g_request);