d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

nvme: account for PRACT when calculating max sectors per transfer

Browse Source 
There is a special case when using 8-byte metadata + PI + PRACT
where no metadata is transferred to/from controller.

Since _nvme_ns_cmd_rw() already calculates the proper sector size
using _nvme_get_host_buffer_sector_size(), which takes PRACT into
account, change the sectors_per_max_io calculation to also take
PRACT into account.

This will avoid certain requests that don't need splitting getting
split.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6247 (master)

(cherry picked from commit 4249dc1010)
Change-Id: I8d450d37c2458453701189f0e0eca4b8fe71173b
Signed-off-by: John Kariuki <John.K.Kariuki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8023
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
Niklas Cassel 2021-02-03 20:42:48 +00:00 committed by Tomasz Zawadzki
parent 99379a07f1
commit d56a2b7214
4 changed files with 37 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lib/nvme/nvme_internal.h
View File

@ -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;

2
lib/nvme/nvme_ns.c
View File

@ -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;

28
lib/nvme/nvme_ns_cmd.c
View File

@ -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,

64
test/unit/lib/nvme/nvme_ns_cmd.c/nvme_ns_cmd_ut.c
View File

@ -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);