Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/panfrost: cycle_counter ref leak in panfrost_perfcnt_reset() on timeout --
commit 5c53c60920207774a536024e17e8e90206c7ae20 Author: Boris Brezillon <[email protected]> drm/panfrost: Enable cycle counting when perfcnt is enabled This commit enables cycle counting when performance counters are active so that block headers contain valid timestamps, and removes the warning in panfrost_gpu_soft_reset() when resetting pfdev->cycle_counter.use_count. > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > index a9def452d5ef9..b26da057a1a01 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c [ ... ] > @@ -84,6 +86,7 @@ static int panfrost_perfcnt_hw_enable(struct > panfrost_device *pfdev) > return ret; > > perfcnt->owns_as_ref = true; > + panfrost_cycle_counter_get(pfdev); [Severity: Medium] As reported by Sashiko on v2 [1], can this leak a reference on pfdev->cycle_counter.use_count if panfrost_gpu_soft_reset() times out during panfrost_device_reset()? In panfrost_gpu_soft_reset(), if both the soft reset and hard reset polling loops time out, the function returns early before zeroing pfdev->cycle_counter.use_count: drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_soft_reset() { ... if (ret) { ... if (ret) { dev_err(pfdev->base.dev, "gpu hard reset timed out\n"); return ret; } } ... atomic_set(&pfdev->cycle_counter.use_count, 0); } However, panfrost_device_reset() ignores the return value of panfrost_gpu_soft_reset() and continues with panfrost_mmu_reset() and panfrost_perfcnt_reset(): drivers/gpu/drm/panfrost/panfrost_device.c:panfrost_device_reset() { ... panfrost_gpu_soft_reset(pfdev); panfrost_gpu_power_on(pfdev); panfrost_mmu_reset(pfdev); ... panfrost_perfcnt_reset(pfdev); } panfrost_perfcnt_reset() then clears perfcnt->owns_as_ref without releasing the existing cycle counter reference and calls panfrost_perfcnt_hw_enable(), which succeeds in panfrost_mmu_as_get() and calls panfrost_cycle_counter_get() a second time: drivers/gpu/drm/panfrost/panfrost_perfcnt.c:panfrost_perfcnt_reset() { ... /* All active AS are released during the MMU post_reset. */ perfcnt->owns_as_ref = false; perfcnt->state |= PANFROST_PERFCNT_SESSION_INTERRUPTED_BY_RESET; if (panfrost_perfcnt_hw_enable(pfdev)) perfcnt->state |= PANFROST_PERFCNT_SESSION_DEAD; } When the session is later disabled via panfrost_perfcnt_hw_disable(), panfrost_cycle_counter_put() only decrements pfdev->cycle_counter.use_count once, leaving a leaked reference count that prevents the cycle counter from stopping. [1] https://lore.kernel.org/r/20261103-panfrost-reset-races-fixes-v2-1-454207b1d...@collabora.com -- Sashiko AI review · https://sashiko.dev/#/patchset/20261104-panfrost-reset-races-fixes-v3-0-09868a88c...@collabora.com?part=1
