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

On Mon, Aug 03, 2026 at 10:59:24AM -0700, Namhyung Kim wrote:
> On Sun, Aug 02, 2026 at 05:09:12PM -0400, Aaron Tomlin wrote:
> > 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
> >     dynamically assign matching tracepoint handlers (or a dummy ignore
> >     handler) inside perf_sched__process_tracepoint_sample() when
> >     evsel->handler is NULL
> > 
> >   - Perform the trace check post-processing when handling pipe data
> > 
> > Fixes: 27295592c22e ("perf session: Share the common trace sample_check routine as perf_session__has_traces")
> > Signed-off-by: Aaron Tomlin <[email protected]>
> > ---
> >  tools/perf/builtin-sched.c | 65 +++++++++++++++++++++++++++++---------
> >  1 file changed, 50 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 94a323da0799..232e72537df3 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -1938,6 +1938,22 @@ typedef int (*tracepoint_handler)(const struct perf_tool *tool,
> >  				  struct perf_sample *sample,
> >  				  struct machine *machine);
> >  
> > +static struct evsel_str_handler latency_handlers[] = {
> > +	{ "sched:sched_switch",       process_sched_switch_event, },
> > +	{ "sched:sched_stat_runtime", process_sched_runtime_event, },
> > +	{ "sched:sched_wakeup",       process_sched_wakeup_event, },
> > +	{ "sched:sched_waking",       process_sched_wakeup_event, },
> > +	{ "sched:sched_wakeup_new",   process_sched_wakeup_event, },
> > +	{ "sched:sched_migrate_task", process_sched_migrate_task_event, },
> > +};
> > +
> > +static int process_sched_ignore(const struct perf_tool *tool __maybe_unused,
> > +				struct perf_sample *sample __maybe_unused,
> > +				struct machine *machine __maybe_unused)
> > +{
> > +	return 0;
> > +}
> > +
> >  static int perf_sched__process_tracepoint_sample(const struct perf_tool *tool __maybe_unused,
> >  						 union perf_event *event __maybe_unused,
> >  						 struct perf_sample *sample,
> > @@ -1946,7 +1962,22 @@ static int perf_sched__process_tracepoint_sample(const struct perf_tool *tool __
> >  	struct evsel *evsel = sample->evsel;
> >  	int err = 0;
> >  
> > -	if (evsel->handler != NULL) {
> > +	if (evsel->handler == NULL) {
> > +		evsel->handler = process_sched_ignore;
> > +		for (size_t i = 0; i < ARRAY_SIZE(latency_handlers); i++) {
> > +			if (evsel__name_is(evsel, latency_handlers[i].name)) {
> 
> Please consider using 'continue' statement for the opposite case to
> reduce the indent level.

Acknowledged.

> > +				if (!strcmp(latency_handlers[i].name, "sched:sched_wakeup") &&
> > +				    sample->evsel->evlist &&
> > +				    evlist__find_tracepoint_by_name(sample->evsel->evlist, "sched:sched_waking"))
> > +					evsel->handler = process_sched_wakeup_ignore;
> 
> Probably better to use/keep the process_sched_ignore().
> 
> I think we could replace others to use the function and get rid of the
> dummy wakeup_ignore functions.  But it could be a separate change.

I can resolve this now—it'll be cleaner.


Kind regards,
-- 
Aaron Tomlin