Commit 1804f3c5 authored by Daniel Verkamp's avatar Daniel Verkamp Committed by Jim Harris
Browse files

nvme: move init_ctrlrs out of multi-process struct 



Each process should manage its own list of controllers that are
initializing; the list doesn't need to be shared between processes.

This is the first step toward preventing non-PCI controllers from being
added into the shared attached_ctrlrs list.

Change-Id: Ia6f85fe89e28a04f0950da5362bb2f49d1b76da9
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/388695


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 1c4bdde9

lib/nvme/nvme.c

+16 −7
Original line number Diff line number Diff line
@@ -45,6 +45,16 @@ int32_t spdk_nvme_retry_count; 
/* gross timeout of 180 seconds in milliseconds */

static int g_nvme_driver_timeout_ms = 3 * 60 * 1000;



static TAILQ_HEAD(, spdk_nvme_ctrlr) g_nvme_init_ctrlrs =

	TAILQ_HEAD_INITIALIZER(g_nvme_init_ctrlrs);



/* Caller must hold g_spdk_nvme_driver->lock */

void

nvme_ctrlr_connected(struct spdk_nvme_ctrlr *ctrlr)

{

	TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, ctrlr, tailq);

}



int

spdk_nvme_detach(struct spdk_nvme_ctrlr *ctrlr)

{
@@ -300,7 +310,6 @@ nvme_driver_init(void) 


	g_spdk_nvme_driver->initialized = false;



	TAILQ_INIT(&g_spdk_nvme_driver->init_ctrlrs);

	TAILQ_INIT(&g_spdk_nvme_driver->attached_ctrlrs);



	SPDK_STATIC_ASSERT(sizeof(host_id) == sizeof(g_spdk_nvme_driver->default_extended_host_id),
@@ -329,7 +338,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid, void *devhandle, 
			return -1;

		}



		TAILQ_INSERT_TAIL(&g_spdk_nvme_driver->init_ctrlrs, ctrlr, tailq);

		TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, ctrlr, tailq);

		return 0;

	}
@@ -345,9 +354,9 @@ nvme_init_controllers(void *cb_ctx, spdk_nvme_attach_cb attach_cb) 


	nvme_robust_mutex_lock(&g_spdk_nvme_driver->lock);



	/* Initialize all new controllers in the init_ctrlrs list in parallel. */

	while (!TAILQ_EMPTY(&g_spdk_nvme_driver->init_ctrlrs)) {

		TAILQ_FOREACH_SAFE(ctrlr, &g_spdk_nvme_driver->init_ctrlrs, tailq, ctrlr_tmp) {

	/* Initialize all new controllers in the g_nvme_init_ctrlrs list in parallel. */

	while (!TAILQ_EMPTY(&g_nvme_init_ctrlrs)) {

		TAILQ_FOREACH_SAFE(ctrlr, &g_nvme_init_ctrlrs, tailq, ctrlr_tmp) {

			/* Drop the driver lock while calling nvme_ctrlr_process_init()

			 *  since it needs to acquire the driver lock internally when calling

			 *  nvme_ctrlr_start().
@@ -362,7 +371,7 @@ nvme_init_controllers(void *cb_ctx, spdk_nvme_attach_cb attach_cb) 


			if (start_rc) {

				/* Controller failed to initialize. */

				TAILQ_REMOVE(&g_spdk_nvme_driver->init_ctrlrs, ctrlr, tailq);

				TAILQ_REMOVE(&g_nvme_init_ctrlrs, ctrlr, tailq);

				nvme_ctrlr_destruct(ctrlr);

				rc = -1;

				break;
@@ -373,7 +382,7 @@ nvme_init_controllers(void *cb_ctx, spdk_nvme_attach_cb attach_cb) 
				 * Controller has been initialized.

				 *  Move it to the attached_ctrlrs list.

				 */

				TAILQ_REMOVE(&g_spdk_nvme_driver->init_ctrlrs, ctrlr, tailq);

				TAILQ_REMOVE(&g_nvme_init_ctrlrs, ctrlr, tailq);

				TAILQ_INSERT_TAIL(&g_spdk_nvme_driver->attached_ctrlrs, ctrlr, tailq);



				/*

lib/nvme/nvme_internal.h

+1 −1
Original line number Diff line number Diff line
@@ -465,7 +465,6 @@ struct spdk_nvme_ctrlr { 


struct nvme_driver {

	pthread_mutex_t			lock;

	TAILQ_HEAD(, spdk_nvme_ctrlr)	init_ctrlrs;

	TAILQ_HEAD(, spdk_nvme_ctrlr)	attached_ctrlrs;

	bool				initialized;

	uint8_t				default_extended_host_id[16];
@@ -562,6 +561,7 @@ void nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr); 
void	nvme_ctrlr_fail(struct spdk_nvme_ctrlr *ctrlr, bool hot_remove);

int	nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr);

int	nvme_ctrlr_start(struct spdk_nvme_ctrlr *ctrlr);

void	nvme_ctrlr_connected(struct spdk_nvme_ctrlr *ctrlr);



int	nvme_ctrlr_submit_admin_request(struct spdk_nvme_ctrlr *ctrlr,

					struct nvme_request *req);

lib/nvme/nvme_rdma.c

+1 −1
Original line number Diff line number Diff line
@@ -1238,7 +1238,7 @@ nvme_rdma_ctrlr_scan(const struct spdk_nvme_transport_id *discovery_trid, 
	if (direct_connect == true) {

		/* Set the ready state to skip the normal init process */

		discovery_ctrlr->state = NVME_CTRLR_STATE_READY;

		TAILQ_INSERT_TAIL(&g_spdk_nvme_driver->init_ctrlrs, discovery_ctrlr, tailq);

		nvme_ctrlr_connected(discovery_ctrlr);

		nvme_ctrlr_add_process(discovery_ctrlr, 0);

		return 0;

	}

test/unit/lib/nvme/nvme.c/nvme_ut.c

+9 −9
Original line number Diff line number Diff line
@@ -176,7 +176,7 @@ test_spdk_nvme_probe(void) 


	/* driver init passes, transport available, we are primary */

	MOCK_SET(spdk_process_is_primary, bool, true);

	TAILQ_INIT(&dummy.init_ctrlrs);

	TAILQ_INIT(&g_nvme_init_ctrlrs);

	rc = spdk_nvme_probe(trid, cb_ctx, probe_cb, attach_cb, remove_cb);

	CU_ASSERT(rc == 0);
@@ -203,8 +203,8 @@ test_nvme_init_controllers(void) 
	memset(&ctrlr, 0, sizeof(struct spdk_nvme_ctrlr));

	CU_ASSERT(pthread_mutexattr_init(&attr) == 0);

	CU_ASSERT(pthread_mutex_init(&test_driver.lock, &attr) == 0);

	TAILQ_INIT(&test_driver.init_ctrlrs);

	TAILQ_INSERT_TAIL(&test_driver.init_ctrlrs, &ctrlr, tailq);

	TAILQ_INIT(&g_nvme_init_ctrlrs);

	TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, &ctrlr, tailq);

	TAILQ_INIT(&test_driver.attached_ctrlrs);



	/*
@@ -217,7 +217,7 @@ test_nvme_init_controllers(void) 
	rc = nvme_init_controllers(cb_ctx, attach_cb);

	CU_ASSERT(rc == -1);

	CU_ASSERT(g_spdk_nvme_driver->initialized == true);

	CU_ASSERT(TAILQ_EMPTY(&g_spdk_nvme_driver->init_ctrlrs));

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_init_ctrlrs));

	CU_ASSERT(ut_destruct_called == true);



	/*
@@ -225,13 +225,13 @@ test_nvme_init_controllers(void) 
	 * forward by setting the ctrl state so that it can be moved

	 * the attached_ctrlrs list.

	 */

	TAILQ_INSERT_TAIL(&test_driver.init_ctrlrs, &ctrlr, tailq);

	TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, &ctrlr, tailq);

	ctrlr.state = NVME_CTRLR_STATE_READY;

	MOCK_SET(nvme_ctrlr_process_init, int, 0);

	rc = nvme_init_controllers(cb_ctx, attach_cb);

	CU_ASSERT(rc == 0);

	CU_ASSERT(ut_attach_cb_called == true);

	CU_ASSERT(TAILQ_EMPTY(&g_spdk_nvme_driver->init_ctrlrs));

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_init_ctrlrs));

	CU_ASSERT(TAILQ_FIRST(&g_spdk_nvme_driver->attached_ctrlrs) == &ctrlr);



	g_spdk_nvme_driver = NULL;
@@ -307,7 +307,7 @@ test_nvme_driver_init(void) 
	g_spdk_nvme_driver = NULL;

	rc = nvme_driver_init();

	CU_ASSERT(g_spdk_nvme_driver->initialized == false);

	CU_ASSERT(TAILQ_EMPTY(&g_spdk_nvme_driver->init_ctrlrs));

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_init_ctrlrs));

	CU_ASSERT(TAILQ_EMPTY(&g_spdk_nvme_driver->attached_ctrlrs));

	CU_ASSERT(rc == 0);
@@ -622,10 +622,10 @@ test_nvme_ctrlr_probe(void) 
	MOCK_SET(dummy_probe_cb, bool, true);

	MOCK_SET_P(nvme_transport_ctrlr_construct,

		   struct spdk_nvme_ctrlr *, &ut_nvme_transport_ctrlr_construct);

	TAILQ_INIT(&g_spdk_nvme_driver->init_ctrlrs);

	TAILQ_INIT(&g_nvme_init_ctrlrs);

	rc = nvme_ctrlr_probe(trid, devhandle, dummy_probe_cb, cb_ctx);

	CU_ASSERT(rc == 0);

	dummy = TAILQ_FIRST(&g_spdk_nvme_driver->init_ctrlrs);

	dummy = TAILQ_FIRST(&g_nvme_init_ctrlrs);

	CU_ASSERT(dummy == &ut_nvme_transport_ctrlr_construct);



	free(g_spdk_nvme_driver);