Re: [PATCH] Revert "scsi: ufs: core: Remove unnecessary block I/O quiesce for clock scaling"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling blk_mq_quiesce_tagset() before acquiring the clk_scaling_lock write semaphore introduces an ABBA deadlock with device management commands.
--

--- Patch [1]: [PATCH] Revert "scsi: ufs: core: Remove unnecessary block I/O quiesce for clock scaling" ---
commit d454cd4e623d4ff192ea3b557d6b1e8c27de2a9a
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Revert "scsi: ufs: core: Remove unnecessary block I/O quiesce for clock scaling"

This commit reverts a previous change to restore the block I/O quiesce
during clock scaling preparation. It is intended to fix UFS failures
during clock scaling on Qcom platforms.

Closes: https://lore.kernel.org/linux-scsi/al8YkXydXJQZrl_Y@QCOM-aGQu4IUr3Y

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 79331c2bd38de..1a5a851d43b28 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c

[ ... ]

> @@ -1445,7 +1445,7 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, u32 target_gear, bool scale_up
>   *
>   * Return: 0 upon success; -EBUSY upon timeout.
>   */
> -static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba)
> +static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us)
>  {
>  	int ret = 0;
>  	/*
> @@ -1453,13 +1453,16 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba)
>  	 * 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);

[Severity: High]
Could this sequencing introduce a deadlock between clock scaling and
device management commands?

If a device management command starts first, it acquires the read lock in
ufshcd_dev_man_lock():

ufshcd_dev_man_lock()
    down_read(&hba->clk_scaling_lock);

If the devfreq scaling worker then calls ufshcd_clock_scaling_prepare(),
it will quiesce the block queue and block waiting for the write lock:

ufshcd_clock_scaling_prepare()
    blk_mq_quiesce_tagset(&hba->host->tag_set);
    down_write(&hba->clk_scaling_lock);

When the device management thread proceeds to execute its request via
ufshcd_issue_dev_cmd() calling blk_execute_rq(), the request gets queued but
will not dispatch because the queue is quiesced. 

Since blk_execute_rq() blocks indefinitely and the block layer timeout
doesn't track requests that haven't dispatched yet, won't this result in
both threads hanging permanently?

> -	if (!hba->clk_scaling.is_allowed) {
> +	if (!hba->clk_scaling.is_allowed ||
> +	    ufshcd_wait_for_pending_cmds(hba, timeout_us)) {
>  		ret = -EBUSY;
>  		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;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723124733.341789-1-manivannan.sadhasivam@oss.qualcomm.com?part=1