Commit 2ca71169 authored by Evgeniy Kochetov's avatar Evgeniy Kochetov Committed by Tomasz Zawadzki
Browse files

nvme/ctrlr: Remove Get Num Queues initialization step 



NVMe specification in ch.7.6 "Controller Initialization" suggests to
use only Set Features "Number of queues" command and says nothing
about Get Features. All required information is available after Set
Num Queues step.

Fixes #1270

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


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
parent 046bdc4a

lib/nvme/nvme_ctrlr.c

+15 −57
Original line number Diff line number Diff line
@@ -903,10 +903,6 @@ 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_GET_NUM_QUEUES:

		return "get number of queues";

	case NVME_CTRLR_STATE_WAIT_FOR_GET_NUM_QUEUES:

		return "wait for get number of queues";

	case NVME_CTRLR_STATE_CONSTRUCT_NS:

		return "construct namespaces";

	case NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS:
@@ -1522,18 +1518,6 @@ nvme_ctrlr_identify_id_desc_namespaces(struct spdk_nvme_ctrlr *ctrlr) 
	return rc;

}



static void

nvme_ctrlr_set_num_queues_done(void *arg, const struct spdk_nvme_cpl *cpl)

{

	struct spdk_nvme_ctrlr *ctrlr = (struct spdk_nvme_ctrlr *)arg;



	if (spdk_nvme_cpl_is_error(cpl)) {

		SPDK_ERRLOG("Set Features - Number of Queues failed!\n");

	}

	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_GET_NUM_QUEUES,

			     ctrlr->opts.admin_timeout_ms);

}



static void

nvme_ctrlr_update_nvmf_ioccsz(struct spdk_nvme_ctrlr *ctrlr)

{
@@ -1551,41 +1535,14 @@ nvme_ctrlr_update_nvmf_ioccsz(struct spdk_nvme_ctrlr *ctrlr) 
	}

}



static int

nvme_ctrlr_set_num_queues(struct spdk_nvme_ctrlr *ctrlr)

{

	int rc;



	if (ctrlr->opts.num_io_queues > SPDK_NVME_MAX_IO_QUEUES) {

		SPDK_NOTICELOG("Limiting requested num_io_queues %u to max %d\n",

			       ctrlr->opts.num_io_queues, SPDK_NVME_MAX_IO_QUEUES);

		ctrlr->opts.num_io_queues = SPDK_NVME_MAX_IO_QUEUES;

	} else if (ctrlr->opts.num_io_queues < 1) {

		SPDK_NOTICELOG("Requested num_io_queues 0, increasing to 1\n");

		ctrlr->opts.num_io_queues = 1;

	}



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES,

			     ctrlr->opts.admin_timeout_ms);



	rc = nvme_ctrlr_cmd_set_num_queues(ctrlr, ctrlr->opts.num_io_queues,

					   nvme_ctrlr_set_num_queues_done, ctrlr);

	if (rc != 0) {

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);

		return rc;

	}



	return 0;

}



static void

nvme_ctrlr_get_num_queues_done(void *arg, const struct spdk_nvme_cpl *cpl)

nvme_ctrlr_set_num_queues_done(void *arg, const struct spdk_nvme_cpl *cpl)

{

	uint32_t cq_allocated, sq_allocated, min_allocated, i;

	struct spdk_nvme_ctrlr *ctrlr = (struct spdk_nvme_ctrlr *)arg;



	if (spdk_nvme_cpl_is_error(cpl)) {

		SPDK_ERRLOG("Get Features - Number of Queues failed!\n");

		SPDK_ERRLOG("Set Features - Number of Queues failed!\n");

		ctrlr->opts.num_io_queues = 0;

	} else {

		/*
@@ -1622,15 +1579,24 @@ nvme_ctrlr_get_num_queues_done(void *arg, const struct spdk_nvme_cpl *cpl) 
}



static int

nvme_ctrlr_get_num_queues(struct spdk_nvme_ctrlr *ctrlr)

nvme_ctrlr_set_num_queues(struct spdk_nvme_ctrlr *ctrlr)

{

	int rc;



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_WAIT_FOR_GET_NUM_QUEUES,

	if (ctrlr->opts.num_io_queues > SPDK_NVME_MAX_IO_QUEUES) {

		SPDK_NOTICELOG("Limiting requested num_io_queues %u to max %d\n",

			       ctrlr->opts.num_io_queues, SPDK_NVME_MAX_IO_QUEUES);

		ctrlr->opts.num_io_queues = SPDK_NVME_MAX_IO_QUEUES;

	} else if (ctrlr->opts.num_io_queues < 1) {

		SPDK_NOTICELOG("Requested num_io_queues 0, increasing to 1\n");

		ctrlr->opts.num_io_queues = 1;

	}



	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES,

			     ctrlr->opts.admin_timeout_ms);



	/* Obtain the number of queues allocated using Get Features. */

	rc = nvme_ctrlr_cmd_get_num_queues(ctrlr, nvme_ctrlr_get_num_queues_done, ctrlr);

	rc = nvme_ctrlr_cmd_set_num_queues(ctrlr, ctrlr->opts.num_io_queues,

					   nvme_ctrlr_set_num_queues_done, ctrlr);

	if (rc != 0) {

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);

		return rc;
@@ -2452,14 +2418,6 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) 
		spdk_nvme_qpair_process_completions(ctrlr->adminq, 0);

		break;



	case NVME_CTRLR_STATE_GET_NUM_QUEUES:

		rc = nvme_ctrlr_get_num_queues(ctrlr);

		break;



	case NVME_CTRLR_STATE_WAIT_FOR_GET_NUM_QUEUES:

		spdk_nvme_qpair_process_completions(ctrlr->adminq, 0);

		break;



	case NVME_CTRLR_STATE_CONSTRUCT_NS:

		rc = nvme_ctrlr_construct_namespaces(ctrlr);

		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS,

lib/nvme/nvme_internal.h

+0 −10
Original line number Diff line number Diff line
@@ -492,16 +492,6 @@ enum nvme_ctrlr_state { 
	 */

	NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES,



	/**

	 * Get Number of Queues of the controller.

	 */

	NVME_CTRLR_STATE_GET_NUM_QUEUES,



	/**

	 * Waiting for Get Num of Queues command to be completed.

	 */

	NVME_CTRLR_STATE_WAIT_FOR_GET_NUM_QUEUES,



	/**

	 * Construct Namespace data structures of the controller.

	 */

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

+17 −7
Original line number Diff line number Diff line
@@ -362,8 +362,8 @@ int 
nvme_ctrlr_cmd_get_num_queues(struct spdk_nvme_ctrlr *ctrlr,

			      spdk_nvme_cmd_cb cb_fn, void *cb_arg)

{

	fake_cpl_success(cb_fn, cb_arg);

	return 0;

	CU_ASSERT(0);

	return -1;

}



int
@@ -1982,11 +1982,13 @@ 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_GET_NUM_QUEUES);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 0);

	CU_ASSERT(ctrlr.icdoff == 0);



	nvme_ctrlr_destruct(&ctrlr);



	/* Check RDMA trtype, */

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_RDMA;
@@ -1994,13 +1996,15 @@ 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_GET_NUM_QUEUES);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);

	CU_ASSERT(ctrlr.icdoff == 1);

	ctrlr.ioccsz_bytes = 0;

	ctrlr.icdoff = 0;



	nvme_ctrlr_destruct(&ctrlr);



	/* Check TCP trtype, */

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_TCP;
@@ -2008,13 +2012,15 @@ 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_GET_NUM_QUEUES);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);

	CU_ASSERT(ctrlr.icdoff == 1);

	ctrlr.ioccsz_bytes = 0;

	ctrlr.icdoff = 0;



	nvme_ctrlr_destruct(&ctrlr);



	/* Check FC trtype, */

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_FC;
@@ -2022,13 +2028,15 @@ 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_GET_NUM_QUEUES);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 4096);

	CU_ASSERT(ctrlr.icdoff == 1);

	ctrlr.ioccsz_bytes = 0;

	ctrlr.icdoff = 0;



	nvme_ctrlr_destruct(&ctrlr);



	/* Check CUSTOM trtype, */

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_CUSTOM;
@@ -2036,10 +2044,12 @@ 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_GET_NUM_QUEUES);

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONSTRUCT_NS);



	CU_ASSERT(ctrlr.ioccsz_bytes == 0);

	CU_ASSERT(ctrlr.icdoff == 0);



	nvme_ctrlr_destruct(&ctrlr);

}



int main(int argc, char **argv)