diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 2684d2d70..5a9a3a3ab 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1784,7 +1784,6 @@ typedef void (*nvme_active_ns_ctx_deleter)(struct nvme_active_ns_ctx *); struct nvme_active_ns_ctx { struct spdk_nvme_ctrlr *ctrlr; uint32_t page; - uint32_t num_pages; uint32_t next_nsid; uint32_t *new_ns_list; nvme_active_ns_ctx_deleter deleter; @@ -1796,7 +1795,6 @@ static struct nvme_active_ns_ctx * nvme_active_ns_ctx_create(struct spdk_nvme_ctrlr *ctrlr, nvme_active_ns_ctx_deleter deleter) { struct nvme_active_ns_ctx *ctx; - uint32_t num_pages = 0; uint32_t *new_ns_list = NULL; ctx = calloc(1, sizeof(*ctx)); @@ -1805,19 +1803,14 @@ nvme_active_ns_ctx_create(struct spdk_nvme_ctrlr *ctrlr, nvme_active_ns_ctx_dele return NULL; } - if (ctrlr->num_ns) { - /* The allocated size must be a multiple of sizeof(struct spdk_nvme_ns_list) */ - num_pages = (ctrlr->num_ns * sizeof(new_ns_list[0]) - 1) / sizeof(struct spdk_nvme_ns_list) + 1; - new_ns_list = spdk_zmalloc(num_pages * sizeof(struct spdk_nvme_ns_list), ctrlr->page_size, - NULL, SPDK_ENV_LCORE_ID_ANY, SPDK_MALLOC_DMA | SPDK_MALLOC_SHARE); - if (!new_ns_list) { - NVME_CTRLR_ERRLOG(ctrlr, "Failed to allocate active_ns_list!\n"); - free(ctx); - return NULL; - } + new_ns_list = spdk_zmalloc(sizeof(struct spdk_nvme_ns_list), ctrlr->page_size, + NULL, SPDK_ENV_LCORE_ID_ANY, SPDK_MALLOC_SHARE); + if (!new_ns_list) { + NVME_CTRLR_ERRLOG(ctrlr, "Failed to allocate active_ns_list!\n"); + free(ctx); + return NULL; } - ctx->num_pages = num_pages; ctx->new_ns_list = new_ns_list; ctx->ctrlr = ctrlr; ctx->deleter = deleter; @@ -1835,8 +1828,12 @@ nvme_active_ns_ctx_destroy(struct nvme_active_ns_ctx *ctx) static void nvme_ctrlr_identify_active_ns_swap(struct spdk_nvme_ctrlr *ctrlr, uint32_t **new_ns_list) { + uint32_t max_active_ns_idx = 0; + + while ((*new_ns_list)[max_active_ns_idx++]); spdk_free(ctrlr->active_ns_list); ctrlr->active_ns_list = *new_ns_list; + ctrlr->max_active_ns_idx = max_active_ns_idx; *new_ns_list = NULL; } @@ -1844,6 +1841,7 @@ static void nvme_ctrlr_identify_active_ns_async_done(void *arg, const struct spdk_nvme_cpl *cpl) { struct nvme_active_ns_ctx *ctx = arg; + uint32_t *new_ns_list = NULL; if (spdk_nvme_cpl_is_error(cpl)) { ctx->state = NVME_ACTIVE_NS_STATE_ERROR; @@ -1851,11 +1849,22 @@ nvme_ctrlr_identify_active_ns_async_done(void *arg, const struct spdk_nvme_cpl * } ctx->next_nsid = ctx->new_ns_list[1024 * ctx->page + 1023]; - if (ctx->next_nsid == 0 || ++ctx->page == ctx->num_pages) { + if (ctx->next_nsid == 0) { ctx->state = NVME_ACTIVE_NS_STATE_DONE; goto out; } + ctx->page++; + new_ns_list = spdk_realloc(ctx->new_ns_list, + (ctx->page + 1) * sizeof(struct spdk_nvme_ns_list), + ctx->ctrlr->page_size); + if (!new_ns_list) { + SPDK_ERRLOG("Failed to reallocate active_ns_list!\n"); + ctx->state = NVME_ACTIVE_NS_STATE_ERROR; + goto out; + } + + ctx->new_ns_list = new_ns_list; nvme_ctrlr_identify_active_ns_async(ctx); return; @@ -1872,7 +1881,7 @@ nvme_ctrlr_identify_active_ns_async(struct nvme_active_ns_ctx *ctx) uint32_t i; int rc; - if (ctrlr->num_ns == 0) { + if (ctrlr->cdata.nn == 0) { ctx->state = NVME_ACTIVE_NS_STATE_DONE; goto out; } @@ -1884,7 +1893,27 @@ nvme_ctrlr_identify_active_ns_async(struct nvme_active_ns_ctx *ctx) * an active ns list, i.e. all namespaces report as active */ if (ctrlr->vs.raw < SPDK_NVME_VERSION(1, 1, 0) || ctrlr->quirks & NVME_QUIRK_IDENTIFY_CNS) { - for (i = 0; i < ctrlr->num_ns; i++) { + uint32_t *new_ns_list; + uint32_t num_pages; + + /* + * Active NS list must always end with zero element. + * So, we allocate for cdata.nn+1. + */ + num_pages = spdk_divide_round_up(ctrlr->cdata.nn + 1, + sizeof(struct spdk_nvme_ns_list) / sizeof(new_ns_list[0])); + new_ns_list = spdk_realloc(ctx->new_ns_list, + num_pages * sizeof(struct spdk_nvme_ns_list), + ctx->ctrlr->page_size); + if (!new_ns_list) { + SPDK_ERRLOG("Failed to reallocate active_ns_list!\n"); + ctx->state = NVME_ACTIVE_NS_STATE_ERROR; + goto out; + } + + ctx->new_ns_list = new_ns_list; + ctx->new_ns_list[ctrlr->cdata.nn] = 0; + for (i = 0; i < ctrlr->cdata.nn; i++) { ctx->new_ns_list[i] = i + 1; } @@ -2498,6 +2527,7 @@ nvme_ctrlr_destruct_namespaces(struct spdk_nvme_ctrlr *ctrlr) spdk_free(ctrlr->active_ns_list); ctrlr->active_ns_list = NULL; + ctrlr->max_active_ns_idx = 0; } static void @@ -3631,12 +3661,12 @@ nvme_ctrlr_active_ns_idx(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { int32_t result = -1; - if (ctrlr->active_ns_list == NULL || nsid == 0 || nsid > ctrlr->num_ns) { + if (ctrlr->active_ns_list == NULL || nsid == 0 || nsid > ctrlr->cdata.nn) { return result; } int32_t lower = 0; - int32_t upper = ctrlr->num_ns - 1; + int32_t upper = ctrlr->max_active_ns_idx; int32_t mid; while (lower <= upper) { @@ -3673,7 +3703,7 @@ uint32_t spdk_nvme_ctrlr_get_next_active_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t prev_nsid) { int32_t nsid_idx = nvme_ctrlr_active_ns_idx(ctrlr, prev_nsid); - if (ctrlr->active_ns_list && nsid_idx >= 0 && (uint32_t)nsid_idx < ctrlr->num_ns - 1) { + if (nsid_idx >= 0 && (uint32_t)nsid_idx < ctrlr->max_active_ns_idx) { return ctrlr->active_ns_list[nsid_idx + 1]; } return 0; diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 115f33ec1..21397623a 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -826,6 +826,7 @@ struct spdk_nvme_ctrlr { /** * Keep track of active namespaces */ + uint32_t max_active_ns_idx; uint32_t *active_ns_list; struct spdk_bit_array *free_io_qids; diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 22e65434c..8378b2f2c 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -2,7 +2,7 @@ * BSD LICENSE * * Copyright (c) Intel Corporation. All rights reserved. - * Copyright (c) 2020 Mellanox Technologies LTD. All rights reserved. + * Copyright (c) 2020, 2021 Mellanox Technologies LTD. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -387,6 +387,9 @@ nvme_ctrlr_cmd_set_async_event_config(struct spdk_nvme_ctrlr *ctrlr, return 0; } +static uint32_t *g_active_ns_list = NULL; +static uint32_t g_active_ns_list_length = 0; + int nvme_ctrlr_cmd_identify(struct spdk_nvme_ctrlr *ctrlr, uint8_t cns, uint16_t cntid, uint32_t nsid, uint8_t csi, void *payload, size_t payload_size, @@ -397,14 +400,29 @@ nvme_ctrlr_cmd_identify(struct spdk_nvme_ctrlr *ctrlr, uint8_t cns, uint16_t cnt uint32_t i = 0; struct spdk_nvme_ns_list *ns_list = (struct spdk_nvme_ns_list *)payload; - for (i = 1; i <= ctrlr->num_ns; i++) { - if (i <= nsid) { - continue; - } + memset(payload, 0, payload_size); + if (g_active_ns_list == NULL) { + for (i = 1; i <= ctrlr->num_ns; i++) { + if (i <= nsid) { + continue; + } - ns_list->ns_list[count++] = i; - if (count == SPDK_COUNTOF(ns_list->ns_list)) { - break; + ns_list->ns_list[count++] = i; + if (count == SPDK_COUNTOF(ns_list->ns_list)) { + break; + } + } + } else { + for (i = 0; i < g_active_ns_list_length; i++) { + uint32_t cur_nsid = g_active_ns_list[i]; + if (cur_nsid <= nsid) { + continue; + } + + ns_list->ns_list[count++] = cur_nsid; + if (count == SPDK_COUNTOF(ns_list->ns_list)) { + break; + } } } @@ -1882,17 +1900,17 @@ test_nvme_ctrlr_test_active_ns(void) ctrlr.vs.bits.mjr = 1; ctrlr.vs.bits.mnr = minor; ctrlr.vs.bits.ter = 0; - ctrlr.num_ns = 1531; + ctrlr.num_ns = ctrlr.cdata.nn = 1531; nvme_ctrlr_identify_active_ns(&ctrlr); for (nsid = 1; nsid <= ctrlr.num_ns; nsid++) { CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, nsid) == true); } - ctrlr.num_ns = 1559; - for (; nsid <= ctrlr.num_ns; nsid++) { + + for (; nsid <= 1559; nsid++) { CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, nsid) == false); } - ctrlr.num_ns = 1531; + for (nsid = 0; nsid < ctrlr.num_ns; nsid++) { ctrlr.active_ns_list[nsid] = 0; } @@ -1940,7 +1958,7 @@ test_nvme_ctrlr_test_active_ns_error_case(void) ctrlr.vs.bits.mjr = 1; ctrlr.vs.bits.mnr = 2; ctrlr.vs.bits.ter = 0; - ctrlr.num_ns = 2; + ctrlr.cdata.nn = 2; set_status_code = SPDK_NVME_SC_INVALID_FIELD; rc = nvme_ctrlr_identify_active_ns(&ctrlr); @@ -2474,6 +2492,103 @@ test_nvme_ctrlr_set_state(void) MOCK_CLEAR(spdk_get_ticks); } +static void +test_nvme_ctrlr_active_ns_list_v0(void) +{ + DECLARE_AND_CONSTRUCT_CTRLR(); + + ctrlr.vs.bits.mjr = 1; + ctrlr.vs.bits.mnr = 0; + ctrlr.vs.bits.ter = 0; + ctrlr.cdata.nn = 1024; + + ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS; + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_NS */ + SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_NS); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1)); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1024)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1025)); + CU_ASSERT(spdk_nvme_ctrlr_get_first_active_ns(&ctrlr) == 1); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1023) == 1024); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1024) == 0); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1025) == 0); + + nvme_ctrlr_destruct(&ctrlr); +} + +static void +test_nvme_ctrlr_active_ns_list_v2(void) +{ + uint32_t i; + uint32_t active_ns_list[1024]; + DECLARE_AND_CONSTRUCT_CTRLR(); + + ctrlr.vs.bits.mjr = 1; + ctrlr.vs.bits.mnr = 2; + ctrlr.vs.bits.ter = 0; + ctrlr.cdata.nn = 4096; + + g_active_ns_list = active_ns_list; + g_active_ns_list_length = SPDK_COUNTOF(active_ns_list); + + /* No active namespaces */ + memset(active_ns_list, 0, sizeof(active_ns_list)); + ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS; + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_NS */ + SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_NS); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1024)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1025)); + CU_ASSERT(spdk_nvme_ctrlr_get_first_active_ns(&ctrlr) == 0); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1024) == 0); + + nvme_ctrlr_destruct(&ctrlr); + + /* 1024 active namespaces - one full page */ + memset(active_ns_list, 0, sizeof(active_ns_list)); + for (i = 0; i < 1024; ++i) { + active_ns_list[i] = i + 1; + } + + ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS; + g_active_ns_list = active_ns_list; + g_active_ns_list_length = SPDK_COUNTOF(active_ns_list); + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_NS */ + SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_NS); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1)); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1024)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1025)); + CU_ASSERT(spdk_nvme_ctrlr_get_first_active_ns(&ctrlr) == 1); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1023) == 1024); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1024) == 0); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1025) == 0); + + nvme_ctrlr_destruct(&ctrlr); + + /* 1023 active namespaces - full page minus one */ + memset(active_ns_list, 0, sizeof(active_ns_list)); + for (i = 0; i < 1023; ++i) { + active_ns_list[i] = i + 1; + } + + ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS; + SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_NS */ + SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_NS); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1)); + CU_ASSERT(spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1023)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1024)); + CU_ASSERT(!spdk_nvme_ctrlr_is_active_ns(&ctrlr, 1025)); + CU_ASSERT(spdk_nvme_ctrlr_get_first_active_ns(&ctrlr) == 1); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1023) == 0); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1024) == 0); + CU_ASSERT(spdk_nvme_ctrlr_get_next_active_ns(&ctrlr, 1025) == 0); + + nvme_ctrlr_destruct(&ctrlr); + + g_active_ns_list = NULL; + g_active_ns_list_length = 0; +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; @@ -2518,6 +2633,8 @@ int main(int argc, char **argv) CU_ADD_TEST(suite, test_nvme_cmd_map_sgls); CU_ADD_TEST(suite, test_nvme_ctrlr_set_arbitration_feature); CU_ADD_TEST(suite, test_nvme_ctrlr_set_state); + CU_ADD_TEST(suite, test_nvme_ctrlr_active_ns_list_v0); + CU_ADD_TEST(suite, test_nvme_ctrlr_active_ns_list_v2); CU_basic_set_mode(CU_BRM_VERBOSE); CU_basic_run_tests();