Commit b4dace73 authored by Ben Walker's avatar Ben Walker Committed by Tomasz Zawadzki
Browse files

nvme: Do not allocate inactive namespace objects 



Some subsystems report a very large maximum value for the number of
namespaces, but in essentially every case the subsystem is sparsely
populated with active namespaces. To save memory, don't allocate
objects for the inactive ones.

Change-Id: I4cbeb5a7a898d3c685f4a3a9ec4c2ce45efffb92
Signed-off-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9898


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent 9d7e239f

lib/nvme/nvme_ctrlr.c

+91 −37
Original line number Diff line number Diff line
@@ -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 (nsid < 1 || nsid > ctrlr->num_ns) {

		return -EINVAL;

	}



	/* Namespaces are constructed on demand, so simply request it. */

	ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

	if (ns == NULL) {

		return -EINVAL;

		return -ENOMEM;

	}



	return nvme_ns_construct(ns, nsid, ctrlr);

	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;



	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;

			}

		}

	uint32_t nsid;

	struct spdk_nvme_ns *ns;



		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

lib/nvme/nvme_internal.h

+2 −2
Original line number Diff line number Diff line
@@ -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;

test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c

+16 −4
Original line number Diff line number Diff line
@@ -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;

test/unit/lib/nvme/nvme_ctrlr_cmd.c/nvme_ctrlr_cmd_ut.c

+11 −1
Original line number Diff line number Diff line
@@ -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()		\

test/unit/lib/nvme/nvme_ctrlr_ocssd_cmd.c/nvme_ctrlr_ocssd_cmd_ut.c

+15 −2
Original line number Diff line number Diff line
@@ -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;