nvme: combine qpair->is_connecting and is_enabled

These will form the base of a little state machine for managing the nvme
qpair structure.

Change-Id: If6f6df38cc17221ac8fcb7d8c0d7e2e808897a99
Signed-off-by: Seth Howell <seth.howell@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470534
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

lib/nvme/nvme.c

@@ -429,7 +429,7 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid,
 		ctrlr->remove_cb = probe_ctx->remove_cb;
 		ctrlr->cb_ctx = probe_ctx->cb_ctx;
 
-		nvme_qpair_enable(ctrlr->adminq);
+		nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_ENABLED);
 		TAILQ_INSERT_TAIL(&probe_ctx->init_ctrlrs, ctrlr, tailq);
 		return 0;
 	}

lib/nvme/nvme_ctrlr.c

@@ -364,6 +364,7 @@ spdk_nvme_ctrlr_alloc_io_qpair(struct spdk_nvme_ctrlr *ctrlr,
 		nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock);
 		return NULL;
 	}
+	nvme_qpair_set_state(qpair, NVME_QPAIR_CONNECTED);
 	spdk_bit_array_clear(ctrlr->free_io_qids, qid);
 	TAILQ_INSERT_TAIL(&ctrlr->active_io_qpairs, qpair, tailq);
 
@@ -987,18 +988,20 @@ nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr)
 
 	/* Disable all queues before disabling the controller hardware. */
 	TAILQ_FOREACH(qpair, &ctrlr->active_io_qpairs, tailq) {
-		nvme_qpair_disable(qpair);
+		nvme_qpair_set_state(qpair, NVME_QPAIR_DISABLED);
 		nvme_transport_ctrlr_disconnect_qpair(ctrlr, qpair);
 	}
-	nvme_qpair_disable(ctrlr->adminq);
+	nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_DISABLED);
 	nvme_qpair_complete_error_reqs(ctrlr->adminq);
 	nvme_transport_qpair_abort_reqs(ctrlr->adminq, 0 /* retry */);
 	nvme_transport_ctrlr_disconnect_qpair(ctrlr, ctrlr->adminq);
 	if (nvme_transport_ctrlr_connect_qpair(ctrlr, ctrlr->adminq) != 0) {
 		SPDK_ERRLOG("Controller reinitialization failed.\n");
+		nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_DISABLED);
 		rc = -1;
 		goto out;
 	}
+	nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_CONNECTED);
 
 	/* Doorbell buffer config is invalid during reset */
 	nvme_ctrlr_free_doorbell_buffer(ctrlr);
@@ -1006,7 +1009,7 @@ nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr)
 	/* Set the state back to INIT to cause a full hardware reset. */
 	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_INIT, NVME_TIMEOUT_INFINITE);
 
-	nvme_qpair_enable(ctrlr->adminq);
+	nvme_qpair_set_state(ctrlr->adminq, NVME_QPAIR_ENABLED);
 	while (ctrlr->state != NVME_CTRLR_STATE_READY) {
 		if (nvme_ctrlr_process_init(ctrlr) != 0) {
 			SPDK_ERRLOG("controller reinitialization failed\n");
@@ -1019,8 +1022,11 @@ nvme_ctrlr_reset(struct spdk_nvme_ctrlr *ctrlr)
 		/* Reinitialize qpairs */
 		TAILQ_FOREACH(qpair, &ctrlr->active_io_qpairs, tailq) {
 			if (nvme_transport_ctrlr_connect_qpair(ctrlr, qpair) != 0) {
+				nvme_qpair_set_state(qpair, NVME_QPAIR_DISABLED);
 				rc = -1;
+				continue;
 			}
+			nvme_qpair_set_state(qpair, NVME_QPAIR_CONNECTED);
 		}
 	}
 

lib/nvme/nvme_internal.h

@@ -331,6 +331,13 @@ struct nvme_async_event_request {
 	struct spdk_nvme_cpl		cpl;
 };
 
+enum nvme_qpair_state {
+	NVME_QPAIR_DISABLED,
+	NVME_QPAIR_CONNECTING,
+	NVME_QPAIR_CONNECTED,
+	NVME_QPAIR_ENABLED,
+};
+
 struct spdk_nvme_qpair {
 	struct spdk_nvme_ctrlr		*ctrlr;
 
@@ -338,8 +345,7 @@ struct spdk_nvme_qpair {
 
 	uint8_t				qprio;
 
-	uint8_t				is_enabled : 1;
+	uint8_t				state : 3;
-	uint8_t				is_connecting: 1;
 
 	/*
 	 * Members for handling IO qpair deletion inside of a completion context.
@@ -848,8 +854,6 @@ int	nvme_qpair_init(struct spdk_nvme_qpair *qpair, uint16_t id,
 			enum spdk_nvme_qprio qprio,
 			uint32_t num_requests);
 void	nvme_qpair_deinit(struct spdk_nvme_qpair *qpair);
-void	nvme_qpair_enable(struct spdk_nvme_qpair *qpair);
-void	nvme_qpair_disable(struct spdk_nvme_qpair *qpair);
 void	nvme_qpair_complete_error_reqs(struct spdk_nvme_qpair *qpair);
 int	nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair,
 				  struct nvme_request *req);
@@ -973,6 +977,18 @@ nvme_free_request(struct nvme_request *req)
 	STAILQ_INSERT_HEAD(&req->qpair->free_req, req, stailq);
 }
 
+static inline void
+nvme_qpair_set_state(struct spdk_nvme_qpair *qpair, enum nvme_qpair_state state)
+{
+	qpair->state = state;
+}
+
+static inline bool
+nvme_qpair_state_equals(struct spdk_nvme_qpair *qpair, enum nvme_qpair_state state)
+{
+	return qpair->state == state;
+}
+
 static inline void
 nvme_qpair_free_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req)
 {

lib/nvme/nvme_qpair.c

@@ -407,14 +407,18 @@ nvme_qpair_abort_queued_reqs(struct spdk_nvme_qpair *qpair, uint32_t dnr)
 static inline bool
 nvme_qpair_check_enabled(struct spdk_nvme_qpair *qpair)
 {
-	if (!qpair->is_enabled && !qpair->ctrlr->is_resetting) {
+	/*
+	 * This is the point at which we re-enable the qpair after a reset. If the qpair has been
+	 * disabled for some reason, we need to flush it and restart submitting and completing I/O.
+	 */
+	if (!nvme_qpair_state_equals(qpair, NVME_QPAIR_ENABLED) && !qpair->ctrlr->is_resetting) {
 		nvme_qpair_complete_error_reqs(qpair);
 		nvme_qpair_abort_queued_reqs(qpair, 0 /* retry */);
-		nvme_qpair_enable(qpair);
+		nvme_qpair_set_state(qpair, NVME_QPAIR_ENABLED);
 		nvme_transport_qpair_abort_reqs(qpair, 0 /* retry */);
 	}
 
-	return qpair->is_enabled;
+	return nvme_qpair_state_equals(qpair, NVME_QPAIR_ENABLED);
 }
 
 int32_t
@@ -430,7 +434,8 @@ spdk_nvme_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_
 		return 0;
 	}
 
-	if (spdk_unlikely(!nvme_qpair_check_enabled(qpair) && !qpair->is_connecting)) {
+	if (spdk_unlikely(!nvme_qpair_check_enabled(qpair) &&
+			  !nvme_qpair_state_equals(qpair, NVME_QPAIR_CONNECTING))) {
 		/*
 		 * qpair is not enabled, likely because a controller reset is
 		 *  in progress.
@@ -635,9 +640,10 @@ _nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *r
 		req->submit_tick = 0;
 	}
 
-	if (spdk_likely(qpair->is_enabled)) {
+	if (spdk_likely(nvme_qpair_state_equals(qpair, NVME_QPAIR_ENABLED))) {
 		rc = nvme_transport_qpair_submit_request(qpair, req);
-	} else if (req->cmd.opc == SPDK_NVME_OPC_FABRIC && qpair->is_connecting) {
+	} else if (req->cmd.opc == SPDK_NVME_OPC_FABRIC &&
+		   nvme_qpair_state_equals(qpair, NVME_QPAIR_CONNECTING)) {
 		/* Always allow fabrics commands through - these get
 		 * the controller out of reset state.
 		 */
@@ -709,18 +715,6 @@ nvme_qpair_resubmit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *
 	return rc;
 }
 
-void
-nvme_qpair_enable(struct spdk_nvme_qpair *qpair)
-{
-	qpair->is_enabled = true;
-}
-
-void
-nvme_qpair_disable(struct spdk_nvme_qpair *qpair)
-{
-	qpair->is_enabled = false;
-}
-
 static void
 nvme_qpair_abort_reqs(struct spdk_nvme_qpair *qpair, uint32_t dnr)
 {

lib/nvme/nvme_transport.c

@@ -182,9 +182,8 @@ nvme_transport_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_
 int
 nvme_transport_ctrlr_connect_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qpair *qpair)
 {
-	qpair->is_connecting = 1;
+	nvme_qpair_set_state(qpair, NVME_QPAIR_CONNECTING);
 	NVME_TRANSPORT_CALL(ctrlr->trid.trtype, ctrlr_connect_qpair, (ctrlr, qpair));
-	qpair->is_connecting = 0;
 }
 
 volatile struct spdk_nvme_registers *

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

@@ -68,12 +68,6 @@ nvme_ctrlr_destruct(struct spdk_nvme_ctrlr *ctrlr)
 	ut_destruct_called = true;
 }
 
-void
-nvme_qpair_enable(struct spdk_nvme_qpair *qpair)
-{
-	qpair->is_enabled = true;
-}
-
 void
 spdk_nvme_ctrlr_get_default_ctrlr_opts(struct spdk_nvme_ctrlr_opts *opts, size_t opts_size)
 {

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

@@ -267,20 +267,11 @@ spdk_nvme_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_
 	return 0;
 }
 
-void
-nvme_qpair_disable(struct spdk_nvme_qpair *qpair)
-{
-}
-
 void
 nvme_qpair_complete_error_reqs(struct spdk_nvme_qpair *qpair)
 {
 }
 
-void
-nvme_qpair_enable(struct spdk_nvme_qpair *qpair)
-{
-}
 
 void
 nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl)

test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c

@@ -275,12 +275,6 @@ spdk_nvme_qpair_process_completions(struct spdk_nvme_qpair *qpair, uint32_t max_
 	abort();
 }
 
-void
-nvme_qpair_enable(struct spdk_nvme_qpair *qpair)
-{
-	abort();
-}
-
 int
 nvme_request_check_timeout(struct nvme_request *req, uint16_t cid,
 			   struct spdk_nvme_ctrlr_process *active_proc,