public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: <peter.wang@mediatek.com>
To: <linux-scsi@vger.kernel.org>, <martin.petersen@oracle.com>,
	<avri.altman@wdc.com>, <alim.akhtar@samsung.com>,
	<jejb@linux.ibm.com>
Cc: <wsd_upstream@mediatek.com>, <linux-mediatek@lists.infradead.org>,
	<peter.wang@mediatek.com>, <chun-hung.wu@mediatek.com>,
	<alice.chao@mediatek.com>, <cc.chou@mediatek.com>,
	<chaotian.jing@mediatek.com>, <jiajie.hao@mediatek.com>,
	<powen.kao@mediatek.com>, <qilin.tan@mediatek.com>,
	<lin.gui@mediatek.com>, <tun-yu.yu@mediatek.com>,
	<eddie.huang@mediatek.com>, <naomi.chu@mediatek.com>,
	<ed.tsai@mediatek.com>, <bvanassche@acm.org>,
	<quic_nguyenb@quicinc.com>
Subject: [PATCH v7 4/4] ufs: core: skip ISR notifying scsi when ufshcd_abort
Date: Fri, 20 Sep 2024 17:06:43 +0800	[thread overview]
Message-ID: <20240920090643.3566-5-peter.wang@mediatek.com> (raw)
In-Reply-To: <20240920090643.3566-1-peter.wang@mediatek.com>

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 --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 {
-- 
2.45.2


  parent reply	other threads:[~2024-09-20  9:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-20  9:06 [PATCH v7 0/4] fix abort defect peter.wang
2024-09-20  9:06 ` [PATCH v7 1/4] ufs: core: fix the issue of ICU failure peter.wang
2024-09-20  9:06 ` [PATCH v7 2/4] ufs: core: requeue aborted request peter.wang
2024-09-20  9:06 ` [PATCH v7 3/4] ufs: core: add a quirk for MediaTek SDB mode aborted peter.wang
2024-09-20  9:06 ` peter.wang [this message]
2024-09-20 19:36 ` [PATCH v7 0/4] fix abort defect Bart Van Assche
2024-09-23  7:07   ` Peter Wang (王信友)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240920090643.3566-5-peter.wang@mediatek.com \
    --to=peter.wang@mediatek.com \
    --cc=alice.chao@mediatek.com \
    --cc=alim.akhtar@samsung.com \
    --cc=avri.altman@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=cc.chou@mediatek.com \
    --cc=chaotian.jing@mediatek.com \
    --cc=chun-hung.wu@mediatek.com \
    --cc=ed.tsai@mediatek.com \
    --cc=eddie.huang@mediatek.com \
    --cc=jejb@linux.ibm.com \
    --cc=jiajie.hao@mediatek.com \
    --cc=lin.gui@mediatek.com \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=naomi.chu@mediatek.com \
    --cc=powen.kao@mediatek.com \
    --cc=qilin.tan@mediatek.com \
    --cc=quic_nguyenb@quicinc.com \
    --cc=tun-yu.yu@mediatek.com \
    --cc=wsd_upstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox