Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing goto-based error handling with scoped_guard() cleanup in panfrost_perfcnt_enable_locked() violates cleanup API semantics. --
commit 6a11ea2bc38694e85eb36147ae0e357afc1cc799 Author: Adrián Larumbe <[email protected]> drm/panfrost: Fix races between perfcnt and reset sequence This commit addresses concurrency issues between the reset sequence and perfcnt session teardown by governing hardware access with the reset r/w semaphore. It also expands the uAPI to notify userspace if a reset has occurred during a perfcnt session. > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > index b3f71d7fd82a9..9847657179a57 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c [ ... ] > @@ -122,54 +203,31 @@ static int panfrost_perfcnt_enable_locked(struct > panfrost_device *pfdev, > ret = drm_gem_vmap(&bo->base, &map); > if (ret) > goto err_put_mapping; > + > perfcnt->buf = map.vaddr; > + perfcnt->counterset = counterset; > > panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer"); > [ ... ] > + scoped_guard(rwsem_read, &pfdev->reset.lock) { > + /* > + * Clear the counters to start from a fresh state. > + */ > + gpu_write(pfdev, GPU_INT_CLEAR, > GPU_IRQ_PERFCNT_SAMPLE_COMPLETED); > + gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR); > > - gpu_write(pfdev, GPU_PERFCNT_CFG, cfg); > + ret = panfrost_perfcnt_hw_enable(pfdev); > + if (ret) > + goto err_vunmap; [Severity: Low] Does this mix goto-based error handling with scope-based cleanup? The kernel cleanup API guidelines state that we should never mix goto labels and cleanup helpers (like scoped_guard) in the same function. Jumping out of a scoped_guard block to an error label creates confusing ownership semantics. For a given routine, the documentation recommends converting all resources that need a goto cleanup to scope-based cleanup, or converting none of them. > - if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186)) > - gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff); > + perfcnt->reset_happened = false; > + perfcnt->owns_as_ref = true; > + perfcnt->user = user; > + } > > /* The BO ref is retained by the mapping. */ > drm_gem_object_put(&bo->base); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
