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

bdev/nvme: Don't allocate inactive namespaces 



If the number of namespaces is very large, this can cause excessive
memory allocation. This is especially true because when the number of
namespaces is large, it is almost always very sparsely populated.

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


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
parent bb555ef1

module/bdev/nvme/bdev_nvme.c

+29 −24
Original line number Diff line number Diff line
@@ -347,10 +347,13 @@ bdev_nvme_destruct(void *ctx) 


	nvme_ns->bdev = NULL;



	if (!nvme_ns->populated) {

	assert(nvme_ns->id > 0);



	if (nvme_ns->ctrlr->namespaces[nvme_ns->id - 1] == NULL) {

		pthread_mutex_unlock(&nvme_ns->ctrlr->mutex);



		nvme_ctrlr_release(nvme_ns->ctrlr);

		free(nvme_ns);

	} else {

		pthread_mutex_unlock(&nvme_ns->ctrlr->mutex);

	}
@@ -1699,7 +1702,6 @@ nvme_ctrlr_populate_namespace(struct nvme_ctrlr *nvme_ctrlr, struct nvme_ns *nvm 
	}



	nvme_ns->ns = ns;

	nvme_ns->populated = true;

	nvme_ns->ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE;



	if (nvme_ctrlr->ana_log_page != NULL) {
@@ -1714,7 +1716,8 @@ done: 
		nvme_ctrlr->ref++;

		pthread_mutex_unlock(&nvme_ctrlr->mutex);

	} else {

		memset(nvme_ns, 0, sizeof(*nvme_ns));

		nvme_ctrlr->namespaces[nvme_ns->id - 1] = NULL;

		free(nvme_ns);

	}



	if (ctx) {
@@ -1737,12 +1740,14 @@ nvme_ctrlr_depopulate_namespace(struct nvme_ctrlr *nvme_ctrlr, struct nvme_ns *n 


	pthread_mutex_lock(&nvme_ctrlr->mutex);



	nvme_ns->populated = false;

	nvme_ctrlr->namespaces[nvme_ns->id - 1] = NULL;



	if (nvme_ns->bdev != NULL) {

		pthread_mutex_unlock(&nvme_ctrlr->mutex);

		return;

	}



	free(nvme_ns);

	pthread_mutex_unlock(&nvme_ctrlr->mutex);



	nvme_ctrlr_release(nvme_ctrlr);
@@ -1774,7 +1779,7 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
		nvme_ns = nvme_ctrlr->namespaces[i];

		ns_is_active = spdk_nvme_ctrlr_is_active_ns(ctrlr, nsid);



		if (nvme_ns->populated && ns_is_active) {

		if (nvme_ns != NULL && ns_is_active) {

			/* NS is still there but attributes may have changed */

			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

			num_sectors = spdk_nvme_ns_get_num_sectors(ns);
@@ -1794,7 +1799,16 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
			}

		}



		if (!nvme_ns->populated && ns_is_active) {

		if (nvme_ns == NULL && ns_is_active) {

			nvme_ns = calloc(1, sizeof(struct nvme_ns));

			if (nvme_ns == NULL) {

				SPDK_ERRLOG("Failed to allocate namespace\n");

				/* This just fails to attach the namespace. It may work on a future attempt. */

				continue;

			}



			nvme_ctrlr->namespaces[nsid - 1] = nvme_ns;



			nvme_ns->id = nsid;

			nvme_ns->ctrlr = nvme_ctrlr;
@@ -1806,7 +1820,7 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
			nvme_ctrlr_populate_namespace(nvme_ctrlr, nvme_ns, ctx);

		}



		if (nvme_ns->populated && !ns_is_active) {

		if (nvme_ns != NULL && !ns_is_active) {

			nvme_ctrlr_depopulate_namespace(nvme_ctrlr, nvme_ns);

		}

	}
@@ -1835,7 +1849,7 @@ nvme_ctrlr_depopulate_namespaces(struct nvme_ctrlr *nvme_ctrlr) 
		uint32_t nsid = i + 1;



		nvme_ns = nvme_ctrlr->namespaces[nsid - 1];

		if (nvme_ns->populated) {

		if (nvme_ns != NULL) {

			assert(nvme_ns->id == nsid);

			nvme_ctrlr_depopulate_namespace(nvme_ctrlr, nvme_ns);

		}
@@ -1870,9 +1884,10 @@ nvme_ctrlr_set_ana_states(const struct spdk_nvme_ana_group_descriptor *desc, 
		}



		nvme_ns = nvme_ctrlr->namespaces[nsid - 1];

		assert(nvme_ns != NULL);



		if (!nvme_ns->populated) {

		assert(nvme_ns != NULL);

		if (nvme_ns == NULL) {

			/* Target told us that an inactive namespace had an ANA change */

			continue;

		}
@@ -2047,7 +2062,7 @@ nvme_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr, 
{

	struct nvme_ctrlr *nvme_ctrlr;

	struct nvme_ctrlr_trid *trid_entry;

	uint32_t i, num_ns;

	uint32_t num_ns;

	const struct spdk_nvme_ctrlr_data *cdata;

	int rc;
@@ -2074,17 +2089,7 @@ nvme_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr, 
			goto err;

		}



		for (i = 0; i < num_ns; i++) {

			nvme_ctrlr->namespaces[i] = calloc(1, sizeof(struct nvme_ns));

			if (nvme_ctrlr->namespaces[i] == NULL) {

				SPDK_ERRLOG("Failed to allocate block namespace struct\n");

				rc = -ENOMEM;

				goto err;

			}

			nvme_ctrlr->num_ns++;

		}



		assert(num_ns == nvme_ctrlr->num_ns);

		nvme_ctrlr->num_ns = num_ns;

	}



	trid_entry = calloc(1, sizeof(*trid_entry));
@@ -2383,7 +2388,7 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_ctrlr *nvme_ctrlr, 
	for (i = 0; i < nvme_ctrlr->num_ns; i++) {

		nsid = i + 1;

		nvme_ns = nvme_ctrlr->namespaces[nsid - 1];

		if (!nvme_ns->populated) {

		if (nvme_ns == NULL) {

			continue;

		}

		assert(nvme_ns->id == nsid);
@@ -2450,7 +2455,7 @@ bdev_nvme_compare_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
		nsid = i + 1;



		nvme_ns = nvme_ctrlr->namespaces[i];

		if (!nvme_ns->populated) {

		if (nvme_ns == NULL) {

			continue;

		}

module/bdev/nvme/common.h

+0 −7
Original line number Diff line number Diff line
@@ -67,13 +67,6 @@ struct nvme_async_probe_ctx { 


struct nvme_ns {

	uint32_t		id;

	/** Marks whether this data structure has its bdevs

	 *  populated for the associated namespace.  It is used

	 *  to keep track if we need manage the populated

	 *  resources when a newly active namespace is found,

	 *  or when a namespace becomes inactive.

	 */

	bool			populated;

	struct spdk_nvme_ns	*ns;

	struct nvme_ctrlr	*ctrlr;

	struct nvme_bdev	*bdev;

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+17 −13
Original line number Diff line number Diff line
@@ -727,6 +727,10 @@ ut_create_ana_log_page(struct spdk_nvme_ctrlr *ctrlr, char *buf, uint32_t length 
	for (i = 0; i < ctrlr->num_ns; i++) {

		ns = &ctrlr->ns[i];



		if (!ns->is_active) {

			continue;

		}



		memset(ana_desc, 0, UT_ANA_DESC_SIZE);



		ana_desc->ana_group_id = ns->id;
@@ -1830,10 +1834,10 @@ test_aer_cb(void) 
	SPDK_CU_ASSERT_FATAL(nvme_ctrlr != NULL);



	CU_ASSERT(nvme_ctrlr->num_ns == 4);

	CU_ASSERT(nvme_ctrlr->namespaces[0]->populated == false);

	CU_ASSERT(nvme_ctrlr->namespaces[1]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[2]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[3]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[0] == NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[1] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[2] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[3] != NULL);



	bdev = nvme_ctrlr->namespaces[3]->bdev;

	SPDK_CU_ASSERT_FATAL(bdev != NULL);
@@ -1852,10 +1856,10 @@ test_aer_cb(void) 


	aer_cb(nvme_ctrlr, &cpl);



	CU_ASSERT(nvme_ctrlr->namespaces[0]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[1]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[2]->populated == false);

	CU_ASSERT(nvme_ctrlr->namespaces[3]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[0] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[1] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[2] == NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[3] != NULL);

	CU_ASSERT(bdev->disk.blockcnt == 2048);



	/* Change ANA state of active namespaces. */
@@ -2607,11 +2611,11 @@ test_init_ana_log_page(void) 
	SPDK_CU_ASSERT_FATAL(nvme_ctrlr != NULL);



	CU_ASSERT(nvme_ctrlr->num_ns == 5);

	CU_ASSERT(nvme_ctrlr->namespaces[0]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[1]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[2]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[3]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[4]->populated == true);

	CU_ASSERT(nvme_ctrlr->namespaces[0] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[1] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[2] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[3] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[4] != NULL);

	CU_ASSERT(nvme_ctrlr->namespaces[0]->ana_state == SPDK_NVME_ANA_OPTIMIZED_STATE);

	CU_ASSERT(nvme_ctrlr->namespaces[1]->ana_state == SPDK_NVME_ANA_NON_OPTIMIZED_STATE);

	CU_ASSERT(nvme_ctrlr->namespaces[2]->ana_state == SPDK_NVME_ANA_INACCESSIBLE_STATE);