[PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Ziqi Chen]

When preparing for UFS clock scaling, the UFS driver will quiesce all sdevs
queues in the UFS SCSI host tagset list and then unquiesce them when UFS
clock scaling unpreparing. If the UFS SCSI host async scan is in progress
at this time, some LUs may be added to the tagset list between UFS clkscale
prepare and unprepare. This can cause two issues:

1. During clock scaling, there may be IO requests issued through new added
queues that have not been quiesced, leading to task abort issue.

2. These new added queues that have not been quiesced will be unquiesced as
well when UFS clkscale is unprepared, resulting in warning prints.

Therefore, use the mutex lock scan_mutex in ufshcd_clock_scaling_prepare()
and ufshcd_clock_scaling_unprepare() to protect it.

Co-developed-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
Suggested-by: Bart Van Assche <bvanassche@acm.org>

---
v1 -> v2:
Move whole clkscale Initialize process out of ufshcd_add_lus().

v2 -> v3:
Add check for the return value of ufshcd_add_lus().

v3 -> v4:
1. Using lock 'scan_mutex' instead of checking flag 'scan_mutex'.
2. Update patch name and commit message.
---
 drivers/ufs/core/ufshcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d7ff24b48de3..a7513f256057 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1397,6 +1397,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 	 * make sure that there are no outstanding requests when
 	 * 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);
@@ -1407,6 +1408,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
 		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;
 	}
 
@@ -1428,6 +1430,7 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err)
 	mutex_unlock(&hba->wb_mutex);
 
 	blk_mq_unquiesce_tagset(&hba->host->tag_set);
+	mutex_unlock(&hba->host->scan_mutex);
 	ufshcd_release(hba);
 }
 
-- 
2.34.1

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Bart Van Assche]

On 5/22/25 1:12 AM, Ziqi Chen wrote:
> When preparing for UFS clock scaling, the UFS driver will quiesce all sdevs
> queues in the UFS SCSI host tagset list and then unquiesce them when UFS
> clock scaling unpreparing. If the UFS SCSI host async scan is in progress
> at this time, some LUs may be added to the tagset list between UFS clkscale
> prepare and unprepare. This can cause two issues:
> 
> 1. During clock scaling, there may be IO requests issued through new added
> queues that have not been quiesced, leading to task abort issue.
> 
> 2. These new added queues that have not been quiesced will be unquiesced as
> well when UFS clkscale is unprepared, resulting in warning prints.
> 
> Therefore, use the mutex lock scan_mutex in ufshcd_clock_scaling_prepare()
> and ufshcd_clock_scaling_unprepare() to protect it.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Martin K. Petersen]

Ziqi,

> When preparing for UFS clock scaling, the UFS driver will quiesce all
> sdevs queues in the UFS SCSI host tagset list and then unquiesce them
> when UFS clock scaling unpreparing. If the UFS SCSI host async scan is
> in progress at this time, some LUs may be added to the tagset list
> between UFS clkscale prepare and unprepare. This can cause two issues:

Applied to 6.16/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Martin K. Petersen]

On Thu, 22 May 2025 16:12:28 +0800, Ziqi Chen wrote:

> When preparing for UFS clock scaling, the UFS driver will quiesce all sdevs
> queues in the UFS SCSI host tagset list and then unquiesce them when UFS
> clock scaling unpreparing. If the UFS SCSI host async scan is in progress
> at this time, some LUs may be added to the tagset list between UFS clkscale
> prepare and unprepare. This can cause two issues:
> 
> 1. During clock scaling, there may be IO requests issued through new added
> queues that have not been quiesced, leading to task abort issue.
> 
> [...]

Applied to 6.16/scsi-queue, thanks!

[1/1] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress
      https://git.kernel.org/mkp/scsi/c/e97633492f5a

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Thu, 2025-05-22 at 16:12 +0800, Ziqi Chen wrote:
> When preparing for UFS clock scaling, the UFS driver will quiesce all
> sdevs
> queues in the UFS SCSI host tagset list and then unquiesce them when
> UFS
> clock scaling unpreparing. If the UFS SCSI host async scan is in
> progress
> at this time, some LUs may be added to the tagset list between UFS
> clkscale
> prepare and unprepare. This can cause two issues:
> 
> 1. During clock scaling, there may be IO requests issued through new
> added
> queues that have not been quiesced, leading to task abort issue.
> 
> 2. These new added queues that have not been quiesced will be
> unquiesced as
> well when UFS clkscale is unprepared, resulting in warning prints.
> 
> Therefore, use the mutex lock scan_mutex in
> ufshcd_clock_scaling_prepare()
> and ufshcd_clock_scaling_unprepare() to protect it.
> 
> Co-developed-by: Can Guo <quic_cang@quicinc.com>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
> Suggested-by: Bart Van Assche <bvanassche@acm.org>


Hi Ziqi,

With this patch will have circular locking dependency detected.
Here is the backtrace log.
Could consider luns_avail instead mutex?


[   96.496464] [T00600001343] mtkPowerMsgHdl:
======================================================
[   96.498411] [T00600001343] mtkPowerMsgHdl: WARNING: possible
circular locking dependency detected
[   96.500774] [T00600001343] mtkPowerMsgHdl: 6.12.30-android16-5-
g137b86aeb5da-4k #1 Tainted: G           OE     
[   96.504046] [T00600001343] mtkPowerMsgHdl: -------------------------
-----------------------------
[   96.506071] [T00600001343] mtkPowerMsgHdl: mtkPowerMsgHdl/1343 is
trying to acquire lock:
[   96.508301] [T00600001343] ffffff80461f0078
[   96.513718] [T00600001343]  (
[   96.515361] [T00600001343] &devfreq->lock
[   96.516493] [T00600001343] ){+.+.}-{3:3}, at:
qos_max_notifier_call+0x2c/0x98
[   96.517759] [T00600001343] mtkPowerMsgHdl: 
[   96.517759] [T00600001343] but task is already holding lock:
[   96.519313] [T00600001343] ffffff804584b9c8
[   96.521194] [T00600001343]  (
[   96.522295] [T00600001343] &(c->notifiers)->rwsem
[   96.523243] [T00600001343] #2
[   96.524168] [T00600001343] ){++++}-{3:3}
[   96.525203] [T00600001343] , at:
blocking_notifier_call_chain+0x50/0x94
[   96.526268] [T00600001343] mtkPowerMsgHdl: 
[   96.526268] [T00600001343] which lock already depends on the new
lock.
[   96.526268] [T00600001343] 
[   96.527670] [T00600001343] mtkPowerMsgHdl: 
[   96.527670] [T00600001343] the existing dependency chain (in reverse
order) is:
[   96.529671] [T00600001343] mtkPowerMsgHdl: 
[   96.529671] [T00600001343] -> #5
[   96.531759] [T00600001343]  (
[   96.533247] [T00600001343] &(c->notifiers)->rwsem
[   96.534195] [T00600001343] #2
[   96.535121] [T00600001343] ){++++}-{3:3}
[   96.536155] [T00600001343] :
[   96.537223] [T00600001343] mtkPowerMsgHdl:       
down_write+0xa0/0x334
[   96.538159] [T00600001343] mtkPowerMsgHdl:       
blocking_notifier_chain_register+0x5c/0xc0
[   96.539324] [T00600001343] mtkPowerMsgHdl:       
freq_qos_add_notifier+0x94/0xac
[   96.541043] [T00600001343] mtkPowerMsgHdl:       
cpufreq_online+0x220/0x1754
[   96.542554] [T00600001343] mtkPowerMsgHdl:       
cpufreq_add_dev+0x84/0x138
[   96.544024] [T00600001343] mtkPowerMsgHdl:       
subsys_interface_register+0x240/0x30c
[   96.545806] [T00600001343] mtkPowerMsgHdl:       
cpufreq_register_driver+0x2f8/0x478
[   96.547383] [T00600001343] mtkPowerMsgHdl:       
mtk_cpufreq_hw_driver_probe+0x684/0x7d4 [mediatek_cpufreq_hw]
[   96.548396] [T00600001343] mtkPowerMsgHdl:       
platform_probe+0x100/0x160
[   96.550189] [T00600001343] mtkPowerMsgHdl:       
really_probe+0x1b4/0x74c
[   96.551787] [T00600001343] mtkPowerMsgHdl:       
__driver_probe_device+0x158/0x304
[   96.553331] [T00600001343] mtkPowerMsgHdl:       
driver_probe_device+0x70/0x298
[   96.554766] [T00600001343] mtkPowerMsgHdl:       
__driver_attach+0x238/0x518
[   96.556299] [T00600001343] mtkPowerMsgHdl:       
bus_for_each_dev+0x1b4/0x20c
[   96.557885] [T00600001343] mtkPowerMsgHdl:       
driver_attach+0x48/0x58
[   96.559356] [T00600001343] mtkPowerMsgHdl:       
bus_add_driver+0x280/0x56c
[   96.560836] [T00600001343] mtkPowerMsgHdl:       
driver_register+0x15c/0x29c
[   96.562348] [T00600001343] mtkPowerMsgHdl:       
__platform_driver_register+0x68/0x7c
[   96.563805] [T00600001343] mtkPowerMsgHdl:        0xffffffc00b2d5038
[   96.565274] [T00600001343] mtkPowerMsgHdl:       
do_one_initcall+0x214/0x778
[   96.566818] [T00600001343] mtkPowerMsgHdl:       
do_init_module+0x1cc/0x57c
[   96.568697] [T00600001343] mtkPowerMsgHdl:       
load_module+0x2658/0x3068
[   96.571034] [T00600001343] mtkPowerMsgHdl:       
__arm64_sys_finit_module+0x3e0/0x5a4
[   96.572707] [T00600001343] mtkPowerMsgHdl:       
invoke_syscall+0x78/0x250
[   96.574567] [T00600001343] mtkPowerMsgHdl:       
el0_svc_common+0x154/0x1c4
[   96.576512] [T00600001343] mtkPowerMsgHdl:       
do_el0_svc+0x48/0x58
[   96.578597] [T00600001343] mtkPowerMsgHdl:        el0_svc+0x50/0xd0
[   96.580563] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync_handler+0x70/0xbc
[   96.582129] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync+0x188/0x18c
[   96.589641] [T00600001343] mtkPowerMsgHdl: 
[   96.589641] [T00600001343] -> #4 (subsys mutex#2){+.+.}-{3:3}:
[   96.589653] [T00600001343] mtkPowerMsgHdl:       
__mutex_lock_common+0x1dc/0x371c
[   96.589659] [T00600001343] mtkPowerMsgHdl:       
mutex_lock_nested+0x2c/0x38
[   96.589665] [T00600001343] mtkPowerMsgHdl:       
subsys_interface_register+0x160/0x30c
[   96.589670] [T00600001343] mtkPowerMsgHdl:       
cpufreq_register_driver+0x2f8/0x478
[   96.589675] [T00600001343] mtkPowerMsgHdl:       
mtk_cpufreq_hw_driver_probe+0x684/0x7d4 [mediatek_cpufreq_hw]
[   96.589687] [T00600001343] mtkPowerMsgHdl:       
platform_probe+0x100/0x160
[   96.589694] [T00600001343] mtkPowerMsgHdl:       
really_probe+0x1b4/0x74c
[   96.589699] [T00600001343] mtkPowerMsgHdl:       
__driver_probe_device+0x158/0x304
[   96.589704] [T00600001343] mtkPowerMsgHdl:       
driver_probe_device+0x70/0x298
[   96.589709] [T00600001343] mtkPowerMsgHdl:       
__driver_attach+0x238/0x518
[   96.589714] [T00600001343] mtkPowerMsgHdl:       
bus_for_each_dev+0x1b4/0x20c
[   96.589718] [T00600001343] mtkPowerMsgHdl:       
driver_attach+0x48/0x58
[   96.589723] [T00600001343] mtkPowerMsgHdl:       
bus_add_driver+0x280/0x56c
[   96.589727] [T00600001343] mtkPowerMsgHdl:       
driver_register+0x15c/0x29c
[   96.589732] [T00600001343] mtkPowerMsgHdl:       
__platform_driver_register+0x68/0x7c
[   96.589738] [T00600001343] mtkPowerMsgHdl:        0xffffffc00b2d5038
[   96.589753] [T00600001343] mtkPowerMsgHdl:       
do_one_initcall+0x214/0x778
[   96.589758] [T00600001343] mtkPowerMsgHdl:       
do_init_module+0x1cc/0x57c
[   96.589763] [T00600001343] mtkPowerMsgHdl:       
load_module+0x2658/0x3068
[   96.589767] [T00600001343] mtkPowerMsgHdl:       
__arm64_sys_finit_module+0x3e0/0x5a4
[   96.589772] [T00600001343] mtkPowerMsgHdl:       
invoke_syscall+0x78/0x250
[   96.589777] [T00600001343] mtkPowerMsgHdl:       
el0_svc_common+0x154/0x1c4
[   96.589782] [T00600001343] mtkPowerMsgHdl:       
do_el0_svc+0x48/0x58
[   96.589787] [T00600001343] mtkPowerMsgHdl:        el0_svc+0x50/0xd0
[   96.589791] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync_handler+0x70/0xbc
[   96.589795] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync+0x188/0x18c
[   96.589799] [T00600001343] mtkPowerMsgHdl: 
[   96.589799] [T00600001343] -> #3 (cpu_hotplug_lock){++++}-{0:0}:
[   96.589808] [T00600001343] mtkPowerMsgHdl:       
cpus_read_lock+0x54/0x1e8
[   96.589815] [T00600001343] mtkPowerMsgHdl:       
__cpuhp_state_add_instance+0x24/0x54
[   96.589822] [T00600001343] mtkPowerMsgHdl:       
blk_mq_alloc_and_init_hctx+0x940/0xbec
[   96.589831] [T00600001343] mtkPowerMsgHdl:       
blk_mq_realloc_hw_ctxs+0x290/0x9cc
[   96.589841] [T00600001343] mtkPowerMsgHdl:       
blk_mq_init_allocated_queue+0x31c/0x1020
[   96.589849] [T00600001343] mtkPowerMsgHdl:       
__blk_mq_alloc_disk+0x138/0x2b0
[   96.589855] [T00600001343] mtkPowerMsgHdl:       
loop_add+0x2ac/0x840
[   96.589863] [T00600001343] mtkPowerMsgHdl:       
loop_init+0xe8/0x10c
[   96.589873] [T00600001343] mtkPowerMsgHdl:       
do_one_initcall+0x214/0x778
[   96.589884] [T00600001343] mtkPowerMsgHdl:       
do_initcall_level+0x188/0x380
[   96.589895] [T00600001343] mtkPowerMsgHdl:       
do_initcalls+0x74/0x138
[   96.589903] [T00600001343] mtkPowerMsgHdl:       
do_basic_setup+0x78/0x8c
[   96.589909] [T00600001343] mtkPowerMsgHdl:       
kernel_init_freeable+0x260/0x374
[   96.589916] [T00600001343] mtkPowerMsgHdl:       
kernel_init+0x20/0x1cc
[   96.639355] [T00600001343] mtkPowerMsgHdl:       
ret_from_fork+0x10/0x20
[   96.640209] [T00600001343] mtkPowerMsgHdl: 
[   96.640209] [T00600001343] -> #2 (&q->sysfs_lock){+.+.}-{3:3}:
[   96.641460] [T00600001343] mtkPowerMsgHdl:       
__mutex_lock_common+0x1dc/0x371c
[   96.642409] [T00600001343] mtkPowerMsgHdl:       
mutex_lock_nested+0x2c/0x38
[   96.643303] [T00600001343] mtkPowerMsgHdl:       
blk_mq_realloc_hw_ctxs+0x94/0x9cc
[   96.644261] [T00600001343] mtkPowerMsgHdl:       
blk_mq_init_allocated_queue+0x31c/0x1020
[   96.645296] [T00600001343] mtkPowerMsgHdl:       
blk_mq_alloc_queue+0x130/0x214
[   96.646222] [T00600001343] mtkPowerMsgHdl:       
scsi_alloc_sdev+0x708/0xad4
[   96.647119] [T00600001343] mtkPowerMsgHdl:       
scsi_probe_and_add_lun+0x20c/0x27b4
[   96.648096] [T00600001343] mtkPowerMsgHdl:       
__scsi_add_device+0xec/0x178
[   96.648103] [T00600001343] mtkPowerMsgHdl:       
ufshcd_async_scan+0xd4/0x994
[   96.651784] [T00600001343] mtkPowerMsgHdl:       
async_run_entry_fn+0x90/0x364
[   96.652697] [T00600001343] mtkPowerMsgHdl:       
process_one_work+0x6b8/0x1274
[   96.652703] [T00600001343] mtkPowerMsgHdl:       
worker_thread+0x9d4/0xea0
[   96.655784] [T00600001343] mtkPowerMsgHdl:       
kthread+0x298/0x3d8
[   96.656595] [T00600001343] mtkPowerMsgHdl:       
ret_from_fork+0x10/0x20
[   96.657449] [T00600001343] mtkPowerMsgHdl: 
[   96.657449] [T00600001343] -> #1 (&shost->scan_mutex){+.+.}-{3:3}:
[   96.658752] [T00600001343] mtkPowerMsgHdl:       
__mutex_lock_common+0x1dc/0x371c
[   96.659701] [T00600001343] mtkPowerMsgHdl:       
mutex_lock_nested+0x2c/0x38
[   96.660596] [T00600001343] mtkPowerMsgHdl:       
ufshcd_devfreq_scale+0xe0/0x3d0  <== here
[   96.661534] [T00600001343] mtkPowerMsgHdl:       
ufshcd_devfreq_target+0x594/0xa08
[   96.662494] [T00600001343] mtkPowerMsgHdl:       
devfreq_set_target+0x188/0x4b4
[   96.663431] [T00600001343] mtkPowerMsgHdl:       
devfreq_update_target+0x144/0x1a0
[   96.664394] [T00600001343] mtkPowerMsgHdl:       
devfreq_monitor+0x40/0x31c   <== here
[   96.665281] [T00600001343] mtkPowerMsgHdl:       
process_one_work+0x6b8/0x1274
[   96.666196] [T00600001343] mtkPowerMsgHdl:       
worker_thread+0x9d4/0xea0
[   96.667069] [T00600001343] mtkPowerMsgHdl:       
kthread+0x298/0x3d8
[   96.667875] [T00600001343] mtkPowerMsgHdl:       
ret_from_fork+0x10/0x20
[   96.668729] [T00600001343] mtkPowerMsgHdl: 
[   96.668729] [T00600001343] -> #0 (&devfreq->lock){+.+.}-{3:3}:
[   96.669977] [T00600001343] mtkPowerMsgHdl:       
__lock_acquire+0x2e10/0x76d0
[   96.670881] [T00600001343] mtkPowerMsgHdl:       
lock_acquire+0x1d0/0x550
[   96.671740] [T00600001343] mtkPowerMsgHdl:       
__mutex_lock_common+0x1dc/0x371c
[   96.672689] [T00600001343] mtkPowerMsgHdl:       
mutex_lock_nested+0x2c/0x38
[   96.673582] [T00600001343] mtkPowerMsgHdl:       
qos_max_notifier_call+0x2c/0x98   <=== here
[   96.674522] [T00600001343] mtkPowerMsgHdl:       
notifier_call_chain+0x128/0x388
[   96.675460] [T00600001343] mtkPowerMsgHdl:       
blocking_notifier_call_chain+0x68/0x94   <=== here
[   96.676474] [T00600001343] mtkPowerMsgHdl:       
pm_qos_update_target+0x3c0/0x5d4
[   96.677421] [T00600001343] mtkPowerMsgHdl:       
freq_qos_apply+0xa8/0xd8
[   96.678283] [T00600001343] mtkPowerMsgHdl:       
apply_constraint+0xa0/0x1a4
[   96.679178] [T00600001343] mtkPowerMsgHdl:       
__dev_pm_qos_update_request+0x1b4/0x340
[   96.680202] [T00600001343] mtkPowerMsgHdl:       
dev_pm_qos_update_request+0x3c/0x60
[   96.681182] [T00600001343] mtkPowerMsgHdl:       
max_freq_store+0xf4/0x130
[   96.682052] [T00600001343] mtkPowerMsgHdl:       
dev_attr_store+0x64/0x84
[   96.682912] [T00600001343] mtkPowerMsgHdl:       
sysfs_kf_write+0x19c/0x224
[   96.683798] [T00600001343] mtkPowerMsgHdl:       
kernfs_fop_write_iter+0x254/0x408
[   96.684755] [T00600001343] mtkPowerMsgHdl:       
vfs_write+0x674/0x954
[   96.685585] [T00600001343] mtkPowerMsgHdl:       
ksys_write+0xdc/0x174
[   96.686411] [T00600001343] mtkPowerMsgHdl:       
__arm64_sys_write+0x78/0x8c
[   96.687302] [T00600001343] mtkPowerMsgHdl:       
invoke_syscall+0x78/0x250
[   96.688174] [T00600001343] mtkPowerMsgHdl:       
el0_svc_common+0x154/0x1c4
[   96.689056] [T00600001343] mtkPowerMsgHdl:       
do_el0_svc+0x48/0x58
[   96.689872] [T00600001343] mtkPowerMsgHdl:        el0_svc+0x50/0xd0
[   96.690655] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync_handler+0x70/0xbc
[   96.691579] [T00600001343] mtkPowerMsgHdl:       
el0t_64_sync+0x188/0x18c
[   96.692438] [T00600001343] mtkPowerMsgHdl: 
[   96.692438] [T00600001343] other info that might help us debug this:
[   96.692438] [T00600001343] 
[   96.694077] [T00600001343] mtkPowerMsgHdl: Chain exists of:
[   96.694077] [T00600001343]   &devfreq->lock --> subsys mutex#2 -->
&(c->notifiers)->rwsem#2
[   96.694077] [T00600001343] 
[   96.696155] [T00600001343] mtkPowerMsgHdl:  Possible unsafe locking
scenario:
[   96.696155] [T00600001343] 
[   96.697383] [T00600001343] mtkPowerMsgHdl:        CPU0             
CPU1
[   96.698284] [T00600001343] mtkPowerMsgHdl:        ----             
----
[   96.699184] [T00600001343] mtkPowerMsgHdl:   rlock(&(c->notifiers)-
>rwsem#2);
[   96.700080] [T00600001343] mtkPowerMsgHdl:                         
lock(subsys mutex#2);
[   96.701172] [T00600001343] mtkPowerMsgHdl:                         
lock(&(c->notifiers)->rwsem#2);
[   96.702371] [T00600001343] mtkPowerMsgHdl:   lock(&devfreq->lock);
[   96.703145] [T00600001343] mtkPowerMsgHdl: 
[   96.703145] [T00600001343]  *** DEADLOCK ***
[   96.703145] [T00600001343] 
[   96.704524] [T00600001343] mtkPowerMsgHdl: 6 locks held by
mtkPowerMsgHdl/1343:
[   96.705440] [T00600001343] mtkPowerMsgHdl:  #0: ffffff8089ee64f8
(&f->f_pos_lock){+.+.}-{3:3}, at: fdget_pos+0x1a4/0x228
[   96.706816] [T00600001343] mtkPowerMsgHdl:  #1: ffffff803b5ba450
(sb_writers#4){.+.+}-{0:0}, at: vfs_write+0x1f4/0x954
[   96.708166] [T00600001343] mtkPowerMsgHdl:  #2: ffffff80f817fc88
(&of->mutex#2){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x170/0x408
[   96.709652] [T00600001343] mtkPowerMsgHdl:  #3: ffffff804558b840
(kn->active#203){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x18c/0x408
[   96.711165] [T00600001343] mtkPowerMsgHdl:  #4: ffffffc0849ad6a8
(dev_pm_qos_mtx){+.+.}-{3:3}, at: dev_pm_qos_update_request+0x30/0x60
[   96.712691] [T00600001343] mtkPowerMsgHdl:  #5: ffffff804584b9c8
(&(c->notifiers)->rwsem#2){++++}-{3:3}, at:
blocking_notifier_call_chain+0x50/0x94
[   96.714361] [T00600001343] mtkPowerMsgHdl: 
[   96.714361] [T00600001343] stack backtrace:
[   96.715401] [T00600001343] mtkPowerMsgHdl: CPU: 6 UID: 0 PID: 1343
Comm: mtkPowerMsgHdl Tainted: G        W  OE      6.12.30-android16-5-
g137b86aeb5da-4k #1 bfd83f66acc4211b86b2132db3b8eb1c0982822d
[   96.715416] [T00600001343] mtkPowerMsgHdl: Tainted: [W]=WARN,
[O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[   96.715419] [T00600001343] mtkPowerMsgHdl: Hardware name:
MT6993(ENG) (DT)
[   96.715425] [T00600001343] mtkPowerMsgHdl: Call trace:
[   96.715428] [T00600001343] mtkPowerMsgHdl: 
dump_backtrace+0xfc/0x178
[   96.715437] [T00600001343] mtkPowerMsgHdl:  show_stack+0x18/0x24
[   96.715444] [T00600001343] mtkPowerMsgHdl:  dump_stack_lvl+0x40/0x9c
[   96.715450] [T00600001343] mtkPowerMsgHdl:  dump_stack+0x18/0x24
[   96.715456] [T00600001343] mtkPowerMsgHdl: 
print_circular_bug+0x14c/0x1b8
[   96.715462] [T00600001343] mtkPowerMsgHdl: 
check_noncircular+0x24c/0x328
[   96.715467] [T00600001343] mtkPowerMsgHdl: 
__lock_acquire+0x2e10/0x76d0
[   96.715471] [T00600001343] mtkPowerMsgHdl:  lock_acquire+0x1d0/0x550
[   96.715474] [T00600001343] mtkPowerMsgHdl: 
__mutex_lock_common+0x1dc/0x371c
[   96.715481] [T00600001343] mtkPowerMsgHdl: 
mutex_lock_nested+0x2c/0x38
[   96.715487] [T00600001343] mtkPowerMsgHdl: 
qos_max_notifier_call+0x2c/0x98
[   96.715494] [T00600001343] mtkPowerMsgHdl: 
notifier_call_chain+0x128/0x388
[   96.715500] [T00600001343] mtkPowerMsgHdl: 
blocking_notifier_call_chain+0x68/0x94
[   96.715506] [T00600001343] mtkPowerMsgHdl: 
pm_qos_update_target+0x3c0/0x5d4
[   96.715512] [T00600001343] mtkPowerMsgHdl:  freq_qos_apply+0xa8/0xd8
[   96.715517] [T00600001343] mtkPowerMsgHdl: 
apply_constraint+0xa0/0x1a4
[   96.715522] [T00600001343] mtkPowerMsgHdl: 
__dev_pm_qos_update_request+0x1b4/0x340
[   96.715526] [T00600001343] mtkPowerMsgHdl: 
dev_pm_qos_update_request+0x3c/0x60
[   96.715530] [T00600001343] mtkPowerMsgHdl: 
max_freq_store+0xf4/0x130
[   96.715535] [T00600001343] mtkPowerMsgHdl:  dev_attr_store+0x64/0x84
[   96.715538] [T00600001343] mtkPowerMsgHdl: 
sysfs_kf_write+0x19c/0x224
[   96.715545] [T00600001343] mtkPowerMsgHdl: 
kernfs_fop_write_iter+0x254/0x408
[   96.715550] [T00600001343] mtkPowerMsgHdl:  vfs_write+0x674/0x954
[   96.715554] [T00600001343] mtkPowerMsgHdl:  ksys_write+0xdc/0x174
[   96.715557] [T00600001343] mtkPowerMsgHdl: 
__arm64_sys_write+0x78/0x8c
[   96.715561] [T00600001343] mtkPowerMsgHdl: 
invoke_syscall+0x78/0x250
[   96.715566] [T00600001343] mtkPowerMsgHdl: 
el0_svc_common+0x154/0x1c4
[   96.715571] [T00600001343] mtkPowerMsgHdl:  do_el0_svc+0x48/0x58
[   96.715577] [T00600001343] mtkPowerMsgHdl:  el0_svc+0x50/0xd0
[   96.715581] [T00600001343] mtkPowerMsgHdl: 
el0t_64_sync_handler+0x70/0xbc
[   96.715585] [T00600001343] mtkPowerMsgHdl:  el0t_64_sync+0x188/0x18c


Thansk.
Peter

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Bart Van Assche]

On 7/25/25 2:13 AM, Peter Wang (王信友) wrote:
> Could consider luns_avail instead mutex?

That would be wrong. I think it is essential that scan_mutex is used in
this patch. Additionally, the lock inversion is between devfreq->lock
and (c->notifiers)->rwsem so it seems unlikely to me that Ziqi's patch
is the patch that introduced the reported lock inversion.

Bart.

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Fri, 2025-07-25 at 07:54 -0700, Bart Van Assche wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 7/25/25 2:13 AM, Peter Wang (王信友) wrote:
> > Could consider luns_avail instead mutex?
> 
> That would be wrong. I think it is essential that scan_mutex is used
> in
> this patch. Additionally, the lock inversion is between devfreq->lock
> and (c->notifiers)->rwsem so it seems unlikely to me that Ziqi's
> patch
> is the patch that introduced the reported lock inversion.
> 
> Bart.


Hi Bart,

This is a complex situation involving six locks, which may result in
a circular locking dependency.
Let me explain how a new circular locking dependency is formed:

CPU0: take &(c->notifiers)->rwsem#2, wait &devfreq->lock
CPU1: take &devfreq->lock, wait &shost->scan_mutex,  <= Lock introduced
by this patch
CPU2: take &shost->scan_mutex, wait &q->sysfs_lock
CPU3: take &q->sysfs_lock, wait cpu_hotplug_lock
CPU4: take cpu_hotplug_lock, wait subsys mutex#2
CPU5: take subsys mutex#2, wait &(c->notifiers)->rwsem#2  <= Hold By
CPU0

ufshcd_add_lus triggers ufshcd_devfreq_init.
This means that clock scaling can be performed while scanning LUNs.
However, this patch adds another lock to prevent clock scaling 
before the LUN scan is complete. This is a paradoxical situation. 
If we cannnot do clock scaling before the LUN scan is complete, 
then why we start clock scaling before it?

If we don’t put it in luns_avail (start clock scaling after LUNs 
scan complete), do you have a better suggestion
for where to initialize clock scaling (ufshcd_devfreq_init)?

Thanks.
Peter

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Ziqi Chen]

On 7/28/2025 2:34 PM, Peter Wang (王信友) wrote:
> On Fri, 2025-07-25 at 07:54 -0700, Bart Van Assche wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 7/25/25 2:13 AM, Peter Wang (王信友) wrote:
>>> Could consider luns_avail instead mutex?
>>
>> That would be wrong. I think it is essential that scan_mutex is used
>> in
>> this patch. Additionally, the lock inversion is between devfreq->lock
>> and (c->notifiers)->rwsem so it seems unlikely to me that Ziqi's
>> patch
>> is the patch that introduced the reported lock inversion.
>>
>> Bart.
> 
> 
> Hi Bart,
> 
> This is a complex situation involving six locks, which may result in
> a circular locking dependency.
> Let me explain how a new circular locking dependency is formed:
> 
> CPU0: take &(c->notifiers)->rwsem#2, wait &devfreq->lock
> CPU1: take &devfreq->lock, wait &shost->scan_mutex,  <= Lock introduced
> by this patch
> CPU2: take &shost->scan_mutex, wait &q->sysfs_lock
> CPU3: take &q->sysfs_lock, wait cpu_hotplug_lock


Hi Peter,

I Don't think the dependence between CPU2 and CPU3 would happen.

CPU2:
__mutex_lock_common+0x1dc/0x371c  -> (Waiting &q->sysfs_lock)
mutex_lock_nested+0x2c/0x38
blk_mq_realloc_hw_ctxs+0x94/0x9cc
blk_mq_init_allocated_queue+0x31c/0x1020
blk_mq_alloc_queue+0x130/0x214
scsi_alloc_sdev+0x708/0xad4
scsi_probe_and_add_lun+0x20c/0x27b4

CPU3:
pus_read_lock+0x54/0x1e8 -> ( Waiting cpu_hotplug_lock)
__cpuhp_state_add_instance+0x24/0x54
blk_mq_alloc_and_init_hctx+0x940/0xbec
blk_mq_realloc_hw_ctxs+0x290/0x9cc  -> (holding &q->sysfs_lock)
blk_mq_init_allocated_queue+0x31c/0x1020
__blk_mq_alloc_disk+0x138/0x2b0
loop_add+0x2ac/0x840
loop_init+0xe8/0x10c

As my understanding, on single sdev , alloc_disk() and alloc_queue()
is synchronous. On multi sdev , they hold different &q->sysfs_lock
as they would be allocated different request_queue.

In addition to above , if you check the latest version, the function
blk_mq_realloc_hw_ctxs has been changed many times recently. It doesn't
hold &q->sysfs_lock any longer.

https://lore.kernel.org/all/20250304102551.2533767-5-nilay@linux.ibm.com/

-> use &q->elevator_lock instead of  &q->sysfs_lock.

https://lore.kernel.org/all/20250403105402.1334206-1-ming.lei@redhat.com/

-> Don't use &q->elevator_lock in blk_mq_init_allocated_queue context.


> CPU4: take cpu_hotplug_lock, wait subsys mutex#2  > CPU5: take subsys mutex#2, wait &(c->notifiers)->rwsem#2  <= Hold By
> CPU0
> 
> ufshcd_add_lus triggers ufshcd_devfreq_init.
> This means that clock scaling can be performed while scanning LUNs.
> However, this patch adds another lock to prevent clock scaling
> before the LUN scan is complete. This is a paradoxical situation.
> If we cannnot do clock scaling before the LUN scan is complete,
> then why we start clock scaling before it?
> 
> If we don’t put it in luns_avail (start clock scaling after LUNs
> scan complete), do you have a better suggestion
> for where to initialize clock scaling (ufshcd_devfreq_init)?

I have also considered this. you can see my old version of this patch
(patch V2), I moved ufshcd_devfreq_init() out of ufshcd_add_lus().
But due to ufshcd_add_lus() is async, even through move it out , we 
still can not ensure clock scaling be triggered after all lUs probed.

BRs
Ziqi
  >
> Thanks.
> Peter
> 

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Tue, 2025-07-29 at 11:02 +0800, Ziqi Chen wrote:
> 
> 
> 
> Hi Peter,
> 
> I Don't think the dependence between CPU2 and CPU3 would happen.
> 
> CPU2:
> __mutex_lock_common+0x1dc/0x371c  -> (Waiting &q->sysfs_lock)
> mutex_lock_nested+0x2c/0x38
> blk_mq_realloc_hw_ctxs+0x94/0x9cc
> blk_mq_init_allocated_queue+0x31c/0x1020
> blk_mq_alloc_queue+0x130/0x214
> scsi_alloc_sdev+0x708/0xad4
> scsi_probe_and_add_lun+0x20c/0x27b4
> 
> CPU3:
> pus_read_lock+0x54/0x1e8 -> ( Waiting cpu_hotplug_lock)
> __cpuhp_state_add_instance+0x24/0x54
> blk_mq_alloc_and_init_hctx+0x940/0xbec
> blk_mq_realloc_hw_ctxs+0x290/0x9cc  -> (holding &q->sysfs_lock)
> blk_mq_init_allocated_queue+0x31c/0x1020
> __blk_mq_alloc_disk+0x138/0x2b0
> loop_add+0x2ac/0x840
> loop_init+0xe8/0x10c
> 

Hi Ziqi,

This is a warning, and it may not necessarily occur.
However, once this warning is detected, lockdep will stop,
which makes subsequent debugging more difficult.


> As my understanding, on single sdev , alloc_disk() and alloc_queue()
> is synchronous. On multi sdev , they hold different &q->sysfs_lock
> as they would be allocated different request_queue.
> 
> In addition to above , if you check the latest version, the function
> blk_mq_realloc_hw_ctxs has been changed many times recently. It
> doesn't
> hold &q->sysfs_lock any longer.
> 
> https://lore.kernel.org/all/20250304102551.2533767-5-nilay@linux.ibm.com/
> 
> -> use &q->elevator_lock instead of  &q->sysfs_lock.
> 
> https://lore.kernel.org/all/20250403105402.1334206-1-ming.lei@redhat.com/
> 
> -> Don't use &q->elevator_lock in blk_mq_init_allocated_queue
> context.
> 

We will further check these two patches to see if this issue
can be avoided. However, introducing new locks and increasing
systemic issues is still something we should try to avoid as 
much as possible.


> 
> 
> I have also considered this. you can see my old version of this patch
> (patch V2), I moved ufshcd_devfreq_init() out of ufshcd_add_lus().
> But due to ufshcd_add_lus() is async, even through move it out , we
> still can not ensure clock scaling be triggered after all lUs probed.
> 
> BRs
> Ziqi
> > 

I remember that I suggested in v3 that, although checking
hba->luns_avail in ufshcd_device_configure is a bit strange,
it was already strange that ufshcd_add_lus was calling 
ufshcd_devfreq_init in the first place.
However, in theory, this issue should still be solvable
without using a lock.
Another idea is to only start ufshcd_devfreq_init 
when shost->async_scan = 0.

Thanks.
Peter

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Bart Van Assche]

On 7/30/25 5:55 AM, Peter Wang (王信友) wrote:
> However, in theory, this issue should still be solvable
> without using a lock.
> Another idea is to only start ufshcd_devfreq_init
> when shost->async_scan = 0.

Does the lockdep complaint mentioned in this email thread occur on
multiple platforms or only on MediaTek platforms? I don't see any
lockdep complaints with Martin's SCSI for-next branch on my
development platform. If this warning only occurs on MediaTek
platforms, why to modify the UFSHCI core driver to eliminate this
lockdep complaint?

Thanks,

Bart.

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Bart Van Assche]

On 7/30/25 8:37 AM, Bart Van Assche wrote:
> On 7/30/25 5:55 AM, Peter Wang (王信友) wrote:
>> However, in theory, this issue should still be solvable
>> without using a lock.
>> Another idea is to only start ufshcd_devfreq_init
>> when shost->async_scan = 0.
> 
> Does the lockdep complaint mentioned in this email thread occur on
> multiple platforms or only on MediaTek platforms? I don't see any
> lockdep complaints with Martin's SCSI for-next branch on my
> development platform. If this warning only occurs on MediaTek
> platforms, why to modify the UFSHCI core driver to eliminate this
> lockdep complaint?

Answering my own question: clock scaling is not enabled on my
development platform and that's why I'm not seeing this lockdep
complaint.

Bart.

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Bart Van Assche]

On 7/30/25 5:55 AM, Peter Wang (王信友) wrote:
> Another idea is to only start ufshcd_devfreq_init
> when shost->async_scan = 0.

Hmm ... I don't think that this is a solution. There are multiple
ways for triggering a LUN scan and my understanding is that the
clock scaling code should be serialized against LUN scanning.

Here is an example of how LUN scanning can be triggered from user
space by writing into the 'scan' sysfs attribute, even if
shost->async_scan = 0:

echo "- - -" > /sys/class/scsi_host/host.../scan

Bart.

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Ziqi Chen]

On 7/30/2025 11:50 PM, Bart Van Assche wrote:
> On 7/30/25 5:55 AM, Peter Wang (王信友) wrote:
>> Another idea is to only start ufshcd_devfreq_init
>> when shost->async_scan = 0.
> 
> Hmm ... I don't think that this is a solution. There are multiple
> ways for triggering a LUN scan and my understanding is that the
> clock scaling code should be serialized against LUN scanning.
> 
> Here is an example of how LUN scanning can be triggered from user
> space by writing into the 'scan' sysfs attribute, even if
> shost->async_scan = 0:
> 
> echo "- - -" > /sys/class/scsi_host/host.../scan
> 
> Bart.

Hi  Peter && Bart,

How do you think about using

if (!mutex_trylock(&hba->host->scan_mutex))
	return -EAGAIN;

instead of

mutex_lock(&hba->host->scan_mutex);

But this way will cause one line print of devfreq failed.

BRs
Ziqi

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Mon, 2025-08-04 at 15:54 +0800, Ziqi Chen wrote:
> 
> 
> Hi  Peter && Bart,
> 
> How do you think about using
> 
> if (!mutex_trylock(&hba->host->scan_mutex))
>         return -EAGAIN;
> 
> instead of
> 
> mutex_lock(&hba->host->scan_mutex);
> 
> But this way will cause one line print of devfreq failed.
> 
> BRs
> Ziqi
> 


Hi Ziqi,

I think this might have a chance to resolve this circular lockdep
issue.
Mediatek will test it internally and then provide feedback on the
results.

Thanks.
Peter

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Mon, 2025-08-04 at 15:54 +0800, Ziqi Chen wrote:
> 
> Hi  Peter && Bart,
> 
> How do you think about using
> 
> if (!mutex_trylock(&hba->host->scan_mutex))
>         return -EAGAIN;
> 
> instead of
> 
> mutex_lock(&hba->host->scan_mutex);
> 
> But this way will cause one line print of devfreq failed.
> 
> BRs
> Ziqi


Hi Ziqi,

After applying the patch below, the lockdep issue no longer appears.
Would you be able to upstream this fix?

---

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2ff91f2..0af34ce 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -1435,7 +1435,8 @@ static int ufshcd_clock_scaling_prepare(struct
ufs_hba *hba, u64 timeout_us)
 	 * make sure that there are no outstanding requests when
 	 * clock scaling is in progress
 	 */
-	mutex_lock(&hba->host->scan_mutex);
+	if(!mutex_trylock(&hba->host->scan_mutex))
+		return -EAGAIN;
 	blk_mq_quiesce_tagset(&hba->host->tag_set);
 	mutex_lock(&hba->wb_mutex);
 	down_write(&hba->clk_scaling_lock);


Thanks.
Peter

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Ziqi Chen]

On 8/4/2025 8:43 PM, Peter Wang (王信友) wrote:
> On Mon, 2025-08-04 at 15:54 +0800, Ziqi Chen wrote:
>>
>> Hi  Peter && Bart,
>>
>> How do you think about using
>>
>> if (!mutex_trylock(&hba->host->scan_mutex))
>>          return -EAGAIN;
>>
>> instead of
>>
>> mutex_lock(&hba->host->scan_mutex);
>>
>> But this way will cause one line print of devfreq failed.
>>
>> BRs
>> Ziqi
> 
> 
> Hi Ziqi,
> 
> After applying the patch below, the lockdep issue no longer appears.
> Would you be able to upstream this fix?
> 
> ---
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2ff91f2..0af34ce 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -1435,7 +1435,8 @@ static int ufshcd_clock_scaling_prepare(struct
> ufs_hba *hba, u64 timeout_us)
>   	 * make sure that there are no outstanding requests when
>   	 * clock scaling is in progress
>   	 */
> -	mutex_lock(&hba->host->scan_mutex);
> +	if(!mutex_trylock(&hba->host->scan_mutex))
> +		return -EAGAIN;
>   	blk_mq_quiesce_tagset(&hba->host->tag_set);
>   	mutex_lock(&hba->wb_mutex);
>   	down_write(&hba->clk_scaling_lock);
> 
> 
> Thanks.
> Peter

Hi Peter,

I saw you raised a ACK change to revert this whole change on branch
Android16-6.12. Without this change, you will meet stuck issue during
stability reboot test due to request queue quiesce and unquiesce
mismatch.

This lockdep print just a warning, and as per my analysis before, This
is a misjudgment. But stuck/crash issue is a real issue which has real 
instance.

Can you abort your reverting change?


BRs,
Ziqi

Re: [PATCH v4] scsi: ufs: core: Don't perform UFS clkscale if host asyn scan in progress

[Peter Wang (王信友)]

On Tue, 2025-08-05 at 14:03 +0800, Ziqi Chen wrote:
> 
> Hi Peter,
> 
> I saw you raised a ACK change to revert this whole change on branch
> Android16-6.12. Without this change, you will meet stuck issue during
> stability reboot test due to request queue quiesce and unquiesce
> mismatch.
> 
> This lockdep print just a warning, and as per my analysis before,
> This
> is a misjudgment. But stuck/crash issue is a real issue which has
> real
> instance.
> 
> Can you abort your reverting change?
> 
> 
> BRs,
> Ziqi

Hi Ziqi,

The lockdep warning remains a problem that impacts the 
functionality of lockdep. Furthermore, the lockdep issue
is consistently reproducible, unlike the stuck/crash issue,
which is rare and only occurred after the clock scale code 
had been in use for several years.

Therefore, I recommend ACK reverting first and then merging 
the fixed patch together.

Thanks.
Peter