diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 162668002a..128a70f524 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -481,6 +481,7 @@ struct spdk_nvme_ns { uint32_t md_size; uint32_t pi_type; uint32_t sectors_per_max_io; + uint32_t sectors_per_max_io_no_md; uint32_t sectors_per_stripe; uint32_t id; uint16_t flags; diff --git a/lib/nvme/nvme_ns.c b/lib/nvme/nvme_ns.c index f5cf75b659..7f8fa38c90 100644 --- a/lib/nvme/nvme_ns.c +++ b/lib/nvme/nvme_ns.c @@ -64,6 +64,7 @@ nvme_ns_set_identify_data(struct spdk_nvme_ns *ns) } ns->sectors_per_max_io = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->extended_lba_size; + ns->sectors_per_max_io_no_md = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->sector_size; if (nsdata->noiob) { ns->sectors_per_stripe = nsdata->noiob; @@ -556,6 +557,7 @@ void nvme_ns_destruct(struct spdk_nvme_ns *ns) ns->md_size = 0; ns->pi_type = 0; ns->sectors_per_max_io = 0; + ns->sectors_per_max_io_no_md = 0; ns->sectors_per_stripe = 0; ns->flags = 0; ns->csi = SPDK_NVME_CSI_NVM; diff --git a/lib/nvme/nvme_ns_cmd.c b/lib/nvme/nvme_ns_cmd.c index 73246f80c3..6760d05817 100644 --- a/lib/nvme/nvme_ns_cmd.c +++ b/lib/nvme/nvme_ns_cmd.c @@ -62,19 +62,27 @@ nvme_ns_check_request_length(uint32_t lba_count, uint32_t sectors_per_max_io, return child_per_io >= qdepth; } +static inline bool +_nvme_md_excluded_from_xfer(struct spdk_nvme_ns *ns, uint32_t io_flags) +{ + return (io_flags & SPDK_NVME_IO_FLAGS_PRACT) && + (ns->flags & SPDK_NVME_NS_EXTENDED_LBA_SUPPORTED) && + (ns->flags & SPDK_NVME_NS_DPS_PI_SUPPORTED) && + (ns->md_size == 8); +} + static inline uint32_t _nvme_get_host_buffer_sector_size(struct spdk_nvme_ns *ns, uint32_t io_flags) { - uint32_t sector_size = ns->extended_lba_size; + return _nvme_md_excluded_from_xfer(ns, io_flags) ? + ns->sector_size : ns->extended_lba_size; +} - if ((io_flags & SPDK_NVME_IO_FLAGS_PRACT) && - (ns->flags & SPDK_NVME_NS_EXTENDED_LBA_SUPPORTED) && - (ns->flags & SPDK_NVME_NS_DPS_PI_SUPPORTED) && - (ns->md_size == 8)) { - sector_size -= 8; - } - - return sector_size; +static inline uint32_t +_nvme_get_sectors_per_max_io(struct spdk_nvme_ns *ns, uint32_t io_flags) +{ + return _nvme_md_excluded_from_xfer(ns, io_flags) ? + ns->sectors_per_max_io_no_md : ns->sectors_per_max_io; } static struct nvme_request * @@ -393,7 +401,7 @@ _nvme_ns_cmd_rw(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, { struct nvme_request *req; uint32_t sector_size = _nvme_get_host_buffer_sector_size(ns, io_flags); - uint32_t sectors_per_max_io = ns->sectors_per_max_io; + uint32_t sectors_per_max_io = _nvme_get_sectors_per_max_io(ns, io_flags); uint32_t sectors_per_stripe = ns->sectors_per_stripe; req = nvme_allocate_request(qpair, payload, lba_count * sector_size, lba_count * ns->md_size, 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 e352da0f98..fb6eb6104e 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 @@ -240,6 +240,7 @@ prepare_for_test(struct spdk_nvme_ns *ns, struct spdk_nvme_ctrlr *ctrlr, } ns->md_size = md_size; ns->sectors_per_max_io = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->extended_lba_size; + ns->sectors_per_max_io_no_md = spdk_nvme_ns_get_max_io_xfer_size(ns) / ns->sector_size; ns->sectors_per_stripe = stripe_size / ns->extended_lba_size; memset(qpair, 0, sizeof(*qpair)); @@ -909,11 +910,7 @@ test_nvme_ns_cmd_comparev_with_md(void) * Protection information enabled + PRACT * * Special case for 8-byte metadata + PI + PRACT: no metadata transferred - * In theory, 256 blocks * 512 bytes per block = one I/O (128 KB) - * However, the splitting code does not account for PRACT when calculating - * max sectors per transfer, so we actually get two I/Os: - * child 0: 252 blocks - * child 1: 4 blocks + * 256 blocks * 512 bytes per block = single 128 KB I/O (no splitting required) */ prepare_for_test(&ns, &ctrlr, &qpair, 512, 8, 128 * 1024, 0, true); ns.flags |= SPDK_NVME_NS_DPS_PI_SUPPORTED; @@ -923,18 +920,11 @@ test_nvme_ns_cmd_comparev_with_md(void) SPDK_CU_ASSERT_FATAL(rc == 0); SPDK_CU_ASSERT_FATAL(g_request != NULL); - SPDK_CU_ASSERT_FATAL(g_request->num_children == 2); - child0 = TAILQ_FIRST(&g_request->children); + SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); - SPDK_CU_ASSERT_FATAL(child0 != NULL); - CU_ASSERT(child0->payload_offset == 0); - CU_ASSERT(child0->payload_size == 252 * 512); /* NOTE: does not include metadata! */ - child1 = TAILQ_NEXT(child0, child_tailq); - - SPDK_CU_ASSERT_FATAL(child1 != NULL); - CU_ASSERT(child1->payload.md == NULL); - CU_ASSERT(child1->payload_offset == 252 * 512); - CU_ASSERT(child1->payload_size == 4 * 512); + CU_ASSERT(g_request->payload.md == NULL); + CU_ASSERT(g_request->payload_offset == 0); + CU_ASSERT(g_request->payload_size == 256 * 512); /* NOTE: does not include metadata! */ nvme_request_free_children(g_request); nvme_free_request(g_request); @@ -1373,11 +1363,7 @@ test_nvme_ns_cmd_write_with_md(void) * Protection information enabled + PRACT * * Special case for 8-byte metadata + PI + PRACT: no metadata transferred - * In theory, 256 blocks * 512 bytes per block = one I/O (128 KB) - * However, the splitting code does not account for PRACT when calculating - * max sectors per transfer, so we actually get two I/Os: - * child 0: 252 blocks - * child 1: 4 blocks + * 256 blocks * 512 bytes per block = single 128 KB I/O (no splitting required) */ prepare_for_test(&ns, &ctrlr, &qpair, 512, 8, 128 * 1024, 0, true); ns.flags |= SPDK_NVME_NS_DPS_PI_SUPPORTED; @@ -1387,18 +1373,11 @@ test_nvme_ns_cmd_write_with_md(void) SPDK_CU_ASSERT_FATAL(rc == 0); SPDK_CU_ASSERT_FATAL(g_request != NULL); - SPDK_CU_ASSERT_FATAL(g_request->num_children == 2); - child0 = TAILQ_FIRST(&g_request->children); + SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); - SPDK_CU_ASSERT_FATAL(child0 != NULL); - CU_ASSERT(child0->payload_offset == 0); - CU_ASSERT(child0->payload_size == 252 * 512); /* NOTE: does not include metadata! */ - child1 = TAILQ_NEXT(child0, child_tailq); - - SPDK_CU_ASSERT_FATAL(child1 != NULL); - CU_ASSERT(child1->payload.md == NULL); - CU_ASSERT(child1->payload_offset == 252 * 512); - CU_ASSERT(child1->payload_size == 4 * 512); + CU_ASSERT(g_request->payload.md == NULL); + CU_ASSERT(g_request->payload_offset == 0); + CU_ASSERT(g_request->payload_size == 256 * 512); /* NOTE: does not include metadata! */ nvme_request_free_children(g_request); nvme_free_request(g_request); @@ -1758,11 +1737,7 @@ test_nvme_ns_cmd_compare_with_md(void) * Protection information enabled + PRACT * * Special case for 8-byte metadata + PI + PRACT: no metadata transferred - * In theory, 256 blocks * 512 bytes per block = one I/O (128 KB) - * However, the splitting code does not account for PRACT when calculating - * max sectors per transfer, so we actually get two I/Os: - * child 0: 252 blocks - * child 1: 4 blocks + * 256 blocks * 512 bytes per block = single 128 KB I/O (no splitting required) */ prepare_for_test(&ns, &ctrlr, &qpair, 512, 8, 128 * 1024, 0, true); ns.flags |= SPDK_NVME_NS_DPS_PI_SUPPORTED; @@ -1772,18 +1747,11 @@ test_nvme_ns_cmd_compare_with_md(void) SPDK_CU_ASSERT_FATAL(rc == 0); SPDK_CU_ASSERT_FATAL(g_request != NULL); - SPDK_CU_ASSERT_FATAL(g_request->num_children == 2); - child0 = TAILQ_FIRST(&g_request->children); + SPDK_CU_ASSERT_FATAL(g_request->num_children == 0); - SPDK_CU_ASSERT_FATAL(child0 != NULL); - CU_ASSERT(child0->payload_offset == 0); - CU_ASSERT(child0->payload_size == 252 * 512); /* NOTE: does not include metadata! */ - child1 = TAILQ_NEXT(child0, child_tailq); - - SPDK_CU_ASSERT_FATAL(child1 != NULL); - CU_ASSERT(child1->payload.md == NULL); - CU_ASSERT(child1->payload_offset == 252 * 512); - CU_ASSERT(child1->payload_size == 4 * 512); + CU_ASSERT(g_request->payload.md == NULL); + CU_ASSERT(g_request->payload_offset == 0); + CU_ASSERT(g_request->payload_size == 256 * 512); /* NOTE: does not include metadata! */ nvme_request_free_children(g_request); nvme_free_request(g_request);