Hello, I know I'm a bit late, but I've found a bug.
On Monday, 15 December 2025 18:14:50 Central European Summer Time Lukas Zapolskas wrote: > To allow for combining the requests from multiple userspace clients, > an intermediary layer between the HW/FW interfaces and userspace is > created, containing the information for the counter requests and > tracking of insert and extract indices. Each session starts inactive > and must be explicitly activated via PERF_CONTROL.START, and > explicitly stopped via PERF_CONTROL.STOP. Userspace identifies a > single client with its session ID and the panthor file it is > associated with. > > The SAMPLE and STOP commands both produce a single sample when called, > and these samples can be disambiguated via the opaque user data field > passed in the PERF_CONTROL uAPI. If this functionality is not desired, > these fields can be kept as zero, as the kernel copies this value into > the corresponding sample without attempting to interpret it. > > Currently, only manual sampling sessions are supported, providing > samples when userspace calls PERF_CONTROL.SAMPLE, and only a single > session is allowed at a time. Multiple sessions and periodic sampling > will be enabled in following patches. > > No protection is provided against the 32-bit hardware counter > overflows, so for the moment it is up to userspace to ensure that > the counters are sampled at a reasonable frequency. > > The counter set enum is added to the uapi to clarify the restrictions > on calling the interface. > > Signed-off-by: Lukas Zapolskas <[email protected]> > --- > drivers/gpu/drm/panthor/panthor_perf.c | 706 ++++++++++++++++++++++++- > drivers/gpu/drm/panthor/panthor_perf.h | 16 + > 2 files changed, 716 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_perf.c > b/drivers/gpu/drm/panthor/panthor_perf.c > index 3a65d6d326e8..cea8f678c4e1 100644 > --- a/drivers/gpu/drm/panthor/panthor_perf.c > +++ b/drivers/gpu/drm/panthor/panthor_perf.c > [ ... snip ...] > + > +static int session_destroy(struct panthor_perf *perf, struct > panthor_perf_session *session) > +{ > + session_put(session); > + > + return 0; > +} > + > +static int session_teardown(struct panthor_perf *perf, struct > panthor_perf_session *session) > +{ > + if (test_bit(PANTHOR_PERF_SESSION_ACTIVE, session->state)) > + return -EINVAL; > + > + if (READ_ONCE(session->pending_sample_request) != SAMPLE_TYPE_NONE) > + return -EBUSY; > + > + return session_destroy(perf, session); > +} > + > +/** > + * panthor_perf_session_teardown - Teardown the session associated with the > @sid. > + * @pfile: Open panthor file. > + * @perf: Handle to the perf control structure. > + * @sid: Session identifier. > + * > + * Destroys a stopped session where the last sample has been explicitly > consumed > + * or discarded. Active sessions will be ignored. > + * > + * Return: 0 on success, negative error code on failure. > + */ > +int panthor_perf_session_teardown(struct panthor_file *pfile, struct > panthor_perf *perf, u32 sid) > +{ > + int err; > + struct panthor_perf_session *session; > + > + xa_lock(&perf->sessions); > + session = __xa_erase(&perf->sessions, sid); This also needs to check for !session. __xa_erase will return NULL on a bogus `sid`, and the ioctl handler doesn't actually check if the `sid` userspace passed in is still around. A simple if (!session) { xa_unlock(&perf->sessions); return -EINVAL; } did the trick for me. Kind regards, Nicolas Frattaroli > + > + if (xa_is_err(session)) { > + err = xa_err(session); > + goto restore; > + } > + > + if (session->pfile != pfile) { > + err = -EINVAL; > + goto restore; > + } > + > + session_get(session); > + xa_unlock(&perf->sessions); > + > + err = session_teardown(perf, session); > + > + session_put(session); > + > + return err; > + > +restore: > + __xa_store(&perf->sessions, sid, session, GFP_KERNEL); > + xa_unlock(&perf->sessions); > + > + return err; > +} > + > [... snip ...]
