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 bc0ac72ceaf78a79c6a160afa6594381db03f37c Author: Boris Brezillon <[email protected]> drm/panfrost: Enable cycle counting when perfcnt is enabled This commit enables the GPU cycle counter while performance counters are enabled so block headers receive valid timestamps, and drops the warning on a non-zero cycle counter reference count during GPU soft reset. > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c > b/drivers/gpu/drm/panfrost/panfrost_gpu.c > index 6081b27829e1f..7d66b1b509689 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c > @@ -94,10 +94,10 @@ int panfrost_gpu_soft_reset(struct panfrost_device *pfdev) > GPU_IRQ_CLEAN_CACHES_COMPLETED); > > /* > - * All in-flight jobs should have released their cycle > - * counter references upon reset, but let us make sure > + * Even though all in-flight jobs should have released their cycle > counter > + * references upon reset, perfcnt's might still hang about. > */ > - if (drm_WARN_ON(&pfdev->base, > atomic_read(&pfdev->cycle_counter.use_count) != 0)) > + if (atomic_read(&pfdev->cycle_counter.use_count) != 0) > atomic_set(&pfdev->cycle_counter.use_count, 0); > > return 0; > 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] Can this leak a reference on pfdev->cycle_counter.use_count if the GPU reset times out in panfrost_gpu_soft_reset()? In drivers/gpu/drm/panfrost/panfrost_gpu.c, panfrost_gpu_soft_reset() returns early if both soft and hard resets time out, before resetting pfdev->cycle_counter.use_count to 0: drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_soft_reset() { ... if (ret) { dev_err(pfdev->base.dev, "gpu hard reset timed out\n"); return ret; } } ... if (atomic_read(&pfdev->cycle_counter.use_count) != 0) atomic_set(&pfdev->cycle_counter.use_count, 0); } However, in drivers/gpu/drm/panfrost/panfrost_device.c, panfrost_device_reset() ignores the return value of panfrost_gpu_soft_reset() and still proceeds to call panfrost_mmu_reset() and panfrost_perfcnt_reset(): drivers/gpu/drm/panfrost/panfrost_device.c:panfrost_device_reset() { guard(rwsem_write)(&pfdev->reset.lock); panfrost_gpu_soft_reset(pfdev); panfrost_gpu_power_on(pfdev); panfrost_mmu_reset(pfdev); ... panfrost_perfcnt_reset(pfdev); } When an active perfcnt session is present, panfrost_perfcnt_reset() clears perfcnt->owns_as_ref without dropping the existing cycle_counter reference and calls panfrost_perfcnt_hw_enable(): 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; ... } Because panfrost_mmu_reset() cleared pfdev->as_alloc_mask, panfrost_mmu_as_get() succeeds in panfrost_perfcnt_hw_enable() and panfrost_cycle_counter_get() increments pfdev->cycle_counter.use_count a second time without the prior reference having been cleared or released. When panfrost_perfcnt_hw_disable() is later called, panfrost_cycle_counter_put() only decrements pfdev->cycle_counter.use_count once, leaving it non-zero and preventing GPU_CMD_CYCLE_COUNT_STOP from being issued. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261103-panfrost-reset-races-fixes-v2-0-454207b1d...@collabora.com?part=1
