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

nvmf: simplify spdk_nvmf_request_exec() 



A few small tweaks to make this function easier to read:
- Return void (the return value is always 0 and never used)
- Split out Fabrics/admin queue processing 'if' block
- Remove unnecessary switch on status (it can only be 2 values)

Additionally, simplify the I/O command checking logic: we don't need to
check for CC.EN = 1, because it is only possible for I/O queues to be
created after CC.EN is set to 1.

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


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 5accfd57

lib/nvmf/nvmf_internal.h

+1 −1
Original line number Diff line number Diff line
@@ -206,7 +206,7 @@ int spdk_nvmf_poll_group_remove(struct spdk_nvmf_poll_group *group, 
				struct spdk_nvmf_qpair *qpair);

int spdk_nvmf_poll_group_poll(struct spdk_nvmf_poll_group *group);



int spdk_nvmf_request_exec(struct spdk_nvmf_request *req);

void spdk_nvmf_request_exec(struct spdk_nvmf_request *req);

int spdk_nvmf_request_complete(struct spdk_nvmf_request *req);

int spdk_nvmf_request_abort(struct spdk_nvmf_request *req);

lib/nvmf/request.c

+18 −36
Original line number Diff line number Diff line
@@ -147,59 +147,41 @@ spdk_nvmf_request_exec_on_master(void *ctx) 
		status = spdk_nvmf_ctrlr_process_admin_cmd(req);

	}



	switch (status) {

	case SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE:

	if (status == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE) {

		spdk_nvmf_request_complete(req);

		break;

	case SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS:

		break;

	default:

		SPDK_UNREACHABLE();

	}

}



int

void

spdk_nvmf_request_exec(struct spdk_nvmf_request *req)

{

	struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr;

	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_ctrlr *ctrlr = qpair->ctrlr;

	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;

	struct spdk_nvme_cpl *rsp = &req->rsp->nvme_cpl;

	spdk_nvmf_request_exec_status status;



	nvmf_trace_command(req->cmd, req->qpair->type);

	nvmf_trace_command(req->cmd, qpair->type);



	if (cmd->opc == SPDK_NVME_OPC_FABRIC ||

	    req->qpair->type == QPAIR_TYPE_AQ) {

		/* Fabric and admin commands are sent

		 * to the master core for synchronization

		 * reasons.

		 */

		spdk_thread_send_msg(req->qpair->transport->tgt->master_thread,

	if (spdk_unlikely(cmd->opc == SPDK_NVME_OPC_FABRIC || qpair->type == QPAIR_TYPE_AQ)) {

		/* Fabric and admin commands are sent to the master core for synchronization. */

		spdk_thread_send_msg(qpair->transport->tgt->master_thread,

				     spdk_nvmf_request_exec_on_master,

				     req);

		status = SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS;

	} else if (ctrlr == NULL ||

		   !ctrlr->vcprop.cc.bits.en) {

		/* TODO: The EN bit is modified by the master thread. This needs

		 * stronger synchronization.

		 */

		SPDK_ERRLOG("Non-Fabric command sent to disabled controller\n");

		rsp->status.sc = SPDK_NVME_SC_COMMAND_SEQUENCE_ERROR;

		status = SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	} else {

		status = spdk_nvmf_ctrlr_process_io_cmd(req);

		return;

	}



	switch (status) {

	case SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE:

		return spdk_nvmf_request_complete(req);

	case SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS:

		return 0;

	default:

		SPDK_UNREACHABLE();

	if (spdk_unlikely(ctrlr == NULL)) {

		SPDK_ERRLOG("I/O command sent before connect\n");

		rsp->status.sc = SPDK_NVME_SC_COMMAND_SEQUENCE_ERROR;

		spdk_nvmf_request_complete_on_qpair(req);

		return;

	}



	return 0;

	status = spdk_nvmf_ctrlr_process_io_cmd(req);

	if (status == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE) {

		spdk_nvmf_request_complete_on_qpair(req);

	}

}



int