Commit 7346be69 authored by Ziye Yang's avatar Ziye Yang Committed by Jim Harris
Browse files

nvmf: Make the ctrlr create/remove in subsystem in an asynchronous way 



Ctrlrs list maintanined by the subsystem structure should be operated
by the thread which creates the subsystem. And this will make the
operations correct.

Change-Id: I7f881a77b1846658b3acd4270b74f86816e87803
Signed-off-by: default avatarZiye Yang <optimistyzy@gmail.com>
Reviewed-on: https://review.gerrithub.io/401541


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 6d4c78ea

lib/nvmf/ctrlr.c

+75 −26
Original line number Diff line number Diff line
@@ -94,9 +94,42 @@ _spdk_nvmf_request_complete(void *ctx) 
	spdk_nvmf_request_complete(req);

}



static void

_spdk_nvmf_ctrlr_add_admin_qpair(void *ctx)

{

	struct spdk_nvmf_request *req = ctx;

	struct spdk_nvmf_fabric_connect_rsp *rsp = &req->rsp->connect_rsp;

	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_ctrlr *ctrlr = qpair->ctrlr;



	ctrlr->admin_qpair = qpair;

	ctrlr_add_qpair_and_update_rsp(qpair, ctrlr, rsp);

	spdk_nvmf_request_complete(req);

}



static void

_spdk_nvmf_subsystem_add_ctrlr(void *ctx)

{

	struct spdk_nvmf_request *req = ctx;

	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_fabric_connect_rsp *rsp = &req->rsp->connect_rsp;

	struct spdk_nvmf_ctrlr *ctrlr = qpair->ctrlr;



	if (spdk_nvmf_subsystem_add_ctrlr(ctrlr->subsys, ctrlr)) {

		SPDK_ERRLOG("Unable to add controller to subsystem\n");

		free(ctrlr);

		qpair->ctrlr = NULL;

		rsp->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;

		spdk_thread_send_msg(qpair->group->thread, _spdk_nvmf_request_complete, req);

		return;

	}



	spdk_thread_send_msg(qpair->group->thread, _spdk_nvmf_ctrlr_add_admin_qpair, req);

}



static struct spdk_nvmf_ctrlr *

spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem,

		       struct spdk_nvmf_qpair *admin_qpair,

		       struct spdk_nvmf_request *req,

		       struct spdk_nvmf_fabric_connect_cmd *connect_cmd,

		       struct spdk_nvmf_fabric_connect_data *connect_data)

{
@@ -111,6 +144,7 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, 
		return NULL;

	}



	req->qpair->ctrlr = ctrlr;

	TAILQ_INIT(&ctrlr->qpairs);

	ctrlr->kato = connect_cmd->kato;

	ctrlr->async_event_config.raw = 0;
@@ -146,17 +180,15 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, 
	SPDK_DEBUGLOG(SPDK_LOG_NVMF, "cc 0x%x\n", ctrlr->vcprop.cc.raw);

	SPDK_DEBUGLOG(SPDK_LOG_NVMF, "csts 0x%x\n", ctrlr->vcprop.csts.raw);



	if (spdk_nvmf_subsystem_add_ctrlr(subsystem, ctrlr)) {

		SPDK_ERRLOG("Unable to add controller to subsystem\n");

		free(ctrlr);

		return NULL;

	}

	spdk_thread_send_msg(subsystem->thread, _spdk_nvmf_subsystem_add_ctrlr, req);



	return ctrlr;

}



static void ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr)

static void ctrlr_destruct(void *ctx)

{

	struct spdk_nvmf_ctrlr *ctrlr = ctx;



	spdk_nvmf_subsystem_remove_ctrlr(ctrlr->subsys, ctrlr);

	free(ctrlr);

}
@@ -249,10 +281,41 @@ ctrlr_delete_qpair(void *ctx) 
	spdk_nvmf_transport_qpair_fini(qpair);



	if (ctrlr->num_qpairs == 0) {

		ctrlr_destruct(ctrlr);

		spdk_thread_send_msg(ctrlr->subsys->thread, ctrlr_destruct, ctrlr);

	}

}



static void

_spdk_nvmf_ctrlr_add_io_qpair(void *ctx)

{

	struct spdk_nvmf_request *req = ctx;

	struct spdk_nvmf_fabric_connect_rsp *rsp = &req->rsp->connect_rsp;

	struct spdk_nvmf_fabric_connect_data *data = req->data;

	struct spdk_nvmf_ctrlr *ctrlr;

	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_qpair *admin_qpair;

	struct spdk_nvmf_tgt *tgt = qpair->transport->tgt;

	struct spdk_nvmf_subsystem *subsystem;



	SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Connect I/O Queue for controller id 0x%x\n", data->cntlid);



	subsystem = spdk_nvmf_tgt_find_subsystem(tgt, data->subnqn);

	/* We already checked this in spdk_nvmf_ctrlr_connect */

	assert(subsystem != NULL);



	ctrlr = spdk_nvmf_subsystem_get_ctrlr(subsystem, data->cntlid);

	if (ctrlr == NULL) {

		SPDK_ERRLOG("Unknown controller ID 0x%x\n", data->cntlid);

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, cntlid);

		spdk_thread_send_msg(qpair->group->thread, _spdk_nvmf_request_complete, req);

		return;

	}



	admin_qpair = ctrlr->admin_qpair;

	qpair->ctrlr = ctrlr;

	spdk_thread_send_msg(admin_qpair->group->thread, spdk_nvmf_ctrlr_add_io_qpair, req);

}



static int

spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req)

{
@@ -262,7 +325,6 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req) 
	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_tgt *tgt = qpair->transport->tgt;

	struct spdk_nvmf_ctrlr *ctrlr;

	struct spdk_nvmf_qpair *admin_qpair = NULL;

	struct spdk_nvmf_subsystem *subsystem;

	const char *subnqn, *hostnqn;

	void *end;
@@ -352,29 +414,16 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req) 
		}



		/* Establish a new ctrlr */

		ctrlr = spdk_nvmf_ctrlr_create(subsystem, qpair, cmd, data);

		ctrlr = spdk_nvmf_ctrlr_create(subsystem, req, cmd, data);

		if (!ctrlr) {

			SPDK_ERRLOG("spdk_nvmf_ctrlr_create() failed\n");

			rsp->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		}



		ctrlr->admin_qpair = qpair;

		ctrlr_add_qpair_and_update_rsp(qpair, ctrlr, rsp);

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		} else {

		SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Connect I/O Queue for controller id 0x%x\n", data->cntlid);



		ctrlr = spdk_nvmf_subsystem_get_ctrlr(subsystem, data->cntlid);

		if (ctrlr == NULL) {

			SPDK_ERRLOG("Unknown controller ID 0x%x\n", data->cntlid);

			SPDK_NVMF_INVALID_CONNECT_DATA(rsp, cntlid);

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

			return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS;

		}



		admin_qpair = ctrlr->admin_qpair;

		qpair->ctrlr = ctrlr;

		spdk_thread_send_msg(admin_qpair->group->thread, spdk_nvmf_ctrlr_add_io_qpair, req);

	} else {

		spdk_thread_send_msg(subsystem->thread, _spdk_nvmf_ctrlr_add_io_qpair, req);

		return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS;

	}

}

lib/nvmf/nvmf_internal.h

+1 −0
Original line number Diff line number Diff line
@@ -181,6 +181,7 @@ struct spdk_nvmf_ctrlr { 
};



struct spdk_nvmf_subsystem {

	struct spdk_thread		*thread;

	uint32_t			id;

	enum spdk_nvmf_subsystem_state	state;

lib/nvmf/subsystem.c

+1 −0
Original line number Diff line number Diff line
@@ -255,6 +255,7 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt, 
		return NULL;

	}



	subsystem->thread = spdk_get_thread();

	subsystem->state = SPDK_NVMF_SUBSYSTEM_INACTIVE;

	subsystem->tgt = tgt;

	subsystem->id = sid;

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

+2 −2
Original line number Diff line number Diff line
@@ -314,7 +314,7 @@ test_connect(void) 
	/* Valid admin connect command */

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

	rc = spdk_nvmf_ctrlr_connect(&req);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS);

	CU_ASSERT(nvme_status_success(&rsp.nvme_cpl.status));

	CU_ASSERT(qpair.ctrlr != NULL);

	free(qpair.ctrlr);
@@ -441,7 +441,7 @@ test_connect(void) 
	memset(&rsp, 0, sizeof(rsp));

	MOCK_SET(spdk_nvmf_subsystem_get_ctrlr, struct spdk_nvmf_ctrlr *, NULL);

	rc = spdk_nvmf_ctrlr_connect(&req);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS);

	CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC);

	CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVMF_FABRIC_SC_INVALID_PARAM);

	CU_ASSERT(rsp.connect_rsp.status_code_specific.invalid.iattr == 1);