On Tue, Jul 28, 2026 at 04:00:13PM +0100, James Clark wrote:
The linked fixes commit deliberately allows reads of an old sysfs buffer while in Perf mode because they are separate software buffers. However it didn't modify tmc_disable_etr_sink() to match this relaxation. The result is that when a Perf event ends while the sysfs buffer is being read, clean up will be skipped.
Fix it by ignoring the sysfs_reading flag unless the active session is a sysfs one.
I am not clear why this is relevant to per-thread mode, maybe it would be clear for me after reading other patches.
For this patch self, I think the issue comes from the sink buffer's life time - SYSFS mode's buffer has longer life time than a sysfs session, it is designed to allow reading sysfs trace data after the sysfs session, or even during a perf session. So the code gets complex for handling these cases.
I really think we should have a correct life time (or state machine) for sysfs buffer, something like:
Operations | ETR sysfs buffer state machine ----------------------------------+----------------------------------------- | INVALID: Init state, buffer unallocated echo 1 > .../tmc_etr0/enable_sink | READY: user can read zeroed data echo 1 > .../etm0/enable_source | BUSY: user cannot read as hardware is | writing data to buffer echo 0 > .../etm0/enable_source | READY: trace is stopped, user can read | trace data echo 0 > .../tmc_etr0/enable_sink | INVALID: buffer has been released
With the state machine's help, we might even don't need to bother what the sink device's mode is.
When sysfs and Perf share the same memory in ETR_MODE_RESRV mode, a new Perf session needs to overwrite an old inactive sysfs session by zeroing len. This avoids sysfs from reading stale data because it has a separate set of offsets in its etr_buf struct, even if that's backed by the same memory as the Perf one.
This can be naturally resolved if we have a state machine above?
[...]
+static bool tmc_perf_sysfs_shared(struct tmc_drvdata *drvdata,
struct etr_buf *perf_buf)+{
- /* In ETR_MODE_RESRV mode, sysfs and Perf share the same memory. */
- return perf_buf &&
drvdata->sysfs_buf &&drvdata->sysfs_buf->mode == ETR_MODE_RESRV &&perf_buf->mode == ETR_MODE_RESRV;+}
static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, struct coresight_path *path) { @@ -1772,6 +1782,18 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, goto unlock_out; }
- /*
* Don't use if it's shared and being read by sysfs. Sysfs may only* start reading (the cleared zero length buffer) after the first* tmc_enable_etr_sink_perf(), which changes the result of this check,* so it should only be done once.*/- if ((drvdata->sysfs_reading &&
tmc_perf_sysfs_shared(drvdata, etr_perf->etr_buf))) {rc = -EBUSY;goto unlock_out;- }
This is quite tricky. If someone reads the sysfs entry in the middle of a perf session, and the traced task is migrated to a different CPU or wakes up, the sink device may need to be re-enabled, however the sink enabling could fail due to the concurrent sysfs read here.
Thanks, Leo