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

[sashiko-bot@kernel.org]

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?

> +		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