diff mbox series

[v7,4/4] ufs: core: skip ISR notifying scsi when ufshcd_abort

Message ID 20240920090643.3566-5-peter.wang@mediatek.com
State New
Headers show
Series fix abort defect | expand

Commit Message

Peter Wang (王信友) Sept. 20, 2024, 9:06 a.m. UTC
From: Peter Wang <peter.wang@mediatek.com>

When a SCSI abort occurs, ufshcd_try_to_abort_task may trigger
the ISR, and the ISR may release the command and notify SCSI
via scsi_done. This patch prevents the ISR from notifying
SCSI to requeue, allowing SCSI to decide whether to requeue.

Below is ufshcd_abort legacy SDB flow:

ufshcd_abort()
  ufshcd_try_to_abort_task() // will trigger ISR
  get outstanding_lock
  clear outstanding_reqs tag
  ufshcd_release_scsi_cmd()
  release outstanding_lock

ufshcd_intr()
  ufshcd_sl_intr()
    ufshcd_transfer_req_compl()
      ufshcd_poll()
        get outstanding_lock
        clear outstanding_reqs tag
        release outstanding_lock
        __ufshcd_transfer_req_compl()
          ufshcd_compl_one_cqe()
          cmd->result = DID_REQUEUE
          ufshcd_release_scsi_cmd()
          scsi_done();

In most cases, ufshcd_intr will not reach scsi_done because the
outstanding_reqs tag is cleared by the original thread.
Therefore, whether there is an interrupt or not doesn't affect
the result because the ISR will do nothing in most cases.

In a very low chance, the ISR will reach scsi_done and notify
SCSI to requeue, and the original thread will not
call ufshcd_release_scsi_cmd. So should release because
outstanding_reqs is clear by ISR.

Below is ufshcd_abort MCQ flow:

ufshcd_abort()
  ufshcd_mcq_abort()
    ufshcd_try_to_abort_task()	// will trigger ISR
    ufshcd_release_scsi_cmd()

ufs_mtk_mcq_intr()
  ufshcd_mcq_poll_cqe_lock()
    ufshcd_mcq_process_cqe()
      ufshcd_compl_one_cqe()
        cmd->result = DID_ABORT
        ufshcd_release_scsi_cmd() // will release twice
        scsi_done()

In the case of MCQ ufshcd_abort, there is an issue where
ufshcd_release_scsi_cmd might be called twice. We could simply
skip the ISR release and scsi_done.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/ufs/core/ufs-mcq.c |  6 ++++++
 drivers/ufs/core/ufshcd.c  | 27 +++++++++++++++++++++++++++
 include/ufs/ufshcd.h       |  1 +
 3 files changed, 34 insertions(+)
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 3903947dbed1..73d7cf337e2f 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -699,6 +699,12 @@  int ufshcd_mcq_abort(struct scsi_cmnd *cmd)
 		return FAILED;
 	}
 
+	/*
+	 * In MCQ mode, set this variable so that the ISR posted by
+	 * the host controller can be skipped.
+	 */
+	lrbp->abort_initiated_by = UFS_SCSI_ABORT;
+
 	/*
 	 * The command is not in the submission queue, and it is not
 	 * in the completion queue either. Query the device to see if
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index b34125238a70..21091b11b4ba 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5537,6 +5537,27 @@  void ufshcd_compl_one_cqe(struct ufs_hba *hba, int task_tag,
 			ufshcd_update_monitor(hba, lrbp);
 		ufshcd_add_command_trace(hba, task_tag, UFS_CMD_COMP);
 		cmd->result = ufshcd_transfer_rsp_status(hba, lrbp, cqe);
+
+		/*
+		 * Let the SCSI layer decide how to handle the ufshcd_abort
+		 * situation, neither releasing nor notifying scsi_done in MCQ
+		 * mode. SDB mode should release because outstanding_reqs is
+		 * clear by ISR.
+		 */
+		if (lrbp->abort_initiated_by == UFS_SCSI_ABORT) {
+			ocs = ufshcd_get_tr_ocs(lrbp, cqe);
+			if ((hba->mcq_enabled) && (ocs == OCS_ABORTED))
+				 return;
+
+			if ((!hba->mcq_enabled) &&
+			    ((ocs == OCS_INVALID_COMMAND_STATUS) ||
+			     ((hba->quirks & UFSHCD_QUIRK_OCS_ABORTED) &&
+			      (ocs == OCS_ABORTED)))) {
+				ufshcd_release_scsi_cmd(hba, lrbp);
+				return;
+			}
+		}
+
 		ufshcd_release_scsi_cmd(hba, lrbp);
 		/* Do not touch lrbp after scsi done */
 		scsi_done(cmd);
@@ -7673,6 +7694,12 @@  static int ufshcd_abort(struct scsi_cmnd *cmd)
 		goto release;
 	}
 
+	/*
+	 * In SDB mode, set this variable so that the ISR posted by
+	 * the host controller clear UTRLCLR can be skipped.
+	 */
+	lrbp->abort_initiated_by = UFS_SCSI_ABORT;
+
 	err = ufshcd_try_to_abort_task(hba, tag);
 	if (err) {
 		dev_err(hba->dev, "%s: failed with err %d\n", __func__, err);
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 4d17a13ac558..4785a45040eb 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -148,6 +148,7 @@  enum ufs_pm_level {
 enum ufs_abort_by {
 	UFS_NO_ABORT,
 	UFS_ERR_HANDLER,
+	UFS_SCSI_ABORT
 };
 
 struct ufs_pm_lvl_states {