* [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions
@ 2026-10-20 8:21 Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
` (6 more replies)
0 siblings, 7 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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 converts ufshcd_eh_timed_out() to the helper as a cleanup.
Patch 5 uses the helper return value in ufshcd_eh_timed_out() to prevent
releasing driver resources for a request already completed by another
context.
Patch 6 uses the helper in ufshcd_abort() to fix a double release in
the missing-interrupt path.
Patch 7 skips releasing reserved requests at the end of ufshcd_abort().
This series is based on mkp/for-next (db8b4d2bf2d3).
Tested on QEMU ARM64 UFS in SDB mode across LU reset, abort, and PM
timeout paths without double DMA unmaps.
Changes since v2:
- Move WARN_ONCE() removal to 1/7 and use __must_hold()/guard() (Bart).
- Add patch 5 to fix a double release in ufshcd_eh_timed_out().
- Add patch 6 to fix a double release in ufshcd_abort() (Bean, Bart).
- Add patch 7 to skip releasing reserved requests in ufshcd_abort().
- Link to v2: https://lore.kernel.org/r/20261017084403.875420-1-stanleyjhu@google.com
Stanley Jhu (7):
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: Use ufshcd_clear_outstanding_reqs() in
ufshcd_eh_timed_out()
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 | 56 ++++++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 21 deletions(-)
base-commit: db8b4d2bf2d3e02b4ff78bbf69d00e11795840de
--
2.56.0.212.g129ede20d9-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 16:53 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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.
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 | 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 16:58 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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>
---
v3: Add __must_hold() and use guard(spinlock_irqsave) (Bart Van Assche).
drivers/ufs/core/ufshcd.c | 39 +++++++++++++++++++++++++++------------
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 12aafcf1f9b3..1b28dbba03a9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6052,6 +6052,29 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
}
}
+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;
+}
+
+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.
@@ -6071,12 +6094,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;
+ completed_reqs =
+ __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
+ queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
spin_unlock_irqrestore(&hba->outstanding_lock, flags);
if (completed_reqs)
@@ -8111,7 +8131,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 +8225,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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 19:57 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 4/7] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out() Stanley Jhu
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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>
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 1b28dbba03a9..778932298e27 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7978,7 +7978,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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 4/7] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
` (2 preceding siblings ...)
2026-10-20 8:21 ` [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 5/7] scsi: ufs: core: Fix double release " Stanley Jhu
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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_eh_timed_out() has its own copy of the code that clears the
timed-out tag in hba->outstanding_reqs. Use
ufshcd_clear_outstanding_reqs() instead. No functional change.
Cc: stable@vger.kernel.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 | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 778932298e27..03efbd85bba9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9651,12 +9651,9 @@ 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);
+ ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag));
}
if (ufshcd_is_scsi_cmd(scmd)) {
--
2.56.0.212.g129ede20d9-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 5/7] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
` (3 preceding siblings ...)
2026-10-20 8:21 ` [PATCH v3 4/7] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out() Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 17:11 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort() Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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
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 03efbd85bba9..111f5805f178 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9653,7 +9653,8 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
if (!hba->mcq_enabled) {
struct request *rq = scsi_cmd_to_rq(scmd);
- ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag));
+ 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
` (4 preceding siblings ...)
2026-10-20 8:21 ` [PATCH v3 5/7] scsi: ufs: core: Fix double release " Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 20:13 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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 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() and release the command only if
ufshcd_abort() claimed it and it is a SCSI command.
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 | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 111f5805f178..ee898f4a5bb1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8176,7 +8176,9 @@ 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)) &&
+ ufshcd_is_scsi_cmd(cmd))
+ ufshcd_release_scsi_cmd(hba, cmd);
goto release;
}
--
2.56.0.212.g129ede20d9-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
` (5 preceding siblings ...)
2026-10-20 8:21 ` [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort() Stanley Jhu
@ 2026-10-20 8:21 ` Stanley Jhu
2026-10-20 20:16 ` Bart Van Assche
6 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:21 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
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 ee898f4a5bb1..5d1a9b4e9b8d 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8228,7 +8228,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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE()
2026-10-20 8:21 ` [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
@ 2026-10-20 16:53 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 16:53 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/20/26 1:21 AM, Stanley Jhu wrote:
> 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.
Patch 2/7 has a Cc: stable tag so this patch should have that tag too.
Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
2026-10-20 8:21 ` [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
@ 2026-10-20 16:58 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 16:58 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/20/26 1:21 AM, Stanley Jhu wrote:
> +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;
> +}
> +
> +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);
> +}
Please add a short comment above the above two functions that explains
the meaning of the return value.
> /*
> * Return: > 0 if one or more commands have been completed or 0 if no
> * requests have been completed.
> @@ -6071,12 +6094,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;
> + completed_reqs =
> + __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
> + queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
The above line is about 98 columns wide. That's too much - all other
code in this driver is 80 columns wide. One way to address this is by
introducing a local "from_irq" boolean variable:
+ from_irq = queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT;
+ completed_reqs = __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
+ from_irq);
Otherwise this patch looks good to me.
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 5/7] scsi: ufs: core: Fix double release in ufshcd_eh_timed_out()
2026-10-20 8:21 ` [PATCH v3 5/7] scsi: ufs: core: Fix double release " Stanley Jhu
@ 2026-10-20 17:11 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 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/20/26 1:21 AM, Stanley Jhu wrote:
> Fixes: 01d5e237b339 ("scsi: ufs: core: Handle PM commands timeout before SCSI EH")
> Cc: stable@vger.kernel.org
This patch modifies the same code as patch 4/7 while patch 4/7 does not
have a Cc: stable tag. Hence, this patch can't be applied on stable
kernels without patch 4/7. Please combine both patches into a single
patch. Otherwise this patch looks good to me.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
2026-10-20 8:21 ` [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
@ 2026-10-20 19:57 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 19:57 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/20/26 1:21 AM, Stanley Jhu wrote:
> 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.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort()
2026-10-20 8:21 ` [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort() Stanley Jhu
@ 2026-10-20 20:13 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 20:13 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/20/26 1:21 AM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 111f5805f178..ee898f4a5bb1 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8176,7 +8176,9 @@ 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)) &&
> + ufshcd_is_scsi_cmd(cmd))
> + ufshcd_release_scsi_cmd(hba, cmd);
> goto release;
> }
This change is incomplete. "err = SUCCESS" is missing above "goto
release". Without this change, the SCSI error handler will escalate
error handling unnecessarily to the next level.
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request in ufshcd_abort()
2026-10-20 8:21 ` [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
@ 2026-10-20 20:16 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-20 20:16 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/20/26 1:21 AM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ee898f4a5bb1..5d1a9b4e9b8d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8228,7 +8228,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;
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-10-20 20:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-20 8:21 [PATCH v3 0/7] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completions Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 1/7] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
2026-10-20 16:53 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 2/7] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
2026-10-20 16:58 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 3/7] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
2026-10-20 19:57 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 4/7] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out() Stanley Jhu
2026-10-20 8:21 ` [PATCH v3 5/7] scsi: ufs: core: Fix double release " Stanley Jhu
2026-10-20 17:11 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 6/7] scsi: ufs: core: Fix double release in ufshcd_abort() Stanley Jhu
2026-10-20 20:13 ` Bart Van Assche
2026-10-20 8:21 ` [PATCH v3 7/7] scsi: ufs: core: Do not release a reserved request " Stanley Jhu
2026-10-20 20:16 ` 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®