Commit 65ff0771 authored by Evgeniy Kochetov's avatar Evgeniy Kochetov Committed by Tomasz Zawadzki
Browse files

nvme/ctrlr: Retrieve active NS list before NS construct 



This patch changes the order of IDENTIFY_ACTIVE_NS and CONSTRUCT_NS
controller states. It is required to further improve memory management
for namespaces by allocating memory only for active ones.

Signed-off-by: default avatarEvgeniy Kochetov <evgeniik@nvidia.com>
Change-Id: Ie540442b1bd9e897afcbaa4319c139109dd0c515
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6503


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 75ddfd65

lib/nvme/nvme_ctrlr.c

+12 −13
Original line number Diff line number Diff line
@@ -1161,12 +1161,12 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state) 
		return "set number of queues";

	case NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES:

		return "wait for set number of queues";

	case NVME_CTRLR_STATE_CONSTRUCT_NS:

		return "construct namespaces";

	case NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS:

		return "identify active ns";

	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_ACTIVE_NS:

		return "wait for identify active ns";

	case NVME_CTRLR_STATE_CONSTRUCT_NS:

		return "construct namespaces";

	case NVME_CTRLR_STATE_IDENTIFY_NS:

		return "identify ns";

	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_NS:
@@ -1953,7 +1953,7 @@ _nvme_active_ns_ctx_deleter(struct nvme_active_ns_ctx *ctx) 
	assert(ctx->state == NVME_ACTIVE_NS_STATE_DONE);

	nvme_ctrlr_identify_active_ns_swap(ctrlr, &ctx->new_ns_list);

	nvme_active_ns_ctx_destroy(ctx);

	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_NS, ctrlr->opts.admin_timeout_ms);

	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONSTRUCT_NS, ctrlr->opts.admin_timeout_ms);

}



static void
@@ -2332,7 +2332,7 @@ nvme_ctrlr_set_num_queues_done(void *arg, const struct spdk_nvme_cpl *cpl) 
		spdk_nvme_ctrlr_free_qid(ctrlr, i);

	}



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONSTRUCT_NS,

	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS,

			     ctrlr->opts.admin_timeout_ms);

}
@@ -2524,10 +2524,6 @@ nvme_ctrlr_destruct_namespaces(struct spdk_nvme_ctrlr *ctrlr) 
		ctrlr->ns = NULL;

		ctrlr->num_ns = 0;

	}



	spdk_free(ctrlr->active_ns_list);

	ctrlr->active_ns_list = NULL;

	ctrlr->max_active_ns_idx = 0;

}



static void
@@ -3240,18 +3236,18 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
		rc = nvme_ctrlr_set_num_queues(ctrlr);

		break;



	case NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS:

		_nvme_ctrlr_identify_active_ns(ctrlr);

		break;



	case NVME_CTRLR_STATE_CONSTRUCT_NS:

		rc = nvme_ctrlr_construct_namespaces(ctrlr);

		if (!rc) {

			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS,

			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_NS,

					     ctrlr->opts.admin_timeout_ms);

		}

		break;



	case NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS:

		_nvme_ctrlr_identify_active_ns(ctrlr);

		break;



	case NVME_CTRLR_STATE_IDENTIFY_NS:

		rc = nvme_ctrlr_identify_namespaces(ctrlr);

		break;
@@ -3485,6 +3481,9 @@ nvme_ctrlr_destruct_poll_async(struct spdk_nvme_ctrlr *ctrlr, 
	}



	nvme_ctrlr_destruct_namespaces(ctrlr);

	spdk_free(ctrlr->active_ns_list);

	ctrlr->active_ns_list = NULL;

	ctrlr->max_active_ns_idx = 0;



	spdk_bit_array_free(&ctrlr->free_io_qids);

lib/nvme/nvme_internal.h

+5 −5
Original line number Diff line number Diff line
@@ -587,11 +587,6 @@ enum nvme_ctrlr_state { 
	 */

	NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES,



	/**

	 * Construct Namespace data structures of the controller.

	 */

	NVME_CTRLR_STATE_CONSTRUCT_NS,



	/**

	 * Get active Namespace list of the controller.

	 */
@@ -602,6 +597,11 @@ enum nvme_ctrlr_state { 
	 */

	NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_ACTIVE_NS,



	/**

	 * Construct Namespace data structures of the controller.

	 */

	NVME_CTRLR_STATE_CONSTRUCT_NS,



	/**

	 * Get Identify Namespace Data structure for each NS.

	 */

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

+20 −10
Original line number Diff line number Diff line
@@ -2083,6 +2083,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) 
	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 0);
@@ -2100,6 +2102,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) 
	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);
@@ -2119,6 +2123,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) 
	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);
@@ -2138,6 +2144,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) 
	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);
@@ -2157,6 +2165,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void) 
	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 0);
@@ -2181,8 +2191,8 @@ test_nvme_ctrlr_init_set_num_queues(void) 
	ctrlr.opts.num_io_queues = 64;

	/* Num queues is zero-based. So, use 31 to get 32 queues */

	fake_cpl.cdw0 = 31 + (31 << 16);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> CONSTRUCT_NS */

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_ACTIVE_NS */

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS);

	CU_ASSERT(ctrlr.opts.num_io_queues == 32);

	fake_cpl.cdw0 = 0;
@@ -2503,8 +2513,8 @@ test_nvme_ctrlr_active_ns_list_v0(void) 
	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);

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> CONSTRUCT_NS */

	SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_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));
@@ -2534,8 +2544,8 @@ test_nvme_ctrlr_active_ns_list_v2(void) 
	/* 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);

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> CONSTRUCT_NS */

	SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_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));
@@ -2553,8 +2563,8 @@ test_nvme_ctrlr_active_ns_list_v2(void) 
	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);

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> CONSTRUCT_NS */

	SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_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));
@@ -2572,8 +2582,8 @@ test_nvme_ctrlr_active_ns_list_v2(void) 
	}



	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);

	SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> CONSTRUCT_NS */

	SPDK_CU_ASSERT_FATAL(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_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));