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

bdev/nvme: Don't rely on knowing ctrlr->num_ns in 


nvme_ctrlr_populate_namespaces

Avoid relying on this number. Different targets have interpreted its
meaning in different ways and it cannot be used anymore in practice. It
may also be very, very large.

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


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 avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 050346e0

module/bdev/nvme/bdev_nvme.c

+24 −15
Original line number Diff line number Diff line
@@ -1758,13 +1758,12 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
			       struct nvme_async_probe_ctx *ctx)

{

	struct spdk_nvme_ctrlr	*ctrlr = nvme_ctrlr->ctrlr;

	struct nvme_ns	*nvme_ns;

	struct nvme_ns	*nvme_ns, *next;

	struct spdk_nvme_ns	*ns;

	struct nvme_bdev	*bdev;

	uint32_t		i;

	uint32_t		nsid;

	int			rc;

	uint64_t		num_sectors;

	bool			ns_is_active;



	if (ctx) {

		/* Initialize this count to 1 to handle the populate functions
@@ -1773,21 +1772,21 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
		ctx->populates_in_progress = 1;

	}



	for (i = 0; i < nvme_ctrlr->num_ns; i++) {

		uint32_t	nsid = i + 1;



		nvme_ns = nvme_ctrlr_get_ns(nvme_ctrlr, nsid);

		ns_is_active = spdk_nvme_ctrlr_is_active_ns(ctrlr, nsid);

	/* First loop over our existing namespaces and see if they have been

	 * removed. */

	nvme_ns = nvme_ctrlr_get_first_active_ns(nvme_ctrlr);

	while (nvme_ns != NULL) {

		next = nvme_ctrlr_get_next_active_ns(nvme_ctrlr, nvme_ns);



		if (nvme_ns != NULL && ns_is_active) {

		if (spdk_nvme_ctrlr_is_active_ns(ctrlr, nvme_ns->id)) {

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

			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);

			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nvme_ns->id);

			num_sectors = spdk_nvme_ns_get_num_sectors(ns);

			bdev = nvme_ns->bdev;

			assert(bdev != NULL);

			if (bdev->disk.blockcnt != num_sectors) {

				SPDK_NOTICELOG("NSID %u is resized: bdev name %s, old size %" PRIu64 ", new size %" PRIu64 "\n",

					       nsid,

					       nvme_ns->id,

					       bdev->disk.name,

					       bdev->disk.blockcnt,

					       num_sectors);
@@ -1797,9 +1796,21 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
						    bdev->disk.name, rc);

				}

			}

		} else {

			/* Namespace was removed */

			nvme_ctrlr_depopulate_namespace(nvme_ctrlr, nvme_ns);

		}



		if (nvme_ns == NULL && ns_is_active) {

		nvme_ns = next;

	}



	/* Loop through all of the namespaces at the nvme level and see if any of them are new */

	nsid = spdk_nvme_ctrlr_get_first_active_ns(ctrlr);

	while (nsid != 0) {

		nvme_ns = nvme_ctrlr_get_ns(nvme_ctrlr, nsid);



		if (nvme_ns == NULL) {

			/* Found a new one */

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

			if (nvme_ns == NULL) {

				SPDK_ERRLOG("Failed to allocate namespace\n");
@@ -1820,9 +1831,7 @@ nvme_ctrlr_populate_namespaces(struct nvme_ctrlr *nvme_ctrlr, 
			nvme_ctrlr_populate_namespace(nvme_ctrlr, nvme_ns, ctx);

		}



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

			nvme_ctrlr_depopulate_namespace(nvme_ctrlr, nvme_ns);

		}

		nsid = spdk_nvme_ctrlr_get_next_active_ns(ctrlr, nsid);

	}



	if (ctx) {

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

+26 −0
Original line number Diff line number Diff line
@@ -271,6 +271,32 @@ struct spdk_nvme_ctrlr_reset_ctx { 
	struct spdk_nvme_ctrlr		*ctrlr;

};



uint32_t

spdk_nvme_ctrlr_get_first_active_ns(struct spdk_nvme_ctrlr *ctrlr)

{

	uint32_t nsid;



	for (nsid = 1; nsid <= ctrlr->num_ns; nsid++) {

		if (ctrlr->ns[nsid - 1].is_active) {

			return nsid;

		}

	}



	return 0;

}



uint32_t

spdk_nvme_ctrlr_get_next_active_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid)

{

	for (nsid = nsid + 1; nsid <= ctrlr->num_ns; nsid++) {

		if (ctrlr->ns[nsid - 1].is_active) {

			return nsid;

		}

	}



	return 0;

}



static TAILQ_HEAD(, spdk_nvme_ctrlr) g_ut_init_ctrlrs = TAILQ_HEAD_INITIALIZER(g_ut_init_ctrlrs);

static TAILQ_HEAD(, spdk_nvme_ctrlr) g_ut_attached_ctrlrs = TAILQ_HEAD_INITIALIZER(

			g_ut_attached_ctrlrs);