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

nvme: add a per-process attached_ctrlrs list 



Only multi-process shared controllers should be inserted into the shared
list in g_spdk_nvme_driver.  To accomplish this, create a second
per-process global list of attached controllers (g_nvme_attached_ctrlrs)
and rename the driver struct field to shared_attached_ctrlrs to clarify
its purpose.  Additionally, a new helper function, nvme_ctrlr_shared(),
returns whether a given controller should be on the shared or
per-process list.

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


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

lib/nvme/nvme.c

+33 −7
Original line number Diff line number Diff line
@@ -48,6 +48,17 @@ 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);



/* Per-process attached controller list */

static TAILQ_HEAD(, spdk_nvme_ctrlr) g_nvme_attached_ctrlrs =

	TAILQ_HEAD_INITIALIZER(g_nvme_attached_ctrlrs);



/* Returns true if ctrlr should be stored on the multi-process shared_attached_ctrlrs list */

static bool

nvme_ctrlr_shared(const struct spdk_nvme_ctrlr *ctrlr)

{

	return ctrlr->trid.trtype == SPDK_NVME_TRANSPORT_PCIE;

}



/* Caller must hold g_spdk_nvme_driver->lock */

void

nvme_ctrlr_connected(struct spdk_nvme_ctrlr *ctrlr)
@@ -63,7 +74,11 @@ spdk_nvme_detach(struct spdk_nvme_ctrlr *ctrlr) 
	nvme_ctrlr_proc_put_ref(ctrlr);



	if (nvme_ctrlr_get_ref_count(ctrlr) == 0) {

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

		if (nvme_ctrlr_shared(ctrlr)) {

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

		} else {

			TAILQ_REMOVE(&g_nvme_attached_ctrlrs, ctrlr, tailq);

		}

		nvme_ctrlr_destruct(ctrlr);

	}
@@ -310,7 +325,7 @@ nvme_driver_init(void) 


	g_spdk_nvme_driver->initialized = false;



	TAILQ_INIT(&g_spdk_nvme_driver->attached_ctrlrs);

	TAILQ_INIT(&g_spdk_nvme_driver->shared_attached_ctrlrs);



	SPDK_STATIC_ASSERT(sizeof(host_id) == sizeof(g_spdk_nvme_driver->default_extended_host_id),

			   "host ID size mismatch");
@@ -383,7 +398,11 @@ nvme_init_controllers(void *cb_ctx, spdk_nvme_attach_cb attach_cb) 
				 *  Move it to the attached_ctrlrs list.

				 */

				TAILQ_REMOVE(&g_nvme_init_ctrlrs, ctrlr, tailq);

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

				if (nvme_ctrlr_shared(ctrlr)) {

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

				} else {

					TAILQ_INSERT_TAIL(&g_nvme_attached_ctrlrs, ctrlr, tailq);

				}



				/*

				 * Increase the ref count before calling attach_cb() as the user may
@@ -431,7 +450,15 @@ spdk_nvme_get_ctrlr_by_trid_unsafe(const struct spdk_nvme_transport_id *trid) 
{

	struct spdk_nvme_ctrlr *ctrlr;



	TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->attached_ctrlrs, tailq) {

	/* Search per-process list */

	TAILQ_FOREACH(ctrlr, &g_nvme_attached_ctrlrs, tailq) {

		if (spdk_nvme_transport_id_compare(&ctrlr->trid, trid) == 0) {

			return ctrlr;

		}

	}



	/* Search multi-process shared list */

	TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->shared_attached_ctrlrs, tailq) {

		if (spdk_nvme_transport_id_compare(&ctrlr->trid, trid) == 0) {

			return ctrlr;

		}
@@ -460,11 +487,10 @@ spdk_nvme_probe_internal(const struct spdk_nvme_transport_id *trid, void *cb_ctx 
	nvme_transport_ctrlr_scan(trid, cb_ctx, probe_cb, remove_cb, direct_connect);



	/*

	 * The RDMA trtype will always construct the ctrlr and go through the

	 *  normal process.

	 * Probe controllers on the shared_attached_ctrlrs list

	 */

	if (!spdk_process_is_primary() && (trid->trtype == SPDK_NVME_TRANSPORT_PCIE)) {

		TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->attached_ctrlrs, tailq) {

		TAILQ_FOREACH(ctrlr, &g_spdk_nvme_driver->shared_attached_ctrlrs, tailq) {

			/* Do not attach other ctrlrs if user specify a valid trid */

			if ((strlen(trid->traddr) != 0) &&

			    (spdk_nvme_transport_id_compare(trid, &ctrlr->trid))) {

lib/nvme/nvme_internal.h

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


struct nvme_driver {

	pthread_mutex_t			lock;

	TAILQ_HEAD(, spdk_nvme_ctrlr)	attached_ctrlrs;



	/** Multi-process shared attached controller list */

	TAILQ_HEAD(, spdk_nvme_ctrlr)	shared_attached_ctrlrs;



	bool				initialized;

	uint8_t				default_extended_host_id[16];

};

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

+48 −11
Original line number Diff line number Diff line
@@ -165,8 +165,8 @@ test_spdk_nvme_probe(void) 
	memset(&ctrlr, 0, sizeof(struct spdk_nvme_ctrlr));

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

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

	TAILQ_INIT(&dummy.attached_ctrlrs);

	TAILQ_INSERT_TAIL(&dummy.attached_ctrlrs, &ctrlr, tailq);

	TAILQ_INIT(&dummy.shared_attached_ctrlrs);

	TAILQ_INSERT_TAIL(&dummy.shared_attached_ctrlrs, &ctrlr, tailq);

	MOCK_SET(attach_cb_called, bool, false);

	/* setup nvme_transport_ctrlr_scan() stub to also check the trype */

	MOCK_SET(check_trtype, bool, true);
@@ -201,11 +201,12 @@ test_nvme_init_controllers(void) 


	g_spdk_nvme_driver = &test_driver;

	memset(&ctrlr, 0, sizeof(struct spdk_nvme_ctrlr));

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE;

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

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

	TAILQ_INIT(&g_nvme_init_ctrlrs);

	TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, &ctrlr, tailq);

	TAILQ_INIT(&test_driver.attached_ctrlrs);

	TAILQ_INIT(&test_driver.shared_attached_ctrlrs);



	/*

	 * Try to initialize, but nvme_ctrlr_process_init will fail.
@@ -223,7 +224,7 @@ test_nvme_init_controllers(void) 
	/*

	 * Controller init OK, need to move the controller state machine

	 * forward by setting the ctrl state so that it can be moved

	 * the attached_ctrlrs list.

	 * the shared_attached_ctrlrs list.

	 */

	TAILQ_INSERT_TAIL(&g_nvme_init_ctrlrs, &ctrlr, tailq);

	ctrlr.state = NVME_CTRLR_STATE_READY;
@@ -232,7 +233,25 @@ test_nvme_init_controllers(void) 
	CU_ASSERT(rc == 0);

	CU_ASSERT(ut_attach_cb_called == true);

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_init_ctrlrs));

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

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_attached_ctrlrs));

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

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



	/*

	 * Non-PCIe controllers should be added to the per-process list, not the shared list.

	 */

	memset(&ctrlr, 0, sizeof(struct spdk_nvme_ctrlr));

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_RDMA;

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

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

	CU_ASSERT(TAILQ_FIRST(&g_nvme_attached_ctrlrs) == &ctrlr);

	TAILQ_REMOVE(&g_nvme_attached_ctrlrs, &ctrlr, tailq);



	g_spdk_nvme_driver = NULL;

	pthread_mutexattr_destroy(&attr);
@@ -308,7 +327,7 @@ test_nvme_driver_init(void) 
	rc = nvme_driver_init();

	CU_ASSERT(g_spdk_nvme_driver->initialized == false);

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_init_ctrlrs));

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

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

	CU_ASSERT(rc == 0);



	g_spdk_nvme_driver = NULL;
@@ -324,9 +343,12 @@ test_spdk_nvme_detach(void) 
	struct spdk_nvme_ctrlr *ret_ctrlr;

	struct nvme_driver test_driver;



	memset(&ctrlr, 0, sizeof(ctrlr));

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE;



	g_spdk_nvme_driver = &test_driver;

	TAILQ_INIT(&test_driver.attached_ctrlrs);

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

	TAILQ_INIT(&test_driver.shared_attached_ctrlrs);

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

	CU_ASSERT_FATAL(pthread_mutex_init(&test_driver.lock, NULL) == 0);



	/*
@@ -338,7 +360,7 @@ test_spdk_nvme_detach(void) 
	 */

	MOCK_SET(nvme_ctrlr_get_ref_count, int, 0);

	rc = spdk_nvme_detach(&ctrlr);

	ret_ctrlr = TAILQ_FIRST(&test_driver.attached_ctrlrs);

	ret_ctrlr = TAILQ_FIRST(&test_driver.shared_attached_ctrlrs);

	CU_ASSERT(ret_ctrlr == NULL);

	CU_ASSERT(ut_destruct_called == true);

	CU_ASSERT(rc == 0);
@@ -349,14 +371,29 @@ test_spdk_nvme_detach(void) 
	 * not empty.

	 */

	MOCK_SET(nvme_ctrlr_get_ref_count, int, 1);

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

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

	ut_destruct_called = false;

	rc = spdk_nvme_detach(&ctrlr);

	ret_ctrlr = TAILQ_FIRST(&test_driver.attached_ctrlrs);

	ret_ctrlr = TAILQ_FIRST(&test_driver.shared_attached_ctrlrs);

	CU_ASSERT(ret_ctrlr != NULL);

	CU_ASSERT(ut_destruct_called == false);

	CU_ASSERT(rc == 0);



	/*

	 * Non-PCIe controllers should be on the per-process attached_ctrlrs list, not the

	 * shared_attached_ctrlrs list.  Test an RDMA controller and ensure it is removed

	 * from the correct list.

	 */

	memset(&ctrlr, 0, sizeof(ctrlr));

	ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_RDMA;

	TAILQ_INIT(&g_nvme_attached_ctrlrs);

	TAILQ_INSERT_TAIL(&g_nvme_attached_ctrlrs, &ctrlr, tailq);

	MOCK_SET(nvme_ctrlr_get_ref_count, int, 0);

	rc = spdk_nvme_detach(&ctrlr);

	CU_ASSERT(TAILQ_EMPTY(&g_nvme_attached_ctrlrs));

	CU_ASSERT(ut_destruct_called == true);

	CU_ASSERT(rc == 0);



	g_spdk_nvme_driver = NULL;

	pthread_mutex_destroy(&test_driver.lock);

}