Re: [PATCH v5 2/6] drm/msm: Resume perfcntrs after hw_init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm: runtime PM reference leak in sel_worker() on early return
--

commit 370e064e444724f1b13c89e7e60a8c61a20e2ad7
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Resume perfcntrs after hw_init

This commit moves performance counter resumption from runtime resume and GPU
recovery into msm_gpu_hw_init(), and updates sel_worker() in msm_perfcntr.c to
bail out if gpu->needs_hw_init is still set.

> diff --git a/drivers/gpu/drm/msm/msm_perfcntr.c b/drivers/gpu/drm/msm/msm_perfcntr.c
> index 7fa2e858bd082..6a35e3e2ca959 100644
> --- a/drivers/gpu/drm/msm/msm_perfcntr.c
> +++ b/drivers/gpu/drm/msm/msm_perfcntr.c
> @@ -199,10 +199,17 @@ sel_worker(struct work_struct *w)
>  		return;
>  
>  	scoped_guard (mutex, &gpu->lock) {
> +		/*
> +		 * If gpu still needs hw_init, bail.  We'll be rescheduled
> +		 * after hw_init, and there is no point to collect counters
> +		 * beforehand
> +		 */
> +		if (gpu->needs_hw_init)
> +			return;

[Severity: High]
Does this early return leak the runtime PM usage reference on &gpu->pdev->dev?

Just before entering the scoped_guard(mutex, &gpu->lock) block, sel_worker()
acquires a runtime PM reference and drops it only after the block finishes:

drivers/gpu/drm/msm/msm_perfcntr.c:sel_worker() {
	if (!pm_runtime_get_if_active(&gpu->pdev->dev))
		return;

	scoped_guard (mutex, &gpu->lock) {
		if (gpu->needs_hw_init)
			return;
		...
	}

	pm_runtime_put_autosuspend(&gpu->pdev->dev);
}

If the GPU is runtime-active while gpu->needs_hw_init is true (for example,
after adreno_runtime_resume() runs for MSM_PARAM_TIMESTAMP or MSM_PARAM_SYSPROF
before msm_gpu_hw_init() has been called) and userspace configures a counter
stream via msm_ioctl_perfcntr_config(), msm_perfcntr_resume_locked() will
still schedule stream->sel_work.

When sel_worker() runs and sees gpu->needs_hw_init == true, returning from
inside scoped_guard releases gpu->lock via cleanup but exits sel_worker()
without calling pm_runtime_put_autosuspend(&gpu->pdev->dev), permanently
preventing the GPU from runtime-suspending. Should this use break instead of
return to exit the scoped_guard block and reach pm_runtime_put_autosuspend()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104175931.28475-1-robin.clark@oss.qualcomm.com?part=2