Re: [PATCH v3 4/8] coresight: tmc-etr: Prevent per-thread events from sharing a sink

[James Clark]

On 25/08/2026 18:32, Leo Yan wrote:
> On Thu, Aug 20, 2026 at 12:09:25PM +0100, James Clark wrote:
>
> [...]
>
> > > I am just wandering if we can improve the sink driver to only allocate
> > > a single bounce buffer that is independent of any threads (and any
> > > associated events).
> > >
> > >                   | T1 |
> > >     CPU0        ------------------------------
> > >                      |   T2      |
> > >     CPU1        ------------------------------
> > >                                  `> T2 stops and can sync trace
> > >                                     from the shared bounce buffer
> > >                                     to AUX_BUF(T2).
> > >
> > >     AUX_BUF(T1) |                            |
> > >     AUX_BUF(T2) |                            |
> > >
> > >     ETR_BUF     | Bounce buf                 |  -> Used by H/W trace
> > >
> > > This might also simplify the CPU-wide case. Each CPU would still have
> > > its own AUX buffer, but the ETR driver would maintain only one bounce
> > > buffer for the shared sink. A reference count could track how many
> > > events are using the sink, with the final event responsible for

What size and mode (CATU, FLAT, etc) would the one bounce buffer be? 
Isn't the problem that a bounce buffer per-session solves that the user 
can pick a different size and mode for each session?

> > Isn't this how it's already working? get_perf_etr_buf_cpu_wide() allocates a
> > single shared buffer with a refcount. I didn't change this, I only changed
> > the rules about what is considered shared or not so that it matches the
> > semantics of the perf events that back the tracing session.
>
> I think this is slightly different from my point.
>
> The CPU-wide path already uses a shared buffer with a reference count
> to support multiple events, while the per-thread path does not.
>
> For the longe term, I would prefer to unify the sink buffer management.
> Ideally, ETR/ETF/ETB should manage the sink buffer in the same way
> regardless of whether the users come from CPU-wide or per-thread modes.
> This would keep perf event semantics out of the low-level sink drivers
> as much as possible. However, this would be a larger change and we could
> defer in the future.

I suppose I'm a bit confused about how it relates to this fix. All the 
buffer management stuff is transparent to the user, it doesn't really 
matter how the driver does it. This fix fixes a bug relating to a user 
visible behavior. If there is a way to simplify the buffer management 
and preserve all of the use cases we can do it, but I don't see why it 
needs to be done now rather than later.

> Now I treat the multiple events in per-thread mode as an implementation
> limitation. For the immediate fix, we just reject this case instead.
>
> > > We use a central place etm_event_build_path() to record and compare
> > > event's owner and target process, then we don't need to spread the
> > > check into sink drivers. We only care about if owner and target must
> > > be consistent.
>
> I experimented with moving the check to a common place during buffer
> allocation: https://termbin.com/dib3r
>
> It needs locking to keep the check in atomicity, but seems doable. We
> don't need to spread event checks across the different sink drivers.

I agree it would be possible to do, my only argument is that it doesn't 
support all the uses cases that we currently support and that are likely 
in use.

> > But we don't know where the target will run when the event is created.
> > That's why the check is delayed until etm_event_start() and the process has
> > been scheduled. Where it runs needs to be taken into account to calculate if
> > this sink can be shared.
>
> Adding the check in etm_event_start() makes the result depend on task
> scheduling.

I didn't add it, it's already there for owner PID so it already depends 
on scheduling. And not checking target TID leads to a WARN on 
etm_event_start() as well. So nothing is being added, just the existing 
check is being made more restrictive and the WARN is fixed.

> I understand some cases you mentioned may benefit from this, but it also
> makes the behaviour less deterministic. I would prefer to reject
> unsupported cases explicitly when opening the events.

You didn't explain how you would account for taking away users' 
currently working use cases. Two users using taskset to run on two 
different cores does behave deterministically. But you are taking that 
away from them by preventing any event from being opened that _may_ 
share a sink in the future, even if it never does.

Taking away concurrent per-thread sessions seems like a huge 
functionality loss.

We can agree to do it and change that behavior, but that's a completely 
different change than fixing this bug with the existing behavior, and 
I'm not sure what the justification for it would be. "Determinism" seems 
like a weak argument when the fix is to make the driver much less 
flexible and useful.

I don't think we would want that change to be a "fixes:" commit either. 
There is some chance it would have to be rolled back if someone 
complains, and then we'd get the WARN back again.

> Thanks,
> Leo