Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang (王信友)]

On Fri, 2026-09-25 at 10:26 -0700, Bart Van Assche wrote:
> A correction: the INIT_LIST_HEAD(&cmd->eh_entry) statement is not
> reached while SCSI host recovery is in progress because
> SCMD_STATE_COMPLETE is set for all pending commands before the SCSI
> host recovery starts. scsi_done() skips SCSI commands for which that
> bit is set due to the following code:
> 
>         if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd-
> >state)))
>                 return;
> 
> Hence, the scmd->result value is ignored if scsi_done() is called
> while SCSI host recovery is in progress.
> 
> Bart.

Thanks to Bart and Stanley for the detailed explanation.
It is clear to me now.

Thanks
Peter

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang (王信友)]

[-- Attachment #1: winmail.dat --]
[-- Type: application/ms-tnef, Size: 12707 bytes --]

[PATCH] scsi: ufs: core: Avoid unsafe MMIO reads in ufshcd_mcq_compl_all_cqes_lock()

[Stanley Jhu]

Reading host controller registers (such as CQTP) is unsafe when the
host controller is disabled (HCE = 0), as accessing registers in an
unclocked or reset state can cause bus stalls and system hangs.

In ufshcd_mcq_compl_all_cqes_lock(), all completion queue entries have
already been inspected, processed, and cleared in memory, so the
software queue is logically empty.

Avoid the unsafe MMIO read of CQTP by synchronizing hwq->cq_tail_slot
directly to hwq->cq_head_slot in software. Upon subsequent controller
re-initialization, ufshcd_mcq_make_queues_operational() will
reconfigure and re-zero all queue pointers.

Fixes: ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufs-mcq.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 8106d55f4041..0e1f99c8d77c 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -349,8 +349,12 @@ void ufshcd_mcq_compl_all_cqes_lock(struct ufs_hba *hba,
                entries--;
        }

-       ufshcd_mcq_update_cq_tail_slot(hwq);
-       hwq->cq_head_slot = hwq->cq_tail_slot;
+       /*
+        * All completion entries have been processed and cleared.
+        * Synchronize tail to head in software to mark the queue empty,
+        * avoiding unsafe MMIO reads while the controller is disabled.
+        */
+       hwq->cq_tail_slot = hwq->cq_head_slot;
        spin_unlock_irqrestore(&hwq->cq_lock, flags);
 }

--
2.43.0

[PATCH v2 0/2] scsi: ufs: core: Fix unsafe MMIO reads and redundant CQ sweeps in MCQ reset

[Stanley Jhu]

During Multi-Circular Queue (MCQ) error recovery and host reset,
ufshcd_mcq_compl_pending_transfer() sweeps or polls completion queues to
reap pending transfers. Two bugs exist in this path:

1. Unsafe MMIO read and spurious errors while HCE = 0 (Patch 1/2):
   ufshcd_host_reset_and_restore() stops the controller (HCE = 0) before
   calling ufshcd_mcq_compl_all_cqes_lock(). Calling
   ufshcd_mcq_update_cq_tail_slot() at the end of the sweep reads CQTPy
   over MMIO while HCE = 0, directly contradicting the function's own
   documented contract that reading host controller registers is unsafe
   when the controller is disabled. In addition, passing expected empty
   slots during a full-ring sweep into ufshcd_mcq_process_cqe() prints
   spurious "Abnormal CQ entry!" errors.

2. Redundant per-request CQ sweeps and polls (Patch 2/2):
   ufshcd_mcq_compl_pending_transfer() runs hardware queue completion
   sweeps (force_compl == true) or CQTPy polls (force_compl == false)
   inside blk_mq_tagset_busy_iter() callbacks, repeating whole-queue
   operations once per busy request instead of once per hardware queue.

Patch 1/2 synchronizes hwq->cq_tail_slot = hwq->cq_head_slot in software
and extracts ufshcd_mcq_compl_cqe() so full-ring sweeps skip empty slots
silently. Patch 2/2 sweeps or polls each hardware queue once before
iterating residual requests and removes ufshcd_mcq_compl_one().

Changes since v1:
- Split into a two-patch series separating ring sweep safety from
  per-request tagset iteration.
- Extract ufshcd_mcq_compl_cqe() to skip empty slots without double CQE
  checks (dropped Peter Wang's v1 Reviewed-by due to this change).
- Decouple hardware queue polling/sweeping for both force_compl paths
  and remove ufshcd_mcq_compl_one().

Tested: Verified MCQ host reset, I/O completion, and queue pointer
  integrity on QEMU ARM64 without MMIO aborts or spurious error logs.

Link: https://lore.kernel.org/r/CAE14pdek6ynze+muDZrK+yNX-3ioe3vprxOA4W22qokg352tJQ@mail.gmail.com

Stanley Jhu (2):
  scsi: ufs: core: Avoid unsafe MMIO reads in
    ufshcd_mcq_compl_all_cqes_lock()
  scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

 drivers/ufs/core/ufs-mcq.c | 35 +++++++++++++++++++++++------------
 drivers/ufs/core/ufshcd.c  | 31 ++++++++++++-------------------
 2 files changed, 35 insertions(+), 31 deletions(-)

-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Stanley Jhu]

In MCQ mode, ufshcd_mcq_compl_pending_transfer() uses
blk_mq_tagset_busy_iter() to iterate over busy requests during error
recovery and host reset. However, both iterator callbacks perform
whole-queue operations redundantly for each visited request:

- force_compl == true: ufshcd_mcq_force_compl_one() calls
  ufshcd_mcq_compl_all_cqes_lock() on every busy request, sweeping the
  entire completion ring (hwq->max_entries slots) once per active
  request under spin_lock_irqsave even though the first sweep already
  cleared all completion entries.
- force_compl == false: ufshcd_mcq_compl_one() acquires cq_lock and
  polls CQTPy over MMIO via ufshcd_mcq_poll_cqe_lock() for every busy
  request without doing any per-request work.

Sweep or poll each hardware queue (hba->uhq[i]) once at the start of
ufshcd_mcq_compl_pending_transfer(). When force_compl is true, run
blk_mq_tagset_busy_iter() afterward to complete residual in-flight
requests with DID_REQUEUE, and remove the now-unused
ufshcd_mcq_compl_one() callback.

Fixes: ab248643d3d6 ("scsi: ufs: core: Add error handling for MCQ mode")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ba244cf40ac..a69dcb04d985 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6044,8 +6044,6 @@ static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
 	if (blk_mq_is_reserved_rq(rq) || !hwq)
 		return true;
 
-	ufshcd_mcq_compl_all_cqes_lock(hba, hwq);
-
 	/*
 	 * For those cmds of which the cqes are not present in the cq, complete
 	 * them explicitly.
@@ -6061,19 +6059,6 @@ static bool ufshcd_mcq_force_compl_one(struct request *rq, void *priv)
 	return true;
 }
 
-static bool ufshcd_mcq_compl_one(struct request *rq, void *priv)
-{
-	struct scsi_device *sdev = rq->q->queuedata;
-	struct Scsi_Host *shost = sdev->host;
-	struct ufs_hba *hba = shost_priv(shost);
-	struct ufs_hw_queue *hwq = ufshcd_mcq_req_to_hwq(hba, rq);
-
-	if (!blk_mq_is_reserved_rq(rq) && hwq)
-		ufshcd_mcq_poll_cqe_lock(hba, hwq);
-
-	return true;
-}
-
 /**
  * ufshcd_mcq_compl_pending_transfer - MCQ mode function. It is
  * invoked from the error handler context or ufshcd_host_reset_and_restore()
@@ -6088,10 +6073,18 @@ static bool ufshcd_mcq_compl_one(struct request *rq, void *priv)
 static void ufshcd_mcq_compl_pending_transfer(struct ufs_hba *hba,
 					      bool force_compl)
 {
-	blk_mq_tagset_busy_iter(&hba->host->tag_set,
-				force_compl ? ufshcd_mcq_force_compl_one :
-					      ufshcd_mcq_compl_one,
-				NULL);
+	int i;
+
+	for (i = 0; i < hba->nr_hw_queues; i++) {
+		if (force_compl)
+			ufshcd_mcq_compl_all_cqes_lock(hba, &hba->uhq[i]);
+		else
+			ufshcd_mcq_poll_cqe_lock(hba, &hba->uhq[i]);
+	}
+
+	if (force_compl)
+		blk_mq_tagset_busy_iter(&hba->host->tag_set,
+					ufshcd_mcq_force_compl_one, NULL);
 }
 
 /**
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/18/26 7:38 AM, Stanley Jhu wrote:
> In MCQ mode, ufshcd_mcq_compl_pending_transfer() uses
> blk_mq_tagset_busy_iter() to iterate over busy requests during error
> recovery and host reset. However, both iterator callbacks perform
> whole-queue operations redundantly for each visited request:
> 
> - force_compl == true: ufshcd_mcq_force_compl_one() calls
>    ufshcd_mcq_compl_all_cqes_lock() on every busy request, sweeping the
>    entire completion ring (hwq->max_entries slots) once per active
>    request under spin_lock_irqsave even though the first sweep already
>    cleared all completion entries.
> - force_compl == false: ufshcd_mcq_compl_one() acquires cq_lock and
>    polls CQTPy over MMIO via ufshcd_mcq_poll_cqe_lock() for every busy
>    request without doing any per-request work.
> 
> Sweep or poll each hardware queue (hba->uhq[i]) once at the start of
> ufshcd_mcq_compl_pending_transfer(). When force_compl is true, run
> blk_mq_tagset_busy_iter() afterward to complete residual in-flight
> requests with DID_REQUEUE, and remove the now-unused
> ufshcd_mcq_compl_one() callback.

Forcibly completing SCSI commands from inside the UFS SCSI host reset
error handling callback is incompatible with the SCSI core error
handler. The "force_compl" behavior should be removed instead of
reworking it. If you take a look at the SDB (single doorbell) code you
will see that forcibly completing requests doesn't happen for SDB mode.

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Stanley Jhu]

On 9/18/26 9:06 AM, Bart Van Assche wrote:
> Forcibly completing SCSI commands from inside the UFS SCSI host reset
> error handling callback is incompatible with the SCSI core error
> handler. The "force_compl" behavior should be removed instead of
> reworking it. If you take a look at the SDB (single doorbell) code you
> will see that forcibly completing requests doesn't happen for SDB mode.

Note that ufshcd_mcq_force_compl_one() already checks
!test_bit(SCMD_STATE_COMPLETE, &cmd->state), so it only completes non-EH
commands (e.g. during an autonomous ufshcd_err_handler() reset); in SDB
mode, ufshcd_hba_stop() (HCE = 0) clears UTRLDBR to 0, so ufshcd_poll()
similarly treats all outstanding_reqs as completed right after
ufshcd_hba_stop().

That said, doing this inside ufshcd_host_reset_and_restore() right after
ufshcd_hba_stop() is indeed the wrong place: at controller stop time we
should only release LLD resources (ufshcd_release_scsi_cmd()), and
requeue any remaining non-EH (!SCMD_STATE_COMPLETE) commands with
DID_REQUEUE only after host/link recovery finishes (so autonomous resets
neither wake callers mid-reset nor leave in-flight I/O stalled for the
30s block layer timeout, similar to autonomous reset handling in
hisi_sas, megaraid_sas, and smartpqi).

Let's drop this v2 series. I will send a separate 2-patch series shortly
to replace this and remove force_compl and
ufshcd_mcq_compl_all_cqes_lock().

Thanks,

Stanley

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/20/26 6:49 AM, Stanley Jhu wrote:
> On 9/18/26 9:06 AM, Bart Van Assche wrote:
>> Forcibly completing SCSI commands from inside the UFS SCSI host reset
>> error handling callback is incompatible with the SCSI core error
>> handler. The "force_compl" behavior should be removed instead of
>> reworking it. If you take a look at the SDB (single doorbell) code you
>> will see that forcibly completing requests doesn't happen for SDB mode.
> 
> Note that ufshcd_mcq_force_compl_one() already checks
> !test_bit(SCMD_STATE_COMPLETE, &cmd->state), so it only completes non-EH
> commands (e.g. during an autonomous ufshcd_err_handler() reset);

What are "non-EH" commands? The SCSI error handler only starts its
error handling strategy after all pending commands have either timed
out or completed.

I think that you are misunderstanding the code. The purpose of the
SCMD_STATE_COMPLETE check is to prevent double completions of SCSI
commands.

> in SDB mode, ufshcd_hba_stop() (HCE = 0) clears UTRLDBR to 0, so
> ufshcd_poll() similarly treats all outstanding_reqs as completed
> right after ufshcd_hba_stop().
> 
> That said, doing this inside ufshcd_host_reset_and_restore() right after
> ufshcd_hba_stop() is indeed the wrong place: at controller stop time we
> should only release LLD resources (ufshcd_release_scsi_cmd()), and
> requeue any remaining non-EH (!SCMD_STATE_COMPLETE) commands with
> DID_REQUEUE only after host/link recovery finishes (so autonomous resets
> neither wake callers mid-reset nor leave in-flight I/O stalled for the
> 30s block layer timeout, similar to autonomous reset handling in
> hisi_sas, megaraid_sas, and smartpqi).

In SDB mode, clearing UTRLDBR will cause all pending commands to be
requeued because the OCS member is initialized to 
OCS_INVALID_COMMAND_STATUS and because ufshcd_transfer_rsp_status()
translates this status value into DID_REQUEUE << 16. I'm concerned
that this approach may cause the deadlines for SCSI commands to be
exceeded. Hence my proposal for MCQ mode not to requeue pending SCSI
commands but instead to let the SCSI error handler decide what to do
with these commands.

Thanks,

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang]

On Mon, 2026-09-21 at 10:13 -0700, Bart Van Assche wrote:
> In SDB mode, clearing UTRLDBR will cause all pending commands to be
> requeued because the OCS member is initialized to
> OCS_INVALID_COMMAND_STATUS and because ufshcd_transfer_rsp_status()
> translates this status value into DID_REQUEUE << 16. I'm concerned
> that this approach may cause the deadlines for SCSI commands to be
> exceeded. Hence my proposal for MCQ mode not to requeue pending SCSI
> commands but instead to let the SCSI error handler decide what to do
> with these commands.
> 

Hi Bart,

The current MCQ flow is similar to SDB. We discussed this two years
ago.
Aborted commands will be requeued immediately, just like in SDB mode
https://patchwork.kernel.org/project/linux-scsi/list/?series=894411&state=*

Thanks
Peter
 

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/22/26 12:37 AM, Peter Wang wrote:
> On Mon, 2026-09-21 at 10:13 -0700, Bart Van Assche wrote:
>> In SDB mode, clearing UTRLDBR will cause all pending commands to be
>> requeued because the OCS member is initialized to
>> OCS_INVALID_COMMAND_STATUS and because ufshcd_transfer_rsp_status()
>> translates this status value into DID_REQUEUE << 16. I'm concerned
>> that this approach may cause the deadlines for SCSI commands to be
>> exceeded. Hence my proposal for MCQ mode not to requeue pending SCSI
>> commands but instead to let the SCSI error handler decide what to do
>> with these commands.
> 
> The current MCQ flow is similar to SDB. We discussed this two years
> ago.
> Aborted commands will be requeued immediately, just like in SDB mode
A SCSI host controller reset must abort all pending commands. Requeuing
SCSI commands during a host controller reset violates the API contract
between the SCSI error handler and SCSI LLDs. I'm not aware of any other
SCSI LLD than the UFS host controller driver that requeues pending
commands upon a host controller reset instead of aborting these.

.eh_host_reset_handler() is expected to reset all logical units and the
host controller. The SCSI architecture manual (SAM) mentions explicitly
that resetting logical units involves aborting all commands.

Thanks,

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang]

On Tue, 2026-09-22 at 09:30 -0700, Bart Van Assche wrote:
> A SCSI host controller reset must abort all pending commands.
> Requeuing
> SCSI commands during a host controller reset violates the API
> contract
> between the SCSI error handler and SCSI LLDs. I'm not aware of any
> other
> SCSI LLD than the UFS host controller driver that requeues pending
> commands upon a host controller reset instead of aborting these.
> 
> .eh_host_reset_handler() is expected to reset all logical units and
> the
> host controller. The SCSI architecture manual (SAM) mentions
> explicitly
> that resetting logical units involves aborting all commands.
> 
> Thanks,
> 
> Bart.

Hi Bart,

I'm not sure if I misunderstood your point. SCSI reset 
(ufshcd_eh_host_reset_handler) does indeed abort all commands
by trigger ufshcd_err_handler and call ufshcd_abort_all.
This shouldn't violate the SCSI API contract. But whether the 
subsequent re-queue (DID_REQUEUE) violates, I haven't seen any 
prohibition?

Because currently the UFS error handler (ufshcd_err_handler) 
directly re-queues (By set DID_REQUEUE) the unfinish command,
and err handler might be triggered in many places, for example, 
like UIC errors, directly resetting and re-queueing should be
reasonable and can be handled quickly. 

If it cannot directly re-queue, then ufshcd_eh_host_reset_handler
would need another similar function that does not handle re-queueing.
But this looks unnecessary, direct re-queuing seems safe?

Thanks
Peter

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Stanley Jhu]

On Wed, 2026-09-23 at 14:09 +0800, Peter Wang wrote:
> SCSI reset
> (ufshcd_eh_host_reset_handler) does indeed abort all commands
> by trigger ufshcd_err_handler and call ufshcd_abort_all.
> This shouldn't violate the SCSI API contract. But whether the
> subsequent re-queue (DID_REQUEUE) violates, I haven't seen any
> prohibition?

One detail in ufshcd_err_handler(): ufshcd_eh_host_reset_handler()
sets hba->force_reset, which jumps directly to do_reset and skips
ufshcd_abort_all() (which only aborts commands when no reset is
needed).

On the reset paths, the DID_REQUEUE after ufshcd_hba_stop() does
not describe what happened to those commands. In SDB it is just
the untouched OCS_INVALID_COMMAND_STATUS placeholder from
ufshcd_prepare_req_desc_hdr() that MCQ later mirrored.

Would DID_RESET be a better fit after a controller reset (including
ufshcd_eh_timed_out() right after ufshcd_link_recovery()), similar
to mpi3mr_flush_scmd() and mpt3sas's _scsih_flush_running_cmds()?
It reports what the driver actually knows at that point without
pulling the command into the SCSI error handler.

Thanks,
Stanley

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/23/26 4:18 AM, Stanley Jhu wrote:
> Would DID_RESET be a better fit after a controller reset (including
> ufshcd_eh_timed_out() right after ufshcd_link_recovery()), similar
> to mpi3mr_flush_scmd() and mpt3sas's _scsih_flush_running_cmds()?
> It reports what the driver actually knows at that point without
> pulling the command into the SCSI error handler.

Calling scsi_done() from a .eh_host_reset_handler() implementation is
not allowed because calling scsi_done() for a command that is on the
SCSI error handler list corrupts that list. One of the functions called
by scsi_done() is scsi_complete(). From that function:

	INIT_LIST_HEAD(&cmd->eh_entry);

This corrupts the command lists maintained by the SCSI error handler,
e.g. work_q and done_q.

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/23/26 10:02 AM, Bart Van Assche wrote:
> On 9/23/26 4:18 AM, Stanley Jhu wrote:
>> Would DID_RESET be a better fit after a controller reset (including
>> ufshcd_eh_timed_out() right after ufshcd_link_recovery()), similar
>> to mpi3mr_flush_scmd() and mpt3sas's _scsih_flush_running_cmds()?
>> It reports what the driver actually knows at that point without
>> pulling the command into the SCSI error handler.
> 
> Calling scsi_done() from a .eh_host_reset_handler() implementation is
> not allowed because calling scsi_done() for a command that is on the
> SCSI error handler list corrupts that list. One of the functions called
> by scsi_done() is scsi_complete(). From that function:
> 
>      INIT_LIST_HEAD(&cmd->eh_entry);
> 
> This corrupts the command lists maintained by the SCSI error handler,
> e.g. work_q and done_q.

A correction: the INIT_LIST_HEAD(&cmd->eh_entry) statement is not
reached while SCSI host recovery is in progress because
SCMD_STATE_COMPLETE is set for all pending commands before the SCSI
host recovery starts. scsi_done() skips SCSI commands for which that
bit is set due to the following code:

	if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
		return;

Hence, the scmd->result value is ignored if scsi_done() is called
while SCSI host recovery is in progress.

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang]

On Fri, 2026-09-25 at 10:26 -0700, Bart Van Assche wrote:
> A correction: the INIT_LIST_HEAD(&cmd->eh_entry) statement is not
> reached while SCSI host recovery is in progress because
> SCMD_STATE_COMPLETE is set for all pending commands before the SCSI
> host recovery starts. scsi_done() skips SCSI commands for which that
> bit is set due to the following code:
> 
>         if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd-
> >state)))
>                 return;
> 
> Hence, the scmd->result value is ignored if scsi_done() is called
> while SCSI host recovery is in progress.
> 
> Bart.

Thanks to Bart and Stanley for the detailed explanation.
It is clear to me now.

Thanks
Peter

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/22/26 11:09 PM, Peter Wang wrote:
> But whether the subsequent re-queue (DID_REQUEUE) violates, I
> haven't seen any prohibition?
While this is not written down explicitly anywhere as far as I
know, requeuing from inside the host error handler is not allowed
at all.
> Because currently the UFS error handler (ufshcd_err_handler)
> directly re-queues (By set DID_REQUEUE) the unfinish command,
> and err handler might be triggered in many places, for example,
> like UIC errors, directly resetting and re-queueing should be
> reasonable and can be handled quickly.

Have you noticed the following code in drivers/scsi/scsi_error.c?

	scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);

> If it cannot directly re-queue, then ufshcd_eh_host_reset_handler
> would need another similar function that does not handle re-queueing.
> But this looks unnecessary, direct re-queuing seems safe?

Requeuing from the .eh_host_reset_handler() is not safe at all. It may
trigger list corruption as follows:
1. The .eh_host_reset_handler() callback sets scmd->result to
    DID_REQUEUE << 16 and calls scsi_done(scmd). This causes
    blk_mq_requeue_request() to insert the SCSI command into
    &q->requeue_list. The SCSI command stays there because the request
    queue is not run immediately because the SCSI host is in the state
    SHOST_RECOVERY.
2. The .eh_host_reset_handler() callback finishes and returns SUCCESS.
3. The SCSI EH thread in scsi_eh_host_reset() sees that the reset
    succeeded and moves cmd to done_q.
4. The SCSI EH thread executes scsi_eh_flush_done_q(done_q).
5. scsi_eh_flush_done_q() invokes
    scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY) for the same scmd
    that is already present on the request queue requeue list.
6. scsi_queue_insert() calls __scsi_queue_insert(), which invokes
    blk_mq_requeue_request() a second time. This causes list corruption.
    Code that examines the corrupted requeue list will either crash or
    trigger an infinite loop.

Bart.

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Peter Wang]

On Sun, 2026-09-20 at 21:49 +0800, Stanley Jhu wrote:
> That said, doing this inside ufshcd_host_reset_and_restore() right
> after
> ufshcd_hba_stop() is indeed the wrong place: at controller stop time


Hi Stanley,

I have different idea. The original design was that only after
the HBA stop can we ensure the host will no longer process the
remaining commands (those that need to be forcibly requeued)
due to receiving IRQs or other factors, thus avoiding unnecessary
racing. Both SDB and MCQ modes are implemented based on this idea.
However, MCQ indeed does not need to read the CQ register to update
the CQ, because it holds no meaning after the host reset either.

Thanks
Peter

Re: [PATCH v2 2/2] scsi: ufs: core: Decouple CQ sweep from request iterator in MCQ

[Bart Van Assche]

On 9/18/26 7:38 AM, Stanley Jhu wrote:
> Sweep or poll each hardware queue (hba->uhq[i]) once at the start of
> ufshcd_mcq_compl_pending_transfer(). When force_compl is true, run
> blk_mq_tagset_busy_iter() afterward to complete residual in-flight
> requests with DID_REQUEUE, and remove the now-unused
> ufshcd_mcq_compl_one() callback.

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