Re: [PATCH v6 2/4] perf sched: Handle missing trace samples in pipe mode

On Sun, Aug 02, 2026 at 12:01:40AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Using `evsel->handler == NULL` to conditionally initialize handlers inside the sample processing hot path causes repeated O(N) evlist iterations for unhandled events and drops late-arriving tracepoints.
> 
> Pre-existing issues:
> - [High] `symbol__init` accesses the `perf_env` before it is populated by pipe feature events, leading to incorrect host-fallback initialization for cross-platform traces.
> - [High] `perf sched timehist` remains completely broken for pipe mode streams.
> --
> 
> --- Patch [2]: [PATCH v6 2/4] perf sched: Handle missing trace samples in pipe mode ---
> commit bdcbf92a5b81c957ee21cce440488c2da3569c14
> Author: Aaron Tomlin <[email protected]>
> 
> perf sched: Handle missing trace samples in pipe mode
> 
> For pipe mode streams, event attributes are received dynamically during
> event processing, meaning session->evlist is not populated prior to
> perf_session__process_events(). To handle pipe input correctly:
>   - Register the missing .attr, .tracing_data, .build_id, and .feature
>     callbacks in cmd_sched()
>   - Promote the handlers array to file-scope (latency_handlers[]) and
>     invoke evlist__set_tracepoints_handlers() dynamically inside
>     perf_sched__process_tracepoint_sample() when evsel->handler is NULL
>   - Perform the trace check post-processing when handling pipe data
> 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 94a323da0799a..0abf9f1f3c2e2 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> [ ... ]
> > @@ -1946,6 +1955,14 @@ static int perf_sched__process_tracepoint_sample(const struct perf_tool *tool __
> >  	struct evsel *evsel = sample->evsel;
> >  	int err = 0;
> >  
> > +	if (evsel->handler == NULL && sample->evsel->evlist) {
> > +		/* prefer sched_waking if it is captured */
> > +		if (evlist__find_tracepoint_by_name(sample->evsel->evlist, "sched:sched_waking"))
> > +			latency_handlers[2].handler = process_sched_wakeup_ignore;
> > +
> > +		evlist__set_tracepoints_handlers(sample->evsel->evlist, latency_handlers);
> > +	}
> > +
> 
> [Severity: High]
> Does this cause an O(N) traversal on every sample for unhandled events?
> 
> If a tracepoint is explicitly recorded but not in latency_handlers (such as
> sched:sched_process_fork), evsel->handler will remain NULL. For every sample
> of these unhandled events, this condition evaluates to true.
> 
> This repeatedly triggers evlist__find_tracepoint_by_name() and a call to
> evlist__set_tracepoints_handlers(), which immediately aborts with -EEXIST.
> 
> Could this drop late-arriving tracepoints and cause a significant performance
> regression on the event processing hot path?
> 
> >  	if (evsel->handler != NULL) {
> >  		tracepoint_handler f = evsel->handler;
> >  		err = f(tool, sample, machine);
> [ ... ]
> > @@ -2011,14 +2020,17 @@ static int perf_sched__read_events(struct perf_sched *sched)
> >  
> >  	symbol__init(perf_session__env(session));
> 

Hi Namhyung, Arnaldo, Ian,

This is a valid identified problem.

Because unhandled tracepoints (e.g. sched:sched_process_fork) are not in
latency_handlers[], evsel->handler remained NULL. This caused every
subsequent sample of an unhandled event to re-trigger the O(N) evlist
search and evlist__set_tracepoints_handlers() call on the sample processing
hot path.

Furthermore, __evlist__set_tracepoints_handlers() aborts immediately with
-EEXIST as soon as it encounters a non-NULL handler (e.g.,
sched:sched_switch), which prevented handlers for late-arriving tracepoints
in pipe mode from being registered and caused their samples to be dropped.

To fix this, I have updated perf_sched__process_tracepoint_sample() to
perform a single-pass, per-evsel lazy handler lookup and assign a dummy
sentinel handler (process_sched_ignore) for unhandled events.

This guarantees that evsel->handler is non-NULL after its first sample,
eliminating the hot-path O(N) traversal, bypassing -EEXIST aborts, and
ensuring late-arriving tracepoints in pipe mode are correctly bound.

> [Severity: High]
> This is a pre-existing issue, but does this early symbol__init() call break
> cross-platform pipe mode?
> 
> In pipe mode, perf_env is populated during event processing. Calling
> symbol__init() here accesses an uninitialized env->arch.
> 
> This causes perf_env__os_release() to fall back to the host's uname(),
> permanently poisoning env->os_release and vmlinux_path__init() with host
> paths.
> 
> > -	/* prefer sched_waking if it is captured */
> > -	if (evlist__find_tracepoint_by_name(session->evlist, "sched:sched_waking"))
> > -		handlers[2].handler = process_sched_wakeup_ignore;
> [ ... ]
> 
> I also noticed a related pre-existing issue in perf_sched__timehist()
> where timehist remains broken for pipe mode streams:
> 
> tools/perf/builtin-sched.c:perf_sched__timehist() {
> [ ... ]
> 	/* setup per-evsel handlers */
> 	if (perf_session__set_tracepoints_handlers(session, handlers))
> 		goto out;
> 
> 	/* sched_switch event at a minimum needs to exist */
> 	if (!evlist__find_tracepoint_by_name(session->evlist, "sched:sched_switch")) {
> 		pr_err("No sched_switch events found. Have you run 'perf sched record'?\n");
> 		goto out;
> 	}
> [ ... ]
> }
> 
> [Severity: High]
> This is a pre-existing issue, but does this strict check break the
> perf sched timehist subcommand for pipe streams?
> 
> Since the evlist is unpopulated before processing pipe events,
> evlist__find_tracepoint_by_name() fails immediately, causing the timehist
> command to abort.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

The above are valid too.

However, I will address all of the above in a separate series.


Kind regards,
-- 
Aaron Tomlin