mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions
@ 2026-10-22  3:44 Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

In Single Doorbell (SDB) mode, multiple completion and error handling
paths can race on the same transfer request when bits in
hba->outstanding_reqs are not claimed atomically with completing the
request. A stale bit in hba->outstanding_reqs lets a later poll or
reset complete the request again and unmap its DMA buffers twice.

Patch 1 removes a WARN_ONCE() in ufshcd_poll() whose condition is always
false.

Patch 2 adds __ufshcd_clear_outstanding_reqs() and
ufshcd_clear_outstanding_reqs() to share the bit-claim logic across
ufshcd_poll() and ufshcd_abort() with no functional change.

Patch 3 uses the helper in ufshcd_clear_lu_cmds() to fix a double
completion during LU reset.

Patch 4 uses the helper in ufshcd_eh_timed_out() and checks its return
value to prevent releasing driver resources twice.

Patch 5 removes the completed-without-interrupt branch in ufshcd_abort()
to fix a double completion on timed-out commands.

Patch 6 skips releasing reserved requests at the end of ufshcd_abort().

This series is based on mkp/for-next (db8b4d2bf2d3).

Tested: QEMU ARM64 in SDB mode across probe, I/O, abort, LU reset, and
PM timeout paths without double completion or resource underflow.

Changes since v4:
- Patch 5/6: remove the completed-without-interrupt branch in
  ufshcd_abort() instead of claiming the request there (Bart).
- Collect Reviewed-by tags from Peter Wang on patches 4/6 and 6/6.
- Link to v4:
  https://lore.kernel.org/r/20261021035450.3488805-1-stanleyjhu@google.com

Stanley Jhu (6):
  scsi: ufs: core: Remove an always-false WARN_ONCE()
  scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
  scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
  scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
  scsi: ufs: core: Remove completed-without-interrupt path in
    ufshcd_abort()
  scsi: ufs: core: Do not release a reserved request in ufshcd_abort()

 drivers/ufs/core/ufshcd.c | 70 +++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 29 deletions(-)


base-commit: db8b4d2bf2d3e02b4ff78bbf69d00e11795840de
-- 
2.56.0.218.g1ec904b173-goog

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE()
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 2/6] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

The WARN_ONCE() in ufshcd_poll() can never fire. completed_reqs is set
to ~tr_doorbell & hba->outstanding_reqs on the line above, and
hba->outstanding_reqs cannot change in between because ufshcd_poll()
holds hba->outstanding_lock. So completed_reqs & ~hba->outstanding_reqs
is always zero. Remove it. No functional change.

Cc: stable@vger.kernel.org
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..12aafcf1f9b3 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6072,9 +6072,6 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	spin_lock_irqsave(&hba->outstanding_lock, flags);
 	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	completed_reqs = ~tr_doorbell & hba->outstanding_reqs;
-	WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
-		  "completed: %#lx; outstanding: %#lx\n", completed_reqs,
-		  hba->outstanding_reqs);
 	if (queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT) {
 		/* Do not complete polled requests from interrupt context. */
 		ufshcd_clear_polled(hba, &completed_reqs);
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 2/6] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 3/6] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

ufshcd_poll() and ufshcd_abort() each have their own copy of the code
that clears bits in hba->outstanding_reqs under hba->outstanding_lock.

Move that code into ufshcd_clear_outstanding_reqs() and its locked
variant __ufshcd_clear_outstanding_reqs(), and use them in both
functions. No functional change.

Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/3779e125-9acc-42c8-9489-51b9e3c6716d@acm.org
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 48 +++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 12aafcf1f9b3..1db572f84971 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6052,6 +6052,37 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
 	}
 }
 
+/*
+ * Return a bitmask of the transfer requests cleared from
+ * @hba->outstanding_reqs.
+ */
+static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
+						     unsigned long mask,
+						     bool from_irq)
+	__must_hold(&hba->outstanding_lock)
+{
+	unsigned long completed_reqs = mask & hba->outstanding_reqs;
+
+	if (from_irq) {
+		/* Do not complete polled requests from interrupt context. */
+		ufshcd_clear_polled(hba, &completed_reqs);
+	}
+	hba->outstanding_reqs &= ~completed_reqs;
+
+	return completed_reqs;
+}
+
+/*
+ * Return a bitmask of the transfer requests cleared from
+ * @hba->outstanding_reqs.
+ */
+static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
+						   unsigned long mask)
+{
+	guard(spinlock_irqsave)(&hba->outstanding_lock);
+	return __ufshcd_clear_outstanding_reqs(hba, mask, false);
+}
+
 /*
  * Return: > 0 if one or more commands have been completed or 0 if no
  * requests have been completed.
@@ -6062,6 +6093,7 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	unsigned long completed_reqs, flags;
 	u32 tr_doorbell;
 	struct ufs_hw_queue *hwq;
+	bool from_irq;
 
 	if (hba->mcq_enabled) {
 		hwq = &hba->uhq[queue_num];
@@ -6071,12 +6103,9 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
 
 	spin_lock_irqsave(&hba->outstanding_lock, flags);
 	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
-	completed_reqs = ~tr_doorbell & hba->outstanding_reqs;
-	if (queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT) {
-		/* Do not complete polled requests from interrupt context. */
-		ufshcd_clear_polled(hba, &completed_reqs);
-	}
-	hba->outstanding_reqs &= ~completed_reqs;
+	from_irq = queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT;
+	completed_reqs = __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
+							 from_irq);
 	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 
 	if (completed_reqs)
@@ -8111,7 +8140,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
 	unsigned long flags;
 	int err = FAILED;
-	bool outstanding;
 	u32 reg;
 
 	ufshcd_hold(hba);
@@ -8206,11 +8234,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 * Clear the corresponding bit from outstanding_reqs since the command
 	 * has been aborted successfully.
 	 */
-	spin_lock_irqsave(&hba->outstanding_lock, flags);
-	outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs);
-	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
-
-	if (outstanding)
+	if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
 		ufshcd_release_scsi_cmd(hba, cmd);
 
 	err = SUCCESS;
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 3/6] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 2/6] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22  3:44 ` [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out() Stanley Jhu
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

In SDB mode, ufshcd_clear_lu_cmds() clears the request slot in the
controller and completes the request, but leaves its bit set in
hba->outstanding_reqs. ufshcd_poll() treats a request whose bit is set
in hba->outstanding_reqs and clear in the doorbell register as finished,
so the next ufshcd_poll() completes the same request again and unmaps
its DMA buffers twice.

Clear the bit with ufshcd_clear_outstanding_reqs() before completing the
request, and complete it only if the bit was still set. If ufshcd_poll()
has already completed the request, the bit is already clear and
ufshcd_clear_lu_cmds() skips it.

Fixes: f18fac1e2b72 ("scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()")
Cc: stable@vger.kernel.org
Reviewed-by: Bean Huo <beanhuo@micron.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 1db572f84971..68b53f89f8c6 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7987,7 +7987,8 @@ static bool ufshcd_clear_lu_cmds(struct request *req, void *priv)
 		return true;
 	}
 
-	ufshcd_compl_one_cqe(hba, tag, NULL);
+	if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
+		ufshcd_compl_one_cqe(hba, tag, NULL);
 	return true;
 }
 
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
                   ` (2 preceding siblings ...)
  2026-10-22  3:44 ` [PATCH v5 3/6] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22 18:03   ` Bean Huo
  2026-10-22  3:44 ` [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort() Stanley Jhu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

In SDB mode, a command that times out during a power management
operation can be released twice:

1. The command times out while its normal completion is running.
2. The completion path clears BIT(rq->tag) in hba->outstanding_reqs and
   releases the command. It has not called scsi_done() yet, so
   SCMD_STATE_COMPLETE is still clear.
3. ufshcd_eh_timed_out() recovers the link, sees SCMD_STATE_COMPLETE
   clear, clears BIT(rq->tag) in hba->outstanding_reqs without checking
   whether it was still set, and releases the same command a second
   time.

Return SCSI_EH_DONE when ufshcd_clear_outstanding_reqs() returns 0, as
another path has already claimed the command.

Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI EH")
Cc: stable@vger.kernel.org
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 68b53f89f8c6..ff5977f7da68 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9660,12 +9660,10 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 	 */
 	if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
 		if (!hba->mcq_enabled) {
-			unsigned long flags;
 			struct request *rq = scsi_cmd_to_rq(scmd);
 
-			spin_lock_irqsave(&hba->outstanding_lock, flags);
-			__clear_bit(rq->tag, &hba->outstanding_reqs);
-			spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+			if (!ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag)))
+				return SCSI_EH_DONE;
 		}
 
 		if (ufshcd_is_scsi_cmd(scmd)) {
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort()
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
                   ` (3 preceding siblings ...)
  2026-10-22  3:44 ` [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out() Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22  9:10   ` Peter Wang
  2026-10-22 21:23   ` Bean Huo
  2026-10-22  3:44 ` [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
  2026-10-22 17:11 ` [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Bart Van Assche
  6 siblings, 2 replies; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

In SDB mode, a command that times out right as the controller finishes
it can be completed twice:

1. The command times out and scsi_timeout() schedules ufshcd_abort().
2. The controller finishes the command and clears its doorbell bit in
   REG_UTP_TRANSFER_REQ_DOOR_BELL, while the completion handler has not
   yet cleared BIT(tag) in hba->outstanding_reqs.
3. ufshcd_abort() sees the doorbell bit clear and calls
   __ufshcd_transfer_req_compl() without clearing BIT(tag) in
   hba->outstanding_reqs.
4. The normal completion handler or the logical unit reset that follows
   sees BIT(tag) still set in hba->outstanding_reqs and completes the
   same command a second time.

This branch only exists for a rare corner case, a completion interrupt
that is lost or arrives late. Remove it to simplify the abort path and
leave such a command to the SCSI error handler, like any other command
that cannot be aborted. The cost is a slower recovery in this corner
case, which is acceptable since it only happens after a timeout.

Fixes: 169f5eb28869 ("scsi: ufs: Optimize SCSI command processing")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/fdb5c391-01ef-4a23-94c2-4f3853909348@acm.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ff5977f7da68..87efa2925867 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8180,15 +8180,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	}
 	hba->req_abort_count++;
 
-	if (!hba->mcq_enabled && !(reg & (1 << tag))) {
-		/* only execute this code in single doorbell mode */
-		dev_err(hba->dev,
-		"%s: cmd was completed, but without a notifying intr, tag = %d",
-		__func__, tag);
-		__ufshcd_transfer_req_compl(hba, 1UL << tag);
-		goto release;
-	}
-
 	/*
 	 * Task abort to the device W-LUN is illegal. When this command
 	 * will fail, due to spec violation, scsi err handling next step
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
                   ` (4 preceding siblings ...)
  2026-10-22  3:44 ` [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort() Stanley Jhu
@ 2026-10-22  3:44 ` Stanley Jhu
  2026-10-22 21:24   ` Bean Huo
  2026-10-22 17:11 ` [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Bart Van Assche
  6 siblings, 1 reply; 12+ messages in thread
From: Stanley Jhu @ 2026-10-22  3:44 UTC (permalink / raw)
  To: Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

In SDB mode, aborting a timed-out device management request drops a
clock hold that the request never took:

1. ufshcd_dev_man_lock() takes one ufshcd_hold() for the whole device
   management command. ufshcd_queue_reserved_command() submits the
   request without taking a hold of its own.
2. The request times out and ufshcd_abort() clears its slot with
   ufshcd_clear_cmd().
3. ufshcd_abort() claims BIT(tag) in hba->outstanding_reqs and calls
   ufshcd_release_scsi_cmd(), which calls ufshcd_release() for a hold
   the request never took.
4. When ufshcd_dev_man_unlock() drops the hold from step 1,
   hba->clk_gating.active_reqs goes below zero.

Release the command only if it is a SCSI command.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/2060a8da-581b-46d1-a510-779f4df716cd@acm.org
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 87efa2925867..141ad1be4848 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8226,7 +8226,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	 * Clear the corresponding bit from outstanding_reqs since the command
 	 * has been aborted successfully.
 	 */
-	if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
+	if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)) &&
+	    ufshcd_is_scsi_cmd(cmd))
 		ufshcd_release_scsi_cmd(hba, cmd);
 
 	err = SUCCESS;
-- 
2.56.0.218.g1ec904b173-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort()
  2026-10-22  3:44 ` [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort() Stanley Jhu
@ 2026-10-22  9:10   ` Peter Wang
  2026-10-22 21:23   ` Bean Huo
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Wang @ 2026-10-22  9:10 UTC (permalink / raw)
  To: Stanley Jhu, Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Can Guo, linux-kernel, stable

On Thu, 2026-10-22 at 11:44 +0800, Stanley Jhu wrote:
> In SDB mode, a command that times out right as the controller
> finishes
> it can be completed twice:
> 
> 1. The command times out and scsi_timeout() schedules ufshcd_abort().
> 2. The controller finishes the command and clears its doorbell bit in
>    REG_UTP_TRANSFER_REQ_DOOR_BELL, while the completion handler has
> not
>    yet cleared BIT(tag) in hba->outstanding_reqs.
> 3. ufshcd_abort() sees the doorbell bit clear and calls
>    __ufshcd_transfer_req_compl() without clearing BIT(tag) in
>    hba->outstanding_reqs.
> 4. The normal completion handler or the logical unit reset that
> follows
>    sees BIT(tag) still set in hba->outstanding_reqs and completes the
>    same command a second time.
> 
> This branch only exists for a rare corner case, a completion
> interrupt
> that is lost or arrives late. Remove it to simplify the abort path
> and
> leave such a command to the SCSI error handler, like any other
> command
> that cannot be aborted. The cost is a slower recovery in this corner
> case, which is acceptable since it only happens after a timeout.
> 
> Fixes: 169f5eb28869 ("scsi: ufs: Optimize SCSI command processing")
> Cc: stable@vger.kernel.org
> Link:
> https://lore.kernel.org/r/fdb5c391-01ef-4a23-94c2-4f3853909348@acm.org
>  
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
> 

Reviewed-by: Peter Wang <peter.wang@mediatek.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions
  2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
                   ` (5 preceding siblings ...)
  2026-10-22  3:44 ` [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
@ 2026-10-22 17:11 ` Bart Van Assche
  6 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2026-10-22 17:11 UTC (permalink / raw)
  To: Stanley Jhu, Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Peter Wang,
	Can Guo, linux-kernel, stable

On 10/21/26 8:44 PM, Stanley Jhu wrote:
> In Single Doorbell (SDB) mode, multiple completion and error handling
> paths can race on the same transfer request when bits in
> hba->outstanding_reqs are not claimed atomically with completing the
> request. A stale bit in hba->outstanding_reqs lets a later poll or
> reset complete the request again and unmap its DMA buffers twice.
For the series:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
  2026-10-22  3:44 ` [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out() Stanley Jhu
@ 2026-10-22 18:03   ` Bean Huo
  0 siblings, 0 replies; 12+ messages in thread
From: Bean Huo @ 2026-10-22 18:03 UTC (permalink / raw)
  To: Stanley Jhu, Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

On Thu, 2026-10-22 at 11:44 +0800, Stanley Jhu wrote:
> In SDB mode, a command that times out during a power management
> operation can be released twice:
> 
> 1. The command times out while its normal completion is running.
> 2. The completion path clears BIT(rq->tag) in hba->outstanding_reqs and
>    releases the command. It has not called scsi_done() yet, so
>    SCMD_STATE_COMPLETE is still clear.
> 3. ufshcd_eh_timed_out() recovers the link, sees SCMD_STATE_COMPLETE
>    clear, clears BIT(rq->tag) in hba->outstanding_reqs without checking
>    whether it was still set, and releases the same command a second
>    time.
> 
> Return SCSI_EH_DONE when ufshcd_clear_outstanding_reqs() returns 0, as
> another path has already claimed the command.
> 
> Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI
> EH")
> Cc: stable@vger.kernel.org
> Reviewed-by: Peter Wang <peter.wang@mediatek.com>
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>

Reviewed-by: Bean Huo <beanhuo@micron.cmo>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort()
  2026-10-22  3:44 ` [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort() Stanley Jhu
  2026-10-22  9:10   ` Peter Wang
@ 2026-10-22 21:23   ` Bean Huo
  1 sibling, 0 replies; 12+ messages in thread
From: Bean Huo @ 2026-10-22 21:23 UTC (permalink / raw)
  To: Stanley Jhu, Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

On Thu, 2026-10-22 at 11:44 +0800, Stanley Jhu wrote:
> In SDB mode, a command that times out right as the controller finishes
> it can be completed twice:
> 
> 1. The command times out and scsi_timeout() schedules ufshcd_abort().
> 2. The controller finishes the command and clears its doorbell bit in
>    REG_UTP_TRANSFER_REQ_DOOR_BELL, while the completion handler has not
>    yet cleared BIT(tag) in hba->outstanding_reqs.
> 3. ufshcd_abort() sees the doorbell bit clear and calls
>    __ufshcd_transfer_req_compl() without clearing BIT(tag) in
>    hba->outstanding_reqs.
> 4. The normal completion handler or the logical unit reset that follows
>    sees BIT(tag) still set in hba->outstanding_reqs and completes the
>    same command a second time.
> 
> This branch only exists for a rare corner case, a completion interrupt
> that is lost or arrives late. Remove it to simplify the abort path and
> leave such a command to the SCSI error handler, like any other command
> that cannot be aborted. The cost is a slower recovery in this corner
> case, which is acceptable since it only happens after a timeout.
> 
> Fixes: 169f5eb28869 ("scsi: ufs: Optimize SCSI command processing")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/fdb5c391-01ef-4a23-94c2-4f3853909348@acm.org
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()
  2026-10-22  3:44 ` [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
@ 2026-10-22 21:24   ` Bean Huo
  0 siblings, 0 replies; 12+ messages in thread
From: Bean Huo @ 2026-10-22 21:24 UTC (permalink / raw)
  To: Stanley Jhu, Martin K . Petersen, linux-scsi
  Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
	Peter Wang, Can Guo, linux-kernel, stable

On Thu, 2026-10-22 at 11:44 +0800, Stanley Jhu wrote:
> In SDB mode, aborting a timed-out device management request drops a
> clock hold that the request never took:
> 
> 1. ufshcd_dev_man_lock() takes one ufshcd_hold() for the whole device
>    management command. ufshcd_queue_reserved_command() submits the
>    request without taking a hold of its own.
> 2. The request times out and ufshcd_abort() clears its slot with
>    ufshcd_clear_cmd().
> 3. ufshcd_abort() claims BIT(tag) in hba->outstanding_reqs and calls
>    ufshcd_release_scsi_cmd(), which calls ufshcd_release() for a hold
>    the request never took.
> 4. When ufshcd_dev_man_unlock() drops the hold from step 1,
>    hba->clk_gating.active_reqs goes below zero.
> 
> Release the command only if it is a SCSI command.
> 
> Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/r/2060a8da-581b-46d1-a510-779f4df716cd@acm.org
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Peter Wang <peter.wang@mediatek.com>
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-10-22 21:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22  3:44 [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
2026-10-22  3:44 ` [PATCH v5 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
2026-10-22  3:44 ` [PATCH v5 2/6] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
2026-10-22  3:44 ` [PATCH v5 3/6] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
2026-10-22  3:44 ` [PATCH v5 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out() Stanley Jhu
2026-10-22 18:03   ` Bean Huo
2026-10-22  3:44 ` [PATCH v5 5/6] scsi: ufs: core: Remove completed-without-interrupt path in ufshcd_abort() Stanley Jhu
2026-10-22  9:10   ` Peter Wang
2026-10-22 21:23   ` Bean Huo
2026-10-22  3:44 ` [PATCH v5 6/6] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
2026-10-22 21:24   ` Bean Huo
2026-10-22 17:11 ` [PATCH v5 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Bart Van Assche

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®