Re: [PATCH] perf/core: Fix ITRACE start suppression for inherited events

[Leo Yan <leo.yan@arm.com>]

On Mon, Sep 28, 2026 at 10:38:42AM +0100, James Clark wrote:
> 
> On 03/09/2026 10:35, Leo Yan wrote:
> > PMU drivers call perf_event_itrace_started() for the event that has
> > started tracing. This sets PERF_ATTACH_ITRACE in that event's
> > attach_state.
> > 
> > For inherited events, however, perf_log_itrace_start() replaces the
> > child event with its parent before checking PERF_ATTACH_ITRACE. The
> > setter and checker therefore operate on different events. If the
> > parent's flag is clear, the child continues to emit ITRACE_START
> > records on subsequent schedule-ins. If the parent has already started,
> > its flag can instead suppress the child's initial record.
> 
> This last part would need an earlier fixes: commit. The problem of
> suppressing child ITRACE_START records existed since the beginning on
> ec0d772 ("perf: Add ITRACE_START record to indicate that tracing has
> started").
> 
> Although the fixes: commit would be correct if the only problem was that
> setting and getting are on different events.

Makes sense. However, if follow your suggestion to change the setter to
fix the setter/checker mismatch, 9a6694cfa239 remains the appropriate
Fixes tag, since it introduced that mismatch; and it is a feasible
point for back port.

> > Remove the parent substitution so that perf_log_itrace_start() checks
> > the same event that the PMU driver marks as started.
> > 
> > This is safe for tool consumers. Intel PT uses the ITRACE_START record
> > to set the current thread context. CoreSight ETM uses the record only
> > to find or create the corresponding thread. Neither decoder depends on
> > the parent event.
> > 
> 
> It's probably harmless to emit more ITRACE_STARTs, but it doesn't fit the
> original purpose of why it was added. It seems to be for when tracing first
> starts, which would be _after_ the corresponding sched event. Once tracing
> has started you can follow the subsequent sched events, so you don't need
> more ITRACE_START records for each child. Could we not fix it by changing
> the setter to follow the parent event to match, which would respect the
> original meaning?

My concern was whether an inherited event and its parent could trace
concurrently on different CPUs.

Normal inherited AUX recording uses CPU restricted events (cpu != -1),
children inherit the parent event’s CPU restriction, a particular parent
event and its children cannot trace concurrently on different CPUs.

Since inherit with cpu == -1 prevents buffer mapping [1], my concern for
this case is also not valid.

I will update the patch to update setter. Thanks for suggestion.

[1] https://man7.org/linux/man-pages/man2/perf_event_open.2.html