Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

perf stat: Fix handling of --for-each-cgroup with --bpf-counters to match non BPF mode

The --for-each-cgroup can have the same cgroup multiple times, but this
confuses BPF counters (since they have the same cgroup id), making only
the last cgroup events to be counted.

Let's check the cgroup name before adding a new entry to the cgroups
list.

Before:

$ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1

Performance counter stats for 'system wide':

<not counted> msec cpu-clock /
<not counted> context-switches /
<not counted> cpu-migrations /
<not counted> page-faults /
<not counted> cycles /
<not counted> instructions /
<not counted> branches /
<not counted> branch-misses /
8,016.04 msec cpu-clock / # 7.998 CPUs utilized
6,152 context-switches / # 767.461 /sec
250 cpu-migrations / # 31.187 /sec
442 page-faults / # 55.139 /sec
613,111,487 cycles / # 0.076 GHz
280,599,604 instructions / # 0.46 insn per cycle
57,692,724 branches / # 7.197 M/sec
3,385,168 branch-misses / # 5.87% of all branches

1.002220125 seconds time elapsed

After it becomes similar to the non-BPF mode:

$ sudo ./perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1

Performance counter stats for 'system wide':

8,013.38 msec cpu-clock / # 7.998 CPUs utilized
6,859 context-switches / # 855.944 /sec
334 cpu-migrations / # 41.680 /sec
345 page-faults / # 43.053 /sec
782,326,119 cycles / # 0.098 GHz
471,645,724 instructions / # 0.60 insn per cycle
94,963,430 branches / # 11.851 M/sec
3,685,511 branch-misses / # 3.88% of all branches

1.001864539 seconds time elapsed

Committer notes:

As a reminder, to test with BPF counters one has to use BUILD_BPF_SKEL=1
in the make command line and have clang/llvm installed when building
perf, otherwise the --bpf-counters option will not be available:

# perf stat -a --bpf-counters --for-each-cgroup /,/ sleep 1
Error: unknown option `bpf-counters'

Usage: perf stat [<options>] [<command>]

-a, --all-cpus system-wide collection from all CPUs
<SNIP>
#

Fixes: bb1c15b60b981d10 ("perf stat: Support regex pattern in --for-each-cgroup")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: bpf@vger.kernel.org
Cc: Ian Rogers <irogers@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Song Liu <songliubraving@fb.com>
Link: https://lore.kernel.org/r/20230104064402.1551516-5-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

authored by

Namhyung Kim and committed by
Arnaldo Carvalho de Melo
54b353a2 2d656b0f

+18 -5
+18 -5
tools/perf/util/cgroup.c
··· 224 224 return 0; 225 225 } 226 226 227 + static int check_and_add_cgroup_name(const char *fpath) 228 + { 229 + struct cgroup_name *cn; 230 + 231 + list_for_each_entry(cn, &cgroup_list, list) { 232 + if (!strcmp(cn->name, fpath)) 233 + return 0; 234 + } 235 + 236 + /* pretend if it's added by ftw() */ 237 + return add_cgroup_name(fpath, NULL, FTW_D, NULL); 238 + } 239 + 227 240 static void release_cgroup_list(void) 228 241 { 229 242 struct cgroup_name *cn; ··· 255 242 struct cgroup_name *cn; 256 243 char *s; 257 244 258 - /* use given name as is - for testing purpose */ 245 + /* use given name as is when no regex is given */ 259 246 for (;;) { 260 247 p = strchr(str, ','); 261 248 e = p ? p : eos; ··· 266 253 s = strndup(str, e - str); 267 254 if (!s) 268 255 return -1; 269 - /* pretend if it's added by ftw() */ 270 - ret = add_cgroup_name(s, NULL, FTW_D, NULL); 256 + 257 + ret = check_and_add_cgroup_name(s); 271 258 free(s); 272 - if (ret) 259 + if (ret < 0) 273 260 return -1; 274 261 } else { 275 - if (add_cgroup_name("", NULL, FTW_D, NULL) < 0) 262 + if (check_and_add_cgroup_name("/") < 0) 276 263 return -1; 277 264 } 278 265