Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/msm: recursive deadlock on perfcntr_lock in sel_worker() - [High] drm/msm: deadlock on fault_coredump_done in msm_gpu_open() --
commit 7aeb57e073a9d565144e21c9afab92c78bbd86fd Author: Rob Clark <[email protected]> drm/msm: Inhibit IFPC for state snapshot This commit inhibits IFPC around gpu_state_get() in msm_gpu_open() so that the GPU remains awake while taking a debugfs state snapshot. > diff --git a/drivers/gpu/drm/msm/msm_debugfs.c > b/drivers/gpu/drm/msm/msm_debugfs.c > index 9373ac19ee953..394d37ca90711 100644 > --- a/drivers/gpu/drm/msm/msm_debugfs.c > +++ b/drivers/gpu/drm/msm/msm_debugfs.c > @@ -83,6 +83,7 @@ static int msm_gpu_open(struct inode *inode, struct file > *file) > scoped_guard (pm_runtime_active, &gpu->pdev->dev) { > scoped_cond_guard (mutex_intr, return -EINTR, &gpu->lock) { > msm_gpu_hw_init(gpu); [Severity: High] With commit 5d6d3def9b62 ("drm/msm: Resume perfcntrs after hw_init") moving msm_perfcntr_resume(gpu) into msm_gpu_hw_init(gpu), can sel_worker() in drivers/gpu/drm/msm/msm_perfcntr.c hit a recursive deadlock on gpu->perfcntr_lock? In drivers/gpu/drm/msm/msm_perfcntr.c, sel_worker() acquires gpu->perfcntr_lock before calling msm_gpu_hw_init(gpu): 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); } } } When gpu->needs_hw_init is true (for example, while the GPU is runtime-active in the autosuspend window after adreno_load_gpu(), MSM_PARAM_TIMESTAMP, or msm_postclose(), or when racing with a thread between pm_runtime_get_sync() and acquiring gpu->lock in msm_gpu_open() or msm_context_set_sysprof()), msm_gpu_hw_init() calls msm_perfcntr_resume(gpu): drivers/gpu/drm/msm/msm_gpu.c:msm_gpu_hw_init() { ... if (!ret) { msm_perfcntr_resume(gpu); gpu->needs_hw_init = false; } ... } And msm_perfcntr_resume() unconditionally tries to acquire 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); } Won't this self-deadlock on gpu->perfcntr_lock while holding gpu->lock with gpu->irq disabled? > + guard(ifpc_disable)(gpu); [Severity: High] Could acquiring ifpc_disable while holding gpu->lock in msm_gpu_open() (as well as in msm_context_set_sysprof() from commit "drm/msm: Rework IFPC disable" earlier in this series) cause an ABBA deadlock with adreno_gpu->fault_coredump_done? If the GPU is actively executing a submit (so gpu->needs_hw_init is false), msm_gpu_open() acquires gpu->lock and invokes: guard(ifpc_disable)(gpu) -> msm_gpu_disable_ifpc_get(gpu) -> a6xx_gmu_ifpc_update(gpu) -> a6xx_gmu_set_oob(gmu, GMU_OOB_PERFCOUNTER_SET) If an active GPU submit concurrently triggers an SMMU page fault with stall-on-fault enabled (ARM_SMMU_FSR_SS), arm_smmu_context_fault() calls report_iommu_fault() -> msm_gpu_fault_handler() -> a6xx_fault_handler() -> adreno_fault_handler() before writing ARM_SMMU_CB_RESUME to un-stall the SMMU: drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_fault_handler() { ... reinit_completion(&adreno_gpu->fault_coredump_done); fault_info.ttbr0 = info->ttbr0; fault_info.iova = iova; fault_info.flags = flags; fault_info.type = type; fault_info.block = block; msm_gpu_fault_crashstate_capture(gpu, &fault_info); complete_all(&adreno_gpu->fault_coredump_done); ... } Inside msm_gpu_fault_crashstate_capture(), the fault handler thread blocks on mutex_lock(&gpu->lock) because msm_gpu_open() is holding gpu->lock. Meanwhile, because the SMMU is stalled on the fault, the GMU is wedged and gmu_poll_timeout() in a6xx_gmu_set_oob() times out and waits on fault_coredump_done while holding gpu->lock: drivers/gpu/drm/msm/adreno/a6xx_gmu.c:a6xx_gmu_set_oob() { ... if (completion_done(&a6xx_gpu->base.fault_coredump_done)) break; /* We may timeout because the GMU is temporarily wedged from * pending faults from the GPU and we are taking a devcoredump. * Wait until the MMU is resumed and try again. */ wait_for_completion(&a6xx_gpu->base.fault_coredump_done); ... } Since msm_gpu_open() holds gpu->lock while waiting on fault_coredump_done, and adreno_fault_handler() cannot complete fault_coredump_done or resume the SMMU until it acquires gpu->lock, won't both threads permanently deadlock? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
