Skip to contentSkip to conversation

scsi: ufs: core: Fix a deadlock between clock scaling and device commands

PATCH Applied Joonhoe Kim 3 participants 4 messages Oct 16 – Oct 29

Subsystems (4)
Stable branch SCSI Arm Qualcomm mailing list UFS host controller
lore
Share & tools
lore

Joonhoe Kim

author
#0 lore raw

PATCH 1/1 scsi: ufs: core: Fix a deadlock between clock scaling and device commands

Since commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()"), device management commands go through blk_execute_rq() on the pseudo SCSI device, which shares the tagset with the logical units, while ufshcd_dev_man_lock() holds clk_scaling_lock for reading.

ufshcd_clock_scaling_prepare() and ufshcd_pause_command_processing() quiesce the tagset before taking clk_scaling_lock for writing. A device management command holding the lock for reading at that point is never dispatched and never times out, so the writer waits forever with all logical units quiesced. On a Lenovo Legion Tab Y700 Gen 5 this ended in a hung task panic.

Take clk_scaling_lock before quiescing the tagset, and in ufshcd_clock_scaling_prepare() also wb_mutex, which is always taken before clk_scaling_lock. Release them in the reverse order. Commit b52d695d0620 ("scsi: ufs: core: Keep internal commands dispatchable during error handling") fixed the same problem in the error handler.

Fixes 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Fixes dc5dcac53278 ("scsi: ufs: core: Add helpers to pause and resume command processing")
Cc [email protected]
Signed-off-by Joonhoe Kim <[email protected]>

---
Reproduced on v7.3-rc4 with out-of-tree board support, using short fio write bursts and readers of the exception_event_status attribute while devfreq scales: a deadlock within 10 s without this patch, two clean 600 s runs with it. The pause path was only checked by reading the code, and the reversed release order only by building it. Sashiko flagged this lock order in [1] and [2].

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

 drivers/ufs/core/ufshcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/ufs/core/ufshcd.c +6 -6
index cc75384c6502..87f78ffc8cbd 100644
@@ -1360,13 +1360,13 @@ int ufshcd_pause_command_processing(struct ufs_hba *hba, u64 timeout_us)
 	int ret = 0;
 
 	mutex_lock(&hba->host->scan_mutex);
-	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	down_write(&hba->clk_scaling_lock);
+	blk_mq_quiesce_tagset(&hba->host->tag_set);
 
 	if (ufshcd_wait_for_pending_cmds(hba, timeout_us)) {
 		ret = -EBUSY;
-		up_write(&hba->clk_scaling_lock);
 		blk_mq_unquiesce_tagset(&hba->host->tag_set);
+		up_write(&hba->clk_scaling_lock);
 		mutex_unlock(&hba->host->scan_mutex);
 	}
 

@@ -1383,8 +1383,8 @@ void ufshcd_resume_command_processing(struct ufs_hba *hba)
 	__releases(&hba->clk_scaling_lock)
 	__releases(&hba->host->scan_mutex)
 {
-	up_write(&hba->clk_scaling_lock);
 	blk_mq_unquiesce_tagset(&hba->host->tag_set);
+	up_write(&hba->clk_scaling_lock);
 	mutex_unlock(&hba->host->scan_mutex);
 }
 

@@ -1461,16 +1461,16 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 	 * clock scaling is in progress
 	 */
 	mutex_lock(&hba->host->scan_mutex);
-	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	mutex_lock(&hba->wb_mutex);
 	down_write(&hba->clk_scaling_lock);
+	blk_mq_quiesce_tagset(&hba->host->tag_set);
 
 	if (!hba->clk_scaling.is_allowed ||
 	    ufshcd_wait_for_pending_cmds(hba, timeout_us)) {
 		ret = -EBUSY;
+		blk_mq_unquiesce_tagset(&hba->host->tag_set);
 		up_write(&hba->clk_scaling_lock);
 		mutex_unlock(&hba->wb_mutex);
-		blk_mq_unquiesce_tagset(&hba->host->tag_set);
 		mutex_unlock(&hba->host->scan_mutex);
 		goto out;
 	}

@@ -1487,9 +1487,9 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err)
 	__releases(&hba->wb_mutex)
 	__releases(&hba->host->scan_mutex)
 {
+	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	up_write(&hba->clk_scaling_lock);
 	mutex_unlock(&hba->wb_mutex);
-	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	mutex_unlock(&hba->host->scan_mutex);
 
 	/* Enable Write Booster if current gear requires it else disable it */
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw

On Fri, 2026-10-16 at 21:46 +0900, Joonhoe Kim wrote

Quoted 22 lines

Since commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()"), device management commands go through blk_execute_rq() on the pseudo SCSI device, which shares the tagset with the logical units, while ufshcd_dev_man_lock() holds clk_scaling_lock for reading.

ufshcd_clock_scaling_prepare() and ufshcd_pause_command_processing() quiesce the tagset before taking clk_scaling_lock for writing. A device
management command holding the lock for reading at that point is never
dispatched and never times out, so the writer waits forever with all logical units quiesced. On a Lenovo Legion Tab Y700 Gen 5 this ended in
a hung task panic.

Take clk_scaling_lock before quiescing the tagset, and in ufshcd_clock_scaling_prepare() also wb_mutex, which is always taken before clk_scaling_lock. Release them in the reverse order. Commit b52d695d0620 ("scsi: ufs: core: Keep internal commands dispatchable during error handling") fixed the same problem in the error handler.

Quoted 7 lines
Fixes 08b12cda6c44 ("scsi: ufs: core: Switch to

scsi_get_internal_cmd()")

Fixes dc5dcac53278 ("scsi: ufs: core: Add helpers to pause and

resume command processing")

Cc [email protected]
Signed-off-by Joonhoe Kim <[email protected]>

---

Reviewed-by Peter Wang <[email protected]>
maintainer Applied
#2 lore raw
In reply to Joonhoe Kim

Joonhoe,

Quoted 5 lines

Since commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()"), device management commands go through blk_execute_rq() on the pseudo SCSI device, which shares the tagset with the logical units, while ufshcd_dev_man_lock() holds clk_scaling_lock for reading.

Applied to 7.4/scsi-staging, thanks!

Signature · 1 line
Martin K. Petersen
maintainer Applied
#3 lore raw
In reply to Joonhoe Kim
Quoted 6 lines · Joonhoe Kim

Since commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()"), device management commands go through blk_execute_rq() on the pseudo SCSI device, which shares the tagset with the logical units, while ufshcd_dev_man_lock() holds clk_scaling_lock for reading.

Quoted 8 lines

ufshcd_clock_scaling_prepare() and ufshcd_pause_command_processing() quiesce the tagset before taking clk_scaling_lock for writing. A device management command holding the lock for reading at that point is never dispatched and never times out, so the writer waits forever with all logical units quiesced. On a Lenovo Legion Tab Y700 Gen 5 this ended in a hung task panic.

[...]

Applied to 7.4/scsi-queue, thanks!

[1/1] scsi: ufs: core: Fix a deadlock between clock scaling and device commands

      https://git.kernel.org/mkp/scsi/c/2327df6d2a29
Signature · 1 line
Martin K. Petersen