diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 264d0c47e..de2d50638 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -2214,7 +2214,11 @@ nvme_ctrlr_destruct_namespace(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) return -EINVAL; } - ns = &ctrlr->ns[nsid - 1]; + ns = ctrlr->ns[nsid - 1]; + if (ns == NULL) { + return 0; + } + nvme_ns_destruct(ns); return 0; @@ -2225,13 +2229,17 @@ nvme_ctrlr_construct_namespace(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { struct spdk_nvme_ns *ns; - ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); - - if (ns == NULL) { + if (nsid < 1 || nsid > ctrlr->num_ns) { return -EINVAL; } - return nvme_ns_construct(ns, nsid, ctrlr); + /* Namespaces are constructed on demand, so simply request it. */ + ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); + if (ns == NULL) { + return -ENOMEM; + } + + return 0; } static void @@ -2240,12 +2248,50 @@ nvme_ctrlr_identify_active_ns_swap(struct spdk_nvme_ctrlr *ctrlr, uint32_t **new { uint32_t active_ns_count = 0; size_t i; + uint32_t nsid, n; + int rc; + /* First, remove namespaces that no longer exist */ + for (i = 0; i < ctrlr->active_ns_count; i++) { + nsid = ctrlr->active_ns_list[i]; + + assert(nsid != 0); + + n = (*new_ns_list)[0]; + active_ns_count = 0; + while (n != 0) { + if (n == nsid) { + break; + } + + n = (*new_ns_list)[active_ns_count++]; + } + + if (n != nsid) { + /* Did not find this namespace id in the new list. */ + NVME_CTRLR_DEBUGLOG(ctrlr, "Namespace %u was removed\n", nsid); + nvme_ctrlr_destruct_namespace(ctrlr, nsid); + } + } + + /* Next, add new namespaces */ + active_ns_count = 0; for (i = 0; i < max_entries; i++) { - if ((*new_ns_list)[active_ns_count] == 0) { + nsid = (*new_ns_list)[active_ns_count]; + + if (nsid == 0) { break; } + /* If the namespace already exists, this will not construct it a second time. */ + rc = nvme_ctrlr_construct_namespace(ctrlr, nsid); + if (rc != 0) { + /* We can't easily handle a failure here. But just move on. */ + assert(false); + NVME_CTRLR_DEBUGLOG(ctrlr, "Failed to allocate a namespace object.\n"); + continue; + } + active_ns_count++; } @@ -2955,6 +3001,7 @@ nvme_ctrlr_destruct_namespaces(struct spdk_nvme_ctrlr *ctrlr) for (i = 1; i <= num_ns; i++) { nvme_ctrlr_destruct_namespace(ctrlr, i); + spdk_free(ctrlr->ns[i - 1]); } spdk_free(ctrlr->ns); @@ -2966,29 +3013,13 @@ nvme_ctrlr_destruct_namespaces(struct spdk_nvme_ctrlr *ctrlr) void nvme_ctrlr_update_namespaces(struct spdk_nvme_ctrlr *ctrlr) { - uint32_t i, nn = ctrlr->cdata.nn; - struct spdk_nvme_ns_data *nsdata; - bool ns_is_active; + uint32_t nsid; + struct spdk_nvme_ns *ns; - for (i = 0; i < nn; i++) { - uint32_t nsid = i + 1; - struct spdk_nvme_ns *ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); - - assert(ns != NULL); - nsdata = &ns->nsdata; - ns_is_active = spdk_nvme_ctrlr_is_active_ns(ctrlr, nsid); - - if (ns_is_active) { - NVME_CTRLR_DEBUGLOG(ctrlr, "Namespace %u was added\n", nsid); - if (nvme_ctrlr_construct_namespace(ctrlr, nsid) != 0) { - continue; - } - } - - if (nsdata->ncap && !ns_is_active) { - NVME_CTRLR_DEBUGLOG(ctrlr, "Namespace %u was removed\n", nsid); - nvme_ctrlr_destruct_namespace(ctrlr, nsid); - } + for (nsid = spdk_nvme_ctrlr_get_first_active_ns(ctrlr); + nsid != 0; nsid = spdk_nvme_ctrlr_get_next_active_ns(ctrlr, nsid)) { + ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); + nvme_ns_construct(ns, nsid, ctrlr); } } @@ -2997,32 +3028,34 @@ nvme_ctrlr_construct_namespaces(struct spdk_nvme_ctrlr *ctrlr) { int rc = 0; uint32_t i, nn = ctrlr->cdata.nn; - struct spdk_nvme_ns *tmp; + struct spdk_nvme_ns **tmp; + struct spdk_nvme_ns *ns; /* ctrlr->num_ns may be 0 (startup) or a different number of namespaces (reset), * so check if we need to reallocate. */ if (nn != ctrlr->num_ns) { - tmp = spdk_realloc(ctrlr->ns, nn * sizeof(struct spdk_nvme_ns), 64); + + tmp = spdk_realloc(ctrlr->ns, nn * sizeof(struct spdk_nvme_ns *), 64); + if (tmp == NULL) { rc = -ENOMEM; goto fail; } if (nn > ctrlr->num_ns) { - memset(tmp + ctrlr->num_ns, 0, (nn - ctrlr->num_ns) * sizeof(struct spdk_nvme_ns)); + memset(tmp + ctrlr->num_ns, 0, (nn - ctrlr->num_ns) * sizeof(struct spdk_nvme_ns *)); } ctrlr->ns = tmp; ctrlr->num_ns = nn; } - /* - * The controller could have been reset with the same number of namespaces. - * If so, we still need to free the iocs specific data, to get a clean slate. - */ for (i = 0; i < ctrlr->num_ns; i++) { - nvme_ns_free_iocs_specific_data(&ctrlr->ns[i]); + ns = ctrlr->ns[i]; + if (ns != NULL) { + nvme_ns_free_iocs_specific_data(ns); + } } return 0; @@ -4459,11 +4492,30 @@ spdk_nvme_ctrlr_get_next_active_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t prev_ struct spdk_nvme_ns * spdk_nvme_ctrlr_get_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { + struct spdk_nvme_ns *ns; + if (nsid < 1 || nsid > ctrlr->num_ns) { return NULL; } - return &ctrlr->ns[nsid - 1]; + nvme_robust_mutex_lock(&ctrlr->ctrlr_lock); + + ns = ctrlr->ns[nsid - 1]; + + if (ns == NULL) { + ns = spdk_zmalloc(sizeof(struct spdk_nvme_ns), 64, NULL, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_SHARE); + if (ns == NULL) { + nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); + return NULL; + } + + NVME_CTRLR_DEBUGLOG(ctrlr, "Namespace %u was added\n", nsid); + ctrlr->ns[nsid - 1] = ns; + } + + nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock); + + return ns; } struct spdk_pci_device * @@ -4547,6 +4599,7 @@ spdk_nvme_ctrlr_attach_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct spdk_nvme_ctrlr_list *payload) { struct nvme_completion_poll_status *status; + struct spdk_nvme_ns *ns; int res; if (nsid == 0) { @@ -4579,7 +4632,8 @@ spdk_nvme_ctrlr_attach_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, return res; } - return nvme_ctrlr_construct_namespace(ctrlr, nsid); + ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid); + return nvme_ns_construct(ns, nsid, ctrlr); } int diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 4d32b7146..27d6b735b 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -857,8 +857,8 @@ struct nvme_register_completion { struct spdk_nvme_ctrlr { /* Hot data (accessed in I/O path) starts here. */ - /** Array of namespaces indexed by nsid - 1 */ - struct spdk_nvme_ns *ns; + /** Array of namespace pointers indexed by nsid - 1 */ + struct spdk_nvme_ns **ns; uint32_t num_ns; 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 b7362c63a..0a254fa0e 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 @@ -2103,7 +2103,7 @@ test_nvme_ctrlr_test_active_ns(void) ctrlr.vs.bits.ter = 0; ctrlr.cdata.nn = 1531; - ctrlr.ns = calloc(ctrlr.cdata.nn, sizeof(struct spdk_nvme_ns)); + ctrlr.ns = calloc(ctrlr.cdata.nn, sizeof(struct spdk_nvme_ns *)); SPDK_CU_ASSERT_FATAL(ctrlr.ns != NULL); ctrlr.num_ns = ctrlr.cdata.nn; @@ -2145,6 +2145,7 @@ test_nvme_ctrlr_test_active_ns(void) for (nsid = 0; nsid < ctrlr.num_ns; nsid++) { ctrlr.active_ns_list[nsid] = nsid + 1; } + ctrlr.active_ns_list[ctrlr.active_ns_count] = 0; ns_id_count = 0; for (nsid = spdk_nvme_ctrlr_get_first_active_ns(&ctrlr); @@ -2900,10 +2901,16 @@ test_nvme_ctrlr_identify_namespaces_iocs_specific_next(void) uint32_t prev_nsid; uint32_t active_ns_list[5] = {1, 2, 3, 4, 5}; struct spdk_nvme_ns ns[5] = {}; + struct spdk_nvme_ns *ns_array[5]; struct spdk_nvme_ctrlr ns_ctrlr[5] = {}; int rc = 0; + int i; - ctrlr.ns = ns; + for (i = 0; i < 5; i++) { + ns_array[i] = &ns[i]; + } + + ctrlr.ns = ns_array; ctrlr.cdata.nn = 5; ctrlr.active_ns_count = 5; ctrlr.num_ns = 5; @@ -3027,15 +3034,20 @@ test_nvme_ctrlr_set_intel_supported_log_pages(void) static void test_nvme_ctrlr_parse_ana_log_page(void) { - int rc; + int rc, i; struct spdk_nvme_ctrlr ctrlr = {}; struct spdk_nvme_ns ns[3] = {}; + struct spdk_nvme_ns *ns_array[3]; struct spdk_nvme_ana_page ana_hdr; char _ana_desc[UT_ANA_DESC_SIZE]; struct spdk_nvme_ana_group_descriptor *ana_desc; uint32_t offset; - ctrlr.ns = ns; + for (i = 0; i < 3; i++) { + ns_array[i] = &ns[i]; + } + + ctrlr.ns = ns_array; ctrlr.cdata.nn = 3; ctrlr.cdata.nanagrpid = 3; ctrlr.num_ns = 3; diff --git a/test/unit/lib/nvme/nvme_ctrlr_cmd.c/nvme_ctrlr_cmd_ut.c b/test/unit/lib/nvme/nvme_ctrlr_cmd.c/nvme_ctrlr_cmd_ut.c index 9f4d1904e..80d0ff5b5 100644 --- a/test/unit/lib/nvme/nvme_ctrlr_cmd.c/nvme_ctrlr_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr_cmd.c/nvme_ctrlr_cmd_ut.c @@ -368,14 +368,24 @@ nvme_ctrlr_submit_admin_request(struct spdk_nvme_ctrlr *ctrlr, struct nvme_reque return 0; } +static struct spdk_nvme_ns g_inactive_ns = {}; + struct spdk_nvme_ns * spdk_nvme_ctrlr_get_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { + struct spdk_nvme_ns *ns; + if (nsid < 1 || nsid > ctrlr->num_ns) { return NULL; } - return &ctrlr->ns[nsid - 1]; + ns = ctrlr->ns[nsid - 1]; + + if (ns == NULL) { + return &g_inactive_ns; + } + + return ns; } #define DECLARE_AND_CONSTRUCT_CTRLR() \ diff --git a/test/unit/lib/nvme/nvme_ctrlr_ocssd_cmd.c/nvme_ctrlr_ocssd_cmd_ut.c b/test/unit/lib/nvme/nvme_ctrlr_ocssd_cmd.c/nvme_ctrlr_ocssd_cmd_ut.c index 1e067efa3..31f5c0fd5 100644 --- a/test/unit/lib/nvme/nvme_ctrlr_ocssd_cmd.c/nvme_ctrlr_ocssd_cmd_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr_ocssd_cmd.c/nvme_ctrlr_ocssd_cmd_ut.c @@ -59,14 +59,24 @@ verify_request_fn_t verify_fn; static const uint32_t expected_geometry_ns = 1; +static struct spdk_nvme_ns g_inactive_ns = {}; + struct spdk_nvme_ns * spdk_nvme_ctrlr_get_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { + struct spdk_nvme_ns *ns; + if (nsid < 1 || nsid > ctrlr->num_ns) { return NULL; } - return &ctrlr->ns[nsid - 1]; + ns = ctrlr->ns[nsid - 1]; + + if (ns == NULL) { + return &g_inactive_ns; + } + + return ns; } int @@ -111,10 +121,13 @@ test_spdk_nvme_ctrlr_is_ocssd_supported(void) { struct spdk_nvme_ctrlr ctrlr = {}; struct spdk_nvme_ns ns = {}; + struct spdk_nvme_ns *ns_array; bool rc; + ns_array = &ns; + ns.nsdata.vendor_specific[0] = 1; - ctrlr.ns = &ns; + ctrlr.ns = &ns_array; ctrlr.quirks |= NVME_QUIRK_OCSSD; ctrlr.cdata.vid = SPDK_PCI_VID_CNEXLABS; ctrlr.num_ns = 1; diff --git a/test/unit/lib/nvme/nvme_cuse.c/nvme_cuse_ut.c b/test/unit/lib/nvme/nvme_cuse.c/nvme_cuse_ut.c index 2468ed2c2..c34b2411e 100644 --- a/test/unit/lib/nvme/nvme_cuse.c/nvme_cuse_ut.c +++ b/test/unit/lib/nvme/nvme_cuse.c/nvme_cuse_ut.c @@ -130,14 +130,24 @@ spdk_nvme_ns_get_sector_size(struct spdk_nvme_ns *ns) return ns->sector_size; } +static struct spdk_nvme_ns g_inactive_ns = {}; + struct spdk_nvme_ns * spdk_nvme_ctrlr_get_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) { + struct spdk_nvme_ns *ns; + if (nsid < 1 || nsid > ctrlr->num_ns) { return NULL; } - return &ctrlr->ns[nsid - 1]; + ns = ctrlr->ns[nsid - 1]; + + if (ns == NULL) { + return &g_inactive_ns; + } + + return ns; } struct cuse_device *g_cuse_device; @@ -281,6 +291,7 @@ test_cuse_nvme_submit_io(void) struct spdk_nvme_ctrlr ctrlr = {}; struct fuse_file_info fi = {}; struct spdk_nvme_ns ns = {}; + struct spdk_nvme_ns *ns_array; struct nvme_user_io *user_io = NULL; char arg[1024] = {}; fuse_req_t req = (void *)0xDEEACDFF; @@ -289,8 +300,10 @@ test_cuse_nvme_submit_io(void) user_io = calloc(3, 4096); SPDK_CU_ASSERT_FATAL(user_io != NULL); + ns_array = &ns; + cuse_device.ctrlr = &ctrlr; - ctrlr.ns = &ns; + ctrlr.ns = &ns_array; ctrlr.num_ns = 1; ns.sector_size = 4096; ns.id = 1;