Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm: self-deadlock on perfcntr_lock in sel_worker() via msm_gpu_hw_init() --
commit 5d6d3def9b6235a74e135666a5a4eea2adca8e09 Author: Rob Clark <[email protected]> drm/msm: Resume perfcntrs after hw_init This commit moves msm_perfcntr_resume() from adreno_runtime_resume() and adreno_recover() into msm_gpu_hw_init() so that performance counter sampling resumes only after hardware initialization completes. > diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > index 0c2c356362518..b9a8c641001d0 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.c > +++ b/drivers/gpu/drm/msm/msm_gpu.c > @@ -168,8 +168,10 @@ int msm_gpu_hw_init(struct msm_gpu *gpu) > > disable_irq(gpu->irq); > ret = gpu->funcs->hw_init(gpu); > - if (!ret) > + if (!ret) { > + msm_perfcntr_resume(gpu); > gpu->needs_hw_init = false; > + } [Severity: High] Can calling msm_perfcntr_resume() inside msm_gpu_hw_init() deadlock on gpu->perfcntr_lock when invoked from sel_worker()? In drivers/gpu/drm/msm/msm_perfcntr.c, sel_worker() acquires both gpu->lock and gpu->perfcntr_lock before calling msm_gpu_hw_init(): drivers/gpu/drm/msm/msm_perfcntr.c:sel_worker() { ... scoped_guard (mutex, &gpu->lock) { guard(mutex)(&gpu->perfcntr_lock); if (stream == gpu->perfcntrs->stream) { msm_gpu_hw_init(gpu); gpu->funcs->perfcntr_configure(gpu, ring, stream); } } } And msm_perfcntr_resume() acquires gpu->perfcntr_lock again: drivers/gpu/drm/msm/msm_perfcntr.c:msm_perfcntr_resume() { if (!gpu->perfcntrs) return 0; guard(mutex)(&gpu->perfcntr_lock); return msm_perfcntr_resume_locked(gpu->perfcntrs->stream); } If the GPU is runtime-PM active while gpu->needs_hw_init is still true (for example, after adreno_runtime_resume() runs via pm_runtime_get_sync() in adreno_get_param() or msm_context_set_sysprof() before any job submission calls msm_gpu_hw_init()), opening a stream in msm_ioctl_perfcntr_config() unconditionally calls msm_perfcntr_resume_locked(), which starts stream->sample_timer and queues stream->sel_work. When sel_worker() runs and calls msm_gpu_hw_init(gpu) with gpu->needs_hw_init set, msm_gpu_hw_init() disables gpu->irq, runs gpu->funcs->hw_init(gpu), and calls msm_perfcntr_resume(gpu), which attempts to take gpu->perfcntr_lock while sel_worker() already holds it. Even without the recursive lock acquisition, wouldn't calling msm_perfcntr_resume() from the sel_worker() -> msm_gpu_hw_init() path also redundantly re-queue stream->sel_work right before sel_worker() calls gpu->funcs->perfcntr_configure()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
