Re: [PATCH v3 50/57] scsi: ufs: Enable lock context analysis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] ufshcd_wait_for_pending_cmds() uses io_schedule_timeout() without setting current->state, causing a busy loop instead of sleeping.
--

--- Patch [50]: [PATCH v3 50/57] scsi: ufs: Enable lock context analysis ---
commit 8185dd4bd8104528b9c4d31b079a63ef3f632638
Author: Bart Van Assche <bvanassche@acm.org>

scsi: ufs: Enable lock context analysis

This commit annotates functions that modify the state of a synchronization
object to enable lock context analysis. It also removes struct semaphore
annotations because lock context annotations are not supported for
semaphores.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index f85582287d14d..22a5f8e9af913 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -1353,6 +1353,8 @@ static int ufshcd_wait_for_pending_cmds(struct ufs_hba *hba,

[Severity: High]
This is a pre-existing issue, but does ufshcd_wait_for_pending_cmds()
busy loop here instead of sleeping?

Looking at ufshcd_wait_for_pending_cmds(), it calls io_schedule_timeout()
without first setting the task state to TASK_UNINTERRUPTIBLE:

drivers/ufs/core/ufshcd.c:ufshcd_wait_for_pending_cmds() {
	...
	do {
		tm_doorbell = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
		tr_pending = ufshcd_pending_cmds(hba);
		if (!tm_doorbell && !tr_pending) {
			timeout = false;
			break;
		} else if (do_last_check) {
			break;
		}

		io_schedule_timeout(msecs_to_jiffies(20));
		if (ktime_to_us(ktime_sub(ktime_get(), start)) >
		    wait_timeout_us) {
	...
}

Because the task state remains TASK_RUNNING, does the underlying
schedule_timeout() function return immediately without yielding the CPU? 

Could this cause severe performance degradation by hogging 100% of a CPU
core and aggressively polling MMIO registers for up to 1 second during
operations like UFS hardware clock scaling?

>   * On failure, all acquired locks are released and the tagset is unquiesced.
>   */
>  int ufshcd_pause_command_processing(struct ufs_hba *hba, u64 timeout_us)
> +	__cond_acquires(0, &hba->host->scan_mutex)
> +	__cond_acquires(0, &hba->clk_scaling_lock)
>  {
>  	int ret = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785451174.git.bvanassche@acm.org?part=50