Re: [PATCH 1/2] perf evsel: Add lazy-initialized probe type detection helpers

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Tue, Jun 16, 2026 at 06:40:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Ftrace-based return probes (kretprobe, uretprobe, fexit) are incorrectly classified as PROBE__NOPE because they lack the __probe_ip field.
> --
> 
> commit fc58c48524e7c99a712f8fb111c94abfbbeb4b6d
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf evsel: Add lazy-initialized probe type detection helpers
> 
> This commit introduces lazy-initialized probe type detection helpers for
> perf evsel. It caches the probe type lookup in a 3-bit field within struct
> evsel to avoid repeated PMU list walks via evsel__find_pmu().
> 
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 05fa0010c858a..ea9fa04429f08 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> 
> [ ... ]
> 
> > @@ -254,6 +254,58 @@ const char *evsel__pmu_name(const struct evsel *evsel)
> >  	return event_type(evsel->core.attr.type);
> >  }
> >  
> > +enum evsel_probe_type {
> > +	PROBE__UNKNOWN	= 0,
> > +	PROBE__NOPE	= 1,
> > +	PROBE__KPROBE	= 2,
> > +	PROBE__UPROBE	= 3,
> > +	/*
> > +	 * Ftrace-based dynamic probes (kprobes/uprobes/fprobes created via
> > +	 * tracefs) report PMU "tracepoint", not "kprobe"/"uprobe".  Detect
> > +	 * them by the __probe_ip field that the kernel adds to all dynamic
> > +	 * probe formats.
> > +	 */
> > +	PROBE__FTRACE	= 4,
> > +};
> > +
> > +static void evsel__resolve_probe_type(struct evsel *evsel)
> > +{
> > +	const char *name = evsel__pmu_name(evsel);
> > +
> > +	if (!strcmp(name, "kprobe"))
> > +		evsel->probe_type = PROBE__KPROBE;
> > +	else if (!strcmp(name, "uprobe"))
> > +		evsel->probe_type = PROBE__UPROBE;
> > +	else if (!strcmp(name, "tracepoint") && evsel__field(evsel, "__probe_ip"))
> 
> [Severity: Medium]
> The commit message mentions this covers kprobes, uprobes, and fprobes
> regardless of their group or system name by checking the __probe_ip field.
> Will this successfully detect ftrace-based return probes?
> 
> It looks like return probes like kretprobe, uretprobe, and fexit do not
> contain a __probe_ip field, but instead define __probe_func and
> __probe_ret_ip.
> 
> If evsel__field() returns NULL for return probes, won't they incorrectly
> fall through to the else branch and be classified as PROBE__NOPE?

Sure, and this is an interesting observation, we need to cover
__probe_ret_ip in the same fashion as __probe_ip, probably __probe_func
as well, but this should be done in an upcoming patch series.

I'll make sure to save this in a TODO file for later work.

- Arnaldo
 
> > +		evsel->probe_type = PROBE__FTRACE;
> > +	else
> > +		evsel->probe_type = PROBE__NOPE;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260616182546.8892-1-acme@kernel.org?part=1