Re: [PATCH] perf stat: Do not open cgroups for BPF counters

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Jul 08, 2026 at 08:14:33PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Fallback from BPF counter mode to standard cgroup mode leaves cgroup file descriptors uninitialized (-1), causing standard perf event opening to fail with -EBADF.
> --
> 
> --- Patch [1]: [PATCH] perf stat: Do not open cgroups for BPF counters ---
> commit 510c63ce03b2362940d1f693b45d5faa86cf3bc1
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf stat: Do not open cgroups for BPF counters
> 
> This commit prevents opening and keeping file descriptors for cgroups
> when using the --bpf-counters and --for-each-cgroup options. BPF mode
> uses a set of shared events and compares cgroup-IDs, so it doesn't need
> individual cgroup file descriptors.
> 
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2874,7 +2874,8 @@ int cmd_stat(int argc, const char **argv)
> >  			goto out;
> >  		}
> >  
> > -		if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list, true) < 0) {
> > +		if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list,
> > +					  !target.use_bpf) < 0) {
> 
> [Severity: Medium]
> Will this break the fallback path when we exceed BPERF_CGROUP__MAX_EVENTS?
> 
> Later in cmd_stat(), we have a fallback that disables BPF counters if there
> are too many events:
> 
> 	if (target.use_bpf && nr_cgroups &&
> 	    (evsel_list->core.nr_entries / nr_cgroups) > BPERF_CGROUP__MAX_EVENTS) {
> 		pr_warning("Disabling BPF counters due to more events...");
> 		target.use_bpf = false;
> 	}
> 
> If this fallback is triggered, target.use_bpf becomes false, but the cgroups
> were already expanded with !target.use_bpf (which was false at the time),
> leaving the cgroup file descriptors uninitialized (-1).

I think it's a false alarm.  The evlist__expand_cgroup() is only used if
stat_config.cgroup_list is set and then it should not have nr_cgroups.
There's a check for that right before the call to the function.

So in that case, it cannot pass the if condition below and never set to
target.use_bpf to false.

Thanks,
Namhyung