[PATCH v4 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions

[PATCH v4 0/6] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions

[Stanley Jhu]

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 uses the helper in ufshcd_abort() to fix a double release in
the missing-interrupt path and returns SUCCESS when it claims the bit.

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 LU reset, abort, and PM timeout
paths without double DMA unmaps or active_reqs underflow.

Changes since v3:
- Document the helper return value in patch 2 and add Cc: stable to
  patch 1 (Bart).
- Squash v3 patches 4 and 5 into patch 4 (Bart).
- Return SUCCESS when ufshcd_abort() claims BIT(tag) in patch 5 (Bart).
- Link to v3:
  https://lore.kernel.org/r/20261020082110.3511797-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: Fix double release in ufshcd_abort()
  scsi: ufs: core: Do not release a reserved request in ufshcd_abort()

 drivers/ufs/core/ufshcd.c | 67 +++++++++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 21 deletions(-)


base-commit: db8b4d2bf2d3e02b4ff78bbf69d00e11795840de
-- 
2.56.0.212.g129ede20d9-goog

[PATCH v4 1/6] scsi: ufs: core: Remove an always-false WARN_ONCE()

[Stanley Jhu]

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.212.g129ede20d9-goog

[PATCH v4 2/6] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs

[Stanley Jhu]

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.212.g129ede20d9-goog

[PATCH v4 3/6] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()

[Stanley Jhu]

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.212.g129ede20d9-goog

[PATCH v4 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()

[Stanley Jhu]

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
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.212.g129ede20d9-goog

[PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Stanley Jhu]

In SDB mode, a timed-out command whose completion interrupt has not
been handled yet can be released twice:

1. The command times out and scsi_timeout() schedules ufshcd_abort().
2. The command finishes in hardware. Its doorbell bit is clear, but the
   completion interrupt has not been handled, so BIT(tag) is still set
   in hba->outstanding_reqs.
3. ufshcd_abort() sees the doorbell bit clear and calls
   __ufshcd_transfer_req_compl(). That releases the command but leaves
   BIT(tag) set in hba->outstanding_reqs. ufshcd_abort() returns
   FAILED.
4. The next ufshcd_poll() call, or the logical unit reset that follows
   the FAILED, sees BIT(tag) set in hba->outstanding_reqs and releases
   the same command again.

Claim BIT(tag) in hba->outstanding_reqs with
ufshcd_clear_outstanding_reqs(), release the command if it is a SCSI
command, and return SUCCESS when ufshcd_abort() claims the bit.

Fixes: 169f5eb28869 ("scsi: ufs: Optimize SCSI command processing")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/2e042211ca0ebc974ad5ec4c0de9f3f93b3c75c0.camel@iokpp.de
Suggested-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ff5977f7da68..4f9b34c1d112 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8185,7 +8185,11 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		dev_err(hba->dev,
 		"%s: cmd was completed, but without a notifying intr, tag = %d",
 		__func__, tag);
-		__ufshcd_transfer_req_compl(hba, 1UL << tag);
+		if (ufshcd_clear_outstanding_reqs(hba, BIT(tag))) {
+			if (ufshcd_is_scsi_cmd(cmd))
+				ufshcd_release_scsi_cmd(hba, cmd);
+			err = SUCCESS;
+		}
 		goto release;
 	}
 
-- 
2.56.0.212.g129ede20d9-goog

[PATCH v4 6/6] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()

[Stanley Jhu]

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>
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 4f9b34c1d112..49a981b8cc32 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8239,7 +8239,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.212.g129ede20d9-goog

Re: [PATCH v4 4/6] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()

[Peter Wang]

On Wed, 2026-10-21 at 12:06 +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
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---

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

Re: [PATCH v4 6/6] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()

[Peter Wang]

On Wed, 2026-10-21 at 12:06 +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>
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---

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

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Peter Wang]

On Wed, 2026-10-21 at 12:06 +0800, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ff5977f7da68..4f9b34c1d112 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8185,7 +8185,11 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  		dev_err(hba->dev,
>  		"%s: cmd was completed, but without a notifying
> intr, tag = %d",
>  		__func__, tag);
> -		__ufshcd_transfer_req_compl(hba, 1UL << tag);
> +		if (ufshcd_clear_outstanding_reqs(hba, BIT(tag))) {
> +			if (ufshcd_is_scsi_cmd(cmd))
> +				ufshcd_release_scsi_cmd(hba, cmd);
> +			err = SUCCESS;
> +		}
> 

Hi Stanley,

If ufshcd_clear_outstanding_reqs(hba, BIT(tag)) returns 0, 
should we also set err = SUCCESS? Since the tag is already done
— either by the ISR or by ufshcd_abort — shouldn't both cases 
be treated the same?

Thanks
Peter

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Stanley Jhu]

On Wed, 2026-10-21 at 21:12 +0800, Peter Wang wrote:
> If ufshcd_clear_outstanding_reqs(hba, BIT(tag)) returns 0,
> should we also set err = SUCCESS? Since the tag is already done
> — either by the ISR or by ufshcd_abort — shouldn't both cases
> be treated the same?

A return value of 0 here means the completion path, rather than
this abort path, has claimed the tag and may still be running
ufshcd_release_scsi_cmd() after it dropped outstanding_lock.
SUCCESS tells the SCSI error handler it can retry or finish the
command right away, so a retry could map the same command again
while the completion path is still unmapping it.

Returning FAILED here also matches the check at the top of
ufshcd_abort(), which already returns FAILED when the tag is no
longer set in hba->outstanding_reqs.

Thanks,
Stanley

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Bart Van Assche]

On 10/21/26 6:28 AM, Stanley Jhu wrote:
> A return value of 0 here means the completion path, rather than
> this abort path, has claimed the tag and may still be running
> ufshcd_release_scsi_cmd() after it dropped outstanding_lock.
Do we really need the code that checks that a command has completed
without notification interrupt? I have never encountered any logs on
any test setup that indicates that this happened.

Thanks,

Bart.

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Stanley Jhu]

On 10/21/26 9:37 AM, Bart Van Assche wrote:
> Do we really need the code that checks that a command has completed
> without notification interrupt? I have never encountered any logs on
> any test setup that indicates that this happened.

Agreed, we do not need this branch, and v5 removes it. If a completion
interrupt is lost or arrives after the command times out, leaving the
command to the SCSI error handler makes recovery slower in that rare
corner case, which is acceptable since it only happens after a timeout.

I will send v5 shortly.

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Peter Wang]

On Wed, 2026-10-21 at 08:28 -0500, Stanley Jhu wrote:
> On Wed, 2026-10-21 at 21:12 +0800, Peter Wang wrote:
> > If ufshcd_clear_outstanding_reqs(hba, BIT(tag)) returns 0,
> > should we also set err = SUCCESS? Since the tag is already done
> > — either by the ISR or by ufshcd_abort — shouldn't both cases
> > be treated the same?
> 
> A return value of 0 here means the completion path, rather than
> this abort path, has claimed the tag and may still be running
> ufshcd_release_scsi_cmd() after it dropped outstanding_lock.
> SUCCESS tells the SCSI error handler it can retry or finish the
> command right away, so a retry could map the same command again
> while the completion path is still unmapping it.
> 
> Returning FAILED here also matches the check at the top of
> ufshcd_abort(), which already returns FAILED when the tag is no
> longer set in hba->outstanding_reqs.
> 
> Thanks,
> Stanley

Okay, that is correct. Thanks for the explanation.

Peter

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Peter Wang]

On Wed, 2026-10-21 at 09:37 -0700, Bart Van Assche wrote:
> On 10/21/26 6:28 AM, Stanley Jhu wrote:
> > A return value of 0 here means the completion path, rather than
> > this abort path, has claimed the tag and may still be running
> > ufshcd_release_scsi_cmd() after it dropped outstanding_lock.
> Do we really need the code that checks that a command has completed
> without notification interrupt? I have never encountered any logs on
> any test setup that indicates that this happened.
> 
> Thanks,
> 
> Bart.

I remember that when the SSU timeout was only 1 sec, we'd pretty
easily hit the concurrent execution case between the timeout and
the complete path. But after change it to 10 sec along with patch 
9a5f6c09d0fa (scsi: ufs: core: Simplify ufshcd_try_to_abort_task()), 
it shouldn't happen anymore.

Thanks
Peter

Re: [PATCH v4 5/6] scsi: ufs: core: Fix double release in ufshcd_abort()

[Peter Wang (王信友)]

On Wed, 2026-10-21 at 08:28 -0500, Stanley Jhu wrote:
> On Wed, 2026-10-21 at 21:12 +0800, Peter Wang wrote:
> > If ufshcd_clear_outstanding_reqs(hba, BIT(tag)) returns 0,
> > should we also set err = SUCCESS? Since the tag is already done
> > — either by the ISR or by ufshcd_abort — shouldn't both cases
> > be treated the same?
> 
> A return value of 0 here means the completion path, rather than
> this abort path, has claimed the tag and may still be running
> ufshcd_release_scsi_cmd() after it dropped outstanding_lock.
> SUCCESS tells the SCSI error handler it can retry or finish the
> command right away, so a retry could map the same command again
> while the completion path is still unmapping it.
> 
> Returning FAILED here also matches the check at the top of
> ufshcd_abort(), which already returns FAILED when the tag is no
> longer set in hba->outstanding_reqs.
> 
> Thanks,
> Stanley

Okay, that is correct. Thanks for the explanation.

Peter