[PATCH v2] scsi: ufs: core: Quiesce SCSI devices instead of host tagset during error handling

[PATCH v2] scsi: ufs: core: Quiesce SCSI devices instead of host tagset during error handling

[Stanley Jhu]

Commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
switched UFS internal commands to allocate requests on
hba->host->pseudo_sdev->request_queue, which shares the host tagset with
regular LUNs.

During error recovery, ufshcd_err_handling_prepare() calls
blk_mq_quiesce_tagset(&hba->host->tag_set), marking all queues in the
tagset as quiesced, including pseudo_sdev->request_queue. When
ufshcd_verify_dev_init() subsequently issues internal commands (e.g. NOP
OUT UPIU) via blk_execute_rq(), blk_mq_run_hw_queue() skips running the
quiesced queue, resulting in an unrecoverable circular wait deadlock.

Resolve this by quiescing only the logical units attached to the host via
shost_for_each_device() during error handling. Because
shost_for_each_device() skips pseudo SCSI devices (via
scsi_device_is_pseudo_dev() in __scsi_iterate_devices()), pseudo_sdev
stays unquiesced and internal commands can be dispatched and completed.

Keeping the logical units quiesced preserves behaviour that predates the
current call. Until commit 2a36646012fc ("scsi: ufs: core: Simplify
ufshcd_err_handling_prepare()"), the prepare path called
ufshcd_scsi_block_requests() to hold off new commands in addition to
blk_mq_wait_quiesce_done() to wait for the ongoing ones; folding both
into blk_mq_quiesce_tagset() kept that behaviour. Waiting without
quiescing would drop the first half, so the queues stay quiesced here.

Clock scaling and command pause (ufshcd_pause_command_processing) paths
continue to use blk_mq_quiesce_tagset() as they require freezing all
queues including internal commands.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/6f78c4bd-a70b-402d-abfd-599091b67674@acm.org/
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
v2:
- Commit message only; no functional change.
- Fix the Link: tag, which cited a Message-ID that is not in the archive.
- Explain why the logical units stay quiesced, referring to the behaviour
  that predates 2a36646012fc, instead of the vague claim about retry storms.

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

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a25b6a0cb5c6..1ca731675ad1 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6781,6 +6781,23 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
 	}
 }
 
+static void ufshcd_quiesce_all_sdevs(struct ufs_hba *hba)
+{
+	struct scsi_device *sdev;
+
+	shost_for_each_device(sdev, hba->host)
+		blk_mq_quiesce_queue_nowait(sdev->request_queue);
+	blk_mq_wait_quiesce_done(&hba->host->tag_set);
+}
+
+static void ufshcd_unquiesce_all_sdevs(struct ufs_hba *hba)
+{
+	struct scsi_device *sdev;
+
+	shost_for_each_device(sdev, hba->host)
+		blk_mq_unquiesce_queue(sdev->request_queue);
+}
+
 static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 {
 	/*
@@ -6816,13 +6833,13 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 		ufshcd_clk_scaling_allow(hba, false);
 	}
 	/* Wait for ongoing ufshcd_queuecommand() calls to finish. */
-	blk_mq_quiesce_tagset(&hba->host->tag_set);
+	ufshcd_quiesce_all_sdevs(hba);
 	cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
-	blk_mq_unquiesce_tagset(&hba->host->tag_set);
+	ufshcd_unquiesce_all_sdevs(hba);
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, false);
-- 
2.55.0.1007.g17ff1f9808-goog

Re: [PATCH v2] scsi: ufs: core: Quiesce SCSI devices instead of host tagset during error handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Switching from blk_mq_unquiesce_tagset() to shost_for_each_device() permanently freezes queues of devices being removed, deadlocking the removal path.
--

commit b89bd79e56222f4a0cb23f7f3bd5ee7ab397b8c4
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: core: Quiesce SCSI devices instead of host tagset during error handling

This commit changes UFS error handling to quiesce only logical units instead
of the entire host tagset. By iterating over active devices, it allows internal
commands to be dispatched on the pseudo SCSI device during error recovery,
preventing a circular wait deadlock.

Link: https://lore.kernel.org/all/6f78c4bd-a70b-402d-abfd-599091b67674@acm.org/

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 234e18b5078f2..b5c63e88af140 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
[ ... ]
> @@ -6805,6 +6805,23 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
[ ... ]
> +static void ufshcd_unquiesce_all_sdevs(struct ufs_hba *hba)
> +{
> +	struct scsi_device *sdev;
> +
> +	shost_for_each_device(sdev, hba->host)
> +		blk_mq_unquiesce_queue(sdev->request_queue);

[Severity: High]
Will this loop skip devices that are in the process of being removed, leaving
their request queues permanently quiesced?

Looking at __scsi_iterate_devices(), which is called by shost_for_each_device(),
it uses scsi_device_get() to acquire a reference to the next device.

If a device is in the SDEV_CANCEL state due to a concurrent removal event
(such as sysfs or hot-unplug), scsi_device_get() returns a negative error.
This causes __scsi_iterate_devices() to skip the device and continue searching:

drivers/scsi/scsi.c:__scsi_iterate_devices() {
    ...
    if (!scsi_device_is_pseudo_dev(next) && !scsi_device_get(next))
        break;
    ...
}

If a device removal occurs concurrently with an error event,
ufshcd_err_handling_prepare() might successfully quiesce the device's queue
before or during its transition to SDEV_CANCEL.

Later, when ufshcd_err_handling_unprepare() calls ufshcd_unquiesce_all_sdevs(),
the removed device is skipped, and its queue is never unquiesced.

The device removal thread will eventually call device_del() -> sd_remove() ->
sd_shutdown() -> sd_sync_cache(). This issues a SYNCHRONIZE CACHE command via
blk_execute_rq().

Since the queue is permanently quiesced, the request is never dispatched and
the block layer timeout timer never starts.

Could this cause the removal thread to deadlock indefinitely in blk_wait_io()
waiting for the completion of the SYNCHRONIZE CACHE command?

> +}
> +
>  static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912094107.2098303-1-stanleyjhu@google.com?part=1

Re: [PATCH v2] scsi: ufs: core: Quiesce SCSI devices instead of host tagset during error handling

[Stanley Jhu]

On Sat, 12 Sep 2026 10:03:24 +0000, sashiko-bot@kernel.org wrote:
> Will this loop skip devices that are in the process of being removed, leaving
> their request queues permanently quiesced?

Confirmed.

The old code unquiesced every queue in the tagset, including queues of
devices already being removed: a queue leaves set->tag_list only in
blk_mq_exit_queue(), which runs after device_del().
shost_for_each_device() skips those devices instead, because
scsi_device_get() fails once a device is in SDEV_CANCEL or SDEV_DEL. Their
queues then stay quiesced forever, and sd_shutdown() -> sd_sync_cache()
hangs in blk_execute_rq() with no timeout, since the request is never
started.

v3 takes a different approach. It keeps blk_mq_quiesce_tagset() as before
and only unquiesces hba->host->pseudo_sdev on top of it, so internal
commands stay dispatchable while the logical units remain quiesced. Clock
scaling and ufshcd_pause_command_processing() are unchanged.

Bart's Reviewed-by is dropped from v3 since the implementation changed.

Thanks,
Stanley Jhu