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 <[email protected]>

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/[email protected]?part=2

Reply via email to