[PATCH v7 12/17] nvme-fc: start error recovery instead of aborting timed out IOs

[Mohamed Khalfella <mkhalfella@purestorage.com>]

Aborts issued from the timeout handler run outside the FCCTRL_TERMIO
window, so they are not counted in ctrl->iocnt and
nvme_fc_delete_association() does not wait for them. The association
can be torn down while the LLDD is still working on the abort.

Instead of aborting the timed out command, reset the controller like
the other fabrics transports do. The association teardown aborts the
outstanding IOs, counts the aborts and waits for them to complete.

Count every abort in ctrl->iocnt, not only the ones issued while
FCCTRL_TERMIO is set, and do not reset the count when the teardown
starts. An abort issued before the teardown is then waited for as well.

Only a LIVE controller starts error recovery. In every other state the
association is being set up, being torn down or already gone, so abort
the timed out command from the timeout handler instead and return
BLK_EH_DONE. The command completes through nvme_fc_fcpio_done() once
the LLDD has finished the abort, and the abort is counted so a teardown
that follows waits for it. This matters for a controller being deleted.
The delete path drains the IOs itself and tears the association down in
nvme_fc_delete_ctrl() once the drain is done, and DELETING cannot move
to RESETTING, so without the abort a timed out IO would hang the drain
against a dead target.

The new nvme_fc_start_ioerr_recovery() is the common entry point for IO
timeouts, transport errors reported by nvme_fc_fcpio_done(), Disconnect
LS and connectivity loss. It moves the controller to RESETTING and
queues ioerr_work only if that succeeds, so nothing is ever queued on a
controller that is CONNECTING or being deleted. A Disconnect LS or a
transport error received while CONNECTING is resolved when the inflight
request fails, or times out and is aborted.

ioerr_work still calls nvme_fc_error_recovery(), which returns without
doing anything for a controller that is not LIVE. With the controller
already in RESETTING when the work runs, no recovery takes place yet
and the controller stays in RESETTING. The next patch moves the
recovery into ioerr_work itself and completes the change.

Link: https://lore.kernel.org/all/20250529214928.2112990-1-mkhalfella@purestorage.com/
Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
---
 drivers/nvme/host/fc.c | 66 ++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 0b9ff50d26ff..c4603be38144 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -227,6 +227,8 @@ static DEFINE_IDA(nvme_fc_ctrl_cnt);
 static struct device *fc_udev_device;
 
 static void nvme_fc_complete_rq(struct request *rq);
+static void nvme_fc_start_ioerr_recovery(struct nvme_fc_ctrl *ctrl,
+					 char *errmsg);
 
 /* *********************** FC-NVME Port Management ************************ */
 
@@ -788,7 +790,7 @@ nvme_fc_ctrl_connectivity_loss(struct nvme_fc_ctrl *ctrl)
 		"Reconnect", ctrl->cnum);
 
 	set_bit(ASSOC_FAILED, &ctrl->flags);
-	nvme_reset_ctrl(&ctrl->ctrl);
+	nvme_fc_start_ioerr_recovery(ctrl, "Connectivity Loss");
 }
 
 /**
@@ -1569,7 +1571,8 @@ nvme_fc_ls_disconnect_assoc(struct nvmefc_ls_rcv_op *lsop)
 	 */
 
 	/* fail the association */
-	nvme_fc_error_recovery(ctrl, "Disconnect Association LS received");
+	nvme_fc_start_ioerr_recovery(ctrl,
+				     "Disconnect Association LS received");
 
 	/* release the reference taken by nvme_fc_match_disconn_ls() */
 	nvme_fc_ctrl_put(ctrl);
@@ -1813,11 +1816,18 @@ __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op)
 	unsigned long flags;
 	int opstate;
 
+	/*
+	 * Every abort is counted, not only those issued by the association
+	 * teardown: the teardown waits for iocnt to drop to zero before it
+	 * deletes the hw queues, and an abort issued earlier, by
+	 * nvme_fc_timeout() while deleting or by the CONNECTING recovery
+	 * path, must not still be in progress in the LLDD then.
+	 */
 	spin_lock_irqsave(&ctrl->lock, flags);
 	opstate = atomic_xchg(&op->state, FCPOP_STATE_ABORTED);
-	if (opstate != FCPOP_STATE_ACTIVE)
+	if (opstate != FCPOP_STATE_ACTIVE) {
 		atomic_set(&op->state, opstate);
-	else if (test_bit(FCCTRL_TERMIO, &ctrl->flags)) {
+	} else {
 		op->flags |= FCOP_FLAGS_TERMIO;
 		ctrl->iocnt++;
 	}
@@ -1856,8 +1866,7 @@ __nvme_fc_fcpop_chk_teardowns(struct nvme_fc_ctrl *ctrl,
 
 	if (opstate == FCPOP_STATE_ABORTED) {
 		spin_lock_irqsave(&ctrl->lock, flags);
-		if (test_bit(FCCTRL_TERMIO, &ctrl->flags) &&
-		    op->flags & FCOP_FLAGS_TERMIO) {
+		if (op->flags & FCOP_FLAGS_TERMIO) {
 			if (!--ctrl->iocnt)
 				wake_up(&ctrl->ioabort_wait);
 		}
@@ -1892,6 +1901,19 @@ char *nvme_fc_io_getuuid(struct nvmefc_fcp_req *req)
 }
 EXPORT_SYMBOL_GPL(nvme_fc_io_getuuid);
 
+/*
+ * Start recovery only if this caller can claim it with a state change.
+ */
+static void
+nvme_fc_start_ioerr_recovery(struct nvme_fc_ctrl *ctrl, char *errmsg)
+{
+	if (nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RESETTING)) {
+		dev_warn(ctrl->ctrl.device, "NVME-FC{%d}: starting error recovery %s\n",
+			 ctrl->cnum, errmsg);
+		queue_work(nvme_reset_wq, &ctrl->ioerr_work);
+	}
+}
+
 static void
 nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 {
@@ -2049,9 +2071,8 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req)
 		nvme_fc_complete_rq(rq);
 
 check_error:
-	if (terminate_assoc &&
-	    nvme_ctrl_state(&ctrl->ctrl) != NVME_CTRL_RESETTING)
-		queue_work(nvme_reset_wq, &ctrl->ioerr_work);
+	if (terminate_assoc)
+		nvme_fc_start_ioerr_recovery(ctrl, "io error");
 }
 
 static int
@@ -2548,24 +2569,32 @@ static enum blk_eh_timer_return nvme_fc_timeout(struct request *rq)
 	struct nvme_fc_cmd_iu *cmdiu = &op->cmd_iu;
 	struct nvme_command *sqe = &cmdiu->sqe;
 
-	/*
-	 * Attempt to abort the offending command. Command completion
-	 * will detect the aborted io and will fail the connection.
-	 */
 	dev_info(ctrl->ctrl.device,
 		"NVME-FC{%d.%d}: io timeout: opcode %d fctype %d (%s) w10/11: "
 		"x%08x/x%08x\n",
 		ctrl->cnum, qnum, sqe->common.opcode, sqe->fabrics.fctype,
 		nvme_fabrics_opcode_str(qnum, sqe),
 		sqe->common.cdw10, sqe->common.cdw11);
-	if (__nvme_fc_abort_op(ctrl, op))
-		nvme_fc_error_recovery(ctrl, "io timeout abort failed");
+
+	if (nvme_ctrl_state(&ctrl->ctrl) != NVME_CTRL_LIVE) {
+		/*
+		 * The association is being set up, being torn down or is
+		 * already gone, so recovery cannot be started from here.
+		 * Abort the command so the connect or teardown it would
+		 * block can make progress. It completes through
+		 * nvme_fc_fcpio_done() once the LLDD has finished the
+		 * abort, and the abort is counted so a teardown that
+		 * follows waits for it.
+		 */
+		__nvme_fc_abort_op(ctrl, op);
+		return BLK_EH_DONE;
+	}
 
 	/*
-	 * the io abort has been initiated. Have the reset timer
-	 * restarted and the abort completion will complete the io
-	 * shortly. Avoids a synchronous wait while the abort finishes.
+	 * A LIVE controller starts error recovery, which tears the
+	 * association down and completes this request.
 	 */
+	nvme_fc_start_ioerr_recovery(ctrl, "io timeout");
 	return BLK_EH_RESET_TIMER;
 }
 
@@ -3217,7 +3246,6 @@ nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl)
 
 	spin_lock_irqsave(&ctrl->lock, flags);
 	set_bit(FCCTRL_TERMIO, &ctrl->flags);
-	ctrl->iocnt = 0;
 	spin_unlock_irqrestore(&ctrl->lock, flags);
 
 	__nvme_fc_abort_outstanding_ios(ctrl, false);
-- 
2.56.0