Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm/a5xx: premature pm_qos_req removal in a5xx_destroy() before cleanup --
commit fc8618fd147137559c55740f177ada72c5eca286 Author: Fabricio Akio <[email protected]> drm/msm/a5xx: Hold a CPU latency request while the GPU is powered This patch holds a CPU latency QoS request while A508, A509, and A512 GPUs are powered on so the SoC does not enter system-level power collapse while GPU work is active. The request is added on GPU resume and removed on GPU suspend or destruction. > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index 0aff8252a00f9..1d4993f8cb7c4 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -1060,6 +1060,10 @@ static void a5xx_destroy(struct msm_gpu *gpu) > drm_gem_object_put(a5xx_gpu->shadow_bo); > } > > + /* Still held if suspending the GPU failed before it was destroyed */ > + if (cpu_latency_qos_request_active(&a5xx_gpu->pm_qos_req)) > + cpu_latency_qos_remove_request(&a5xx_gpu->pm_qos_req); > + > adreno_gpu_cleanup(adreno_gpu); > kfree(a5xx_gpu); > } [Severity: High] Should the removal of a5xx_gpu->pm_qos_req in a5xx_destroy() happen after adreno_gpu_cleanup(adreno_gpu) rather than before it? In adreno_unbind(), if adreno_system_suspend() fails (for example, timing out with -EBUSY because a GPU submission is hung or still active, or if pm_runtime_force_suspend() fails), adreno_system_suspend() restarts the scheduler and returns with runtime PM still enabled: drivers/gpu/drm/msm/adreno/adreno_device.c:adreno_system_suspend() { remaining = wait_event_timeout(gpu->retire_event, gpu->active_submits == 0, msecs_to_jiffies(1000)); if (remaining == 0) { dev_err(dev, "Timeout waiting for GPU to suspend\n"); ret = -EBUSY; goto out; } ret = pm_runtime_force_suspend(dev); out: if (ret) resume_scheduler(gpu); } adreno_unbind() then proceeds directly into gpu->funcs->destroy(gpu) -> a5xx_destroy() while runtime PM and gpu->worker are still running. Runtime PM is only disabled and gpu->worker is only stopped inside adreno_gpu_cleanup() -> msm_gpu_cleanup(): drivers/gpu/drm/msm/adreno/adreno_gpu.c:adreno_gpu_cleanup() { ... pm_runtime_dont_use_autosuspend(&gpu->pdev->dev); if (priv && pm_runtime_enabled(&priv->gpu_pdev->dev)) pm_runtime_disable(&priv->gpu_pdev->dev); msm_gpu_cleanup(&adreno_gpu->base); } Meanwhile, recover_worker on gpu->worker runs a5xx_recover() -> adreno_recover(), which directly calls gpu->funcs->pm_suspend(gpu) and gpu->funcs->pm_resume(gpu). If cpu_latency_qos_remove_request(&a5xx_gpu->pm_qos_req) runs before adreno_gpu_cleanup(adreno_gpu), can recover_worker or runtime PM execute a5xx_pm_suspend() or a5xx_pm_resume() concurrently with or after the removal, re-adding &a5xx_gpu->pm_qos_req to cpu_latency_constraints right before kfree(a5xx_gpu) frees it? [ ... ] > > base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64 Boris 在进行深入审查时，我发现了一个潜在的释放后使用（Use-After-Free）问题，已在上面的内联评论中详细说明。 -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
