Re: [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a potential modulo by zero crash when computing `evsel->core.idx % num_events`.
--

commit d07a1cf9e5bf6b04df795e0b41782b1fa5a160cb
Author: Ian Rogers <irogers@google.com>

perf stat: Propagate supported flag to follower cgroup BPF events

When using BPF counters with cgroups, follower events are not opened, leaving
their supported flag as false and causing metric calculations to report nan.
This commit fixes the issue by propagating the supported flag from the leader
events to the follower events.

> diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c
> index 519fee3dc3d03..dd1851634087e 100644
> --- a/tools/perf/util/bpf_counter_cgroup.c
> +++ b/tools/perf/util/bpf_counter_cgroup.c
[ ... ]
> @@ -186,6 +186,21 @@ static int bperf_load_program(struct evlist *evlist)
>  		i++;
>  	}
>  
> +	/*
> +	 * Propagate supported flag from leaders to followers. Follower events
> +	 * are not opened, so their supported flag remains false.
> +	 */
> +	{
> +		struct evsel *leader;
> +		int num_events = evlist->core.nr_entries / nr_cgroups;
> +
> +		evlist__for_each_entry(evlist, evsel) {
> +			leader = evlist__find_evsel(evlist, evsel->core.idx % num_events);

Can num_events evaluate to 0 here and cause a modulo by zero exception?

If a user runs perf stat with a trailing comma in the cgroup list (e.g.,
perf stat --bpf-counters -e event1 -G cgrp1,), parse_cgroups() in
tools/perf/util/cgroup.c skips the empty string but still increments
nr_cgroups.

This results in nr_cgroups being greater than the number of events, making
evlist->core.nr_entries / nr_cgroups evaluate to 0.

While setup_rodata() called earlier contains a
BUG_ON(evlist_size % nr_cgroups != 0) check, non-debug builds compile with
-DNDEBUG=1 by default, making BUG_ON a no-op. This allows execution to
reach this point and crash with a SIGFPE.

> +			if (leader)
> +				evsel->supported = leader->supported;
> +		}
> +	}
> +
>  	/*
>  	 * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check
>  	 * whether the kernel support it

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519050150.3144907-1-irogers@google.com?part=1