* [PATCH v1 0/2] Fix cgroup metric association with BPF counters
@ 2026-05-18 22:22 Ian Rogers
2026-05-18 22:22 ` [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Ian Rogers @ 2026-05-18 22:22 UTC (permalink / raw)
To: skanev, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
linux-kernel, linux-perf-users
This series fixes an issue where cgroup metrics were not being correctly
associated and reported (showing `nan %`) when using BPF counters
(i.e., with `--bpf-counters`).
The root cause is that cgroup BPF counters only open the "leader" events
(for the first cgroup) and leave "follower" events unopened. Unopened
events have their `supported` flag set to `false` by default. During
metric calculation, `prepare_metric` checks `evsel->supported` and
discards the value (setting it to `NAN`) if it is `false`, leading to
`nan %` in the output.
The first patch fixes this by propagating the `supported` flag from the
leader events to the follower events in `bperf_load_program`.
The second patch adds a new shell test (`stat_metrics_cgrp.sh`) to
permanently cover this scenario (testing both with and without BPF
counters) and prevent regressions.
Reported-by: Svilen Kanev <skanev@google.com>
Ian Rogers (2):
perf stat: Propagate supported flag to follower cgroup BPF events
perf test: Add stat metrics --for-each-cgroup test
tools/perf/tests/shell/stat_metrics_cgrp.sh | 123 ++++++++++++++++++++
tools/perf/util/bpf_counter_cgroup.c | 15 +++
2 files changed, 138 insertions(+)
create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh
--
2.54.0.631.ge1b05301d1-goog
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events 2026-05-18 22:22 [PATCH v1 0/2] Fix cgroup metric association with BPF counters Ian Rogers @ 2026-05-18 22:22 ` Ian Rogers 2026-05-18 22:22 ` [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-18 22:22 UTC (permalink / raw) To: skanev, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, linux-kernel, linux-perf-users When using BPF counters with cgroups, follower events (for cgroups other than the first one) are not opened. Because they are not opened, their `supported` flag was left as `false`. During metric calculation, `prepare_metric` checks if the event is supported. If it is not supported (like the follower events), it explicitly sets the value to `NAN`, which eventually causes the metric to be reported as `nan %`. Fix this by propagating the `supported` flag from the "leader" events (the ones opened for the first cgroup) to the "follower" events. Reported-by: Svilen Kanev <skanev@google.com> Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter_cgroup.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index 519fee3dc3d0..dd1851634087 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); + if (leader) + evsel->supported = leader->supported; + } + } + /* * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check * whether the kernel support it -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-18 22:22 [PATCH v1 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-18 22:22 ` [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers @ 2026-05-18 22:22 ` Ian Rogers 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-18 22:22 UTC (permalink / raw) To: skanev, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark, linux-kernel, linux-perf-users Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting with `--for-each-cgroup`, both with and without `--bpf-counters`. The test: - Checks if system-wide monitoring is supported (skips if not). - Checks if `insn_per_cycle` metric is supported (skips if not). - Finds cgroups to test. - Runs `perf stat` with `insn_per_cycle` metric and verifies that the metric is reported for each cgroup and does not contain invalid values (like `nan` or `nested`) or empty values when counts are present. - Tests both standard mode and BPF counters mode (if supported). Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/tests/shell/stat_metrics_cgrp.sh | 123 ++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh new file mode 100755 index 000000000000..4a29eb9a9067 --- /dev/null +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh @@ -0,0 +1,123 @@ +#!/bin/bash +# perf stat metrics --for-each-cgroup test +# SPDX-License-Identifier: GPL-2.0 + +set -e + +test_cgroups= + +log_verbose() { + echo "$1" +} + +# skip if system-wide is not supported +check_system_wide() +{ + log_verbose "Checking system-wide..." + if ! perf stat -a -e instructions sleep 0.01 > /dev/null 2>&1 + then + log_verbose "Skipping: system-wide monitoring not supported" + exit 2 + fi +} + +# find two cgroups to measure +find_cgroups() +{ + log_verbose "Finding cgroups..." + # try usual systemd slices first + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] + then + test_cgroups="system.slice,user.slice" + log_verbose "Found cgroups: ${test_cgroups}" + return + fi + + # try root and self cgroups + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) + if [ -z ${find_cgroups_self_cgrp} ] + then + # cgroup v2 doesn't specify perf_event + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) + fi + + if [ -z ${find_cgroups_self_cgrp} ] + then + test_cgroups="/" + else + test_cgroups="/,${find_cgroups_self_cgrp}" + fi + log_verbose "Found cgroups: ${test_cgroups}" +} + +# Check if metric is reported for each cgroup +# $1: extra options (e.g. --bpf-counters) +check_metric_reported() +{ + local opts="$1" + local output + + log_verbose "Running check_metric_reported with opts '${opts}'..." + # Run perf stat + if ! output=$(perf stat -a ${opts} \ + --metrics=insn_per_cycle \ + --for-each-cgroup ${test_cgroups} \ + -x, sleep 0.1 2>&1) + then + echo "FAIL: perf stat failed with exit code $?" + echo "Output: ${output}" + exit 1 + fi + + log_verbose "perf stat output:" + log_verbose "${output}" + + # Split test_cgroups by comma + IFS=',' read -r -a cgrps <<< "${test_cgroups}" + + for cgrp in "${cgrps[@]}"; do + # Find lines for this cgroup and metric + local cgrp_lines + cgrp_lines=$(echo "${output}" | grep "${cgrp}" | grep "insn_per_cycle" || true) + + if [ -z "${cgrp_lines}" ] + then + echo "FAIL: No metric lines found for cgroup '${cgrp}'" + exit 1 + fi + + # Check for nan or nested + if echo "${cgrp_lines}" | grep -q -i -E ",nan,|,nested," + then + echo "FAIL: Invalid metric value (nan/nested) for cgroup '${cgrp}'" + echo "Line: ${cgrp_lines}" + exit 1 + fi + # Check for empty metric value + if echo "${cgrp_lines}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" + then + echo "FAIL: Empty metric value for cgroup '${cgrp}'" + echo "Line: ${cgrp_lines}" + exit 1 + fi + done + log_verbose "check_metric_reported passed for opts '${opts}'" +} + +check_system_wide +find_cgroups + +# Test 1: Without BPF counters +check_metric_reported "" + +# Test 2: With BPF counters (if supported) +log_verbose "Checking BPF support..." +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 +then + log_verbose "BPF supported, running Test 2..." + check_metric_reported "--bpf-counters" +else + log_verbose "BPF not supported, skipping Test 2" +fi + +exit 0 -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] Fix cgroup metric association with BPF counters 2026-05-18 22:22 [PATCH v1 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-18 22:22 ` [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-18 22:22 ` [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers @ 2026-05-19 1:51 ` Ian Rogers 2026-05-19 1:51 ` [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers ` (2 more replies) 2 siblings, 3 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 1:51 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev This series fixes an issue where cgroup metrics were not being correctly associated and reported (showing `nan %`) when using BPF counters (i.e., with `--bpf-counters`). The root cause is that cgroup BPF counters only open the "leader" events (for the first cgroup) and leave "follower" events unopened. Unopened events have their `supported` flag set to `false` by default. During metric calculation, `prepare_metric` checks `evsel->supported` and discards the value (setting it to `NAN`) if it is `false`, leading to `nan %` in the output. The first patch fixes this by propagating the `supported` flag from the leader events to the follower events in `bperf_load_program`. The second patch adds a new shell test (`stat_metrics_cgrp.sh`) to permanently cover this scenario (testing both with and without BPF counters) and prevent regressions. Reported-by: Svilen Kanev <skanev@google.com> Changes since v1: - Fix commit message for patch 2 to remove the mention of insn_per_cycle check (which was simplified out). - Quote variables in shell script to prevent word splitting. - Use grep -F in shell script for literal matching of cgroup names containing dots. Ian Rogers (2): perf stat: Propagate supported flag to follower cgroup BPF events perf test: Add stat metrics --for-each-cgroup test tools/perf/tests/shell/stat_metrics_cgrp.sh | 123 ++++++++++++++++++++ tools/perf/util/bpf_counter_cgroup.c | 15 +++ 2 files changed, 138 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers @ 2026-05-19 1:51 ` Ian Rogers 2026-05-19 1:51 ` [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 1:51 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev When using BPF counters with cgroups, follower events (for cgroups other than the first one) are not opened. Because they are not opened, their `supported` flag was left as `false`. During metric calculation, `prepare_metric` checks if the event is supported. If it is not supported (like the follower events), it explicitly sets the value to `NAN`, which eventually causes the metric to be reported as `nan %`. Fix this by propagating the `supported` flag from the "leader" events (the ones opened for the first cgroup) to the "follower" events. Reported-by: Svilen Kanev <skanev@google.com> Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter_cgroup.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index 519fee3dc3d0..dd1851634087 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); + if (leader) + evsel->supported = leader->supported; + } + } + /* * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check * whether the kernel support it -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 1:51 ` [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers @ 2026-05-19 1:51 ` Ian Rogers 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 1:51 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting with `--for-each-cgroup`, both with and without `--bpf-counters`. The test: - Checks if system-wide monitoring is supported (skips if not). - Finds cgroups to test. - Runs `perf stat` with `insn_per_cycle` metric and verifies that the metric is reported for each cgroup and does not contain invalid values (like `nan` or `nested`) or empty values when counts are present. - Tests both standard mode and BPF counters mode (if supported). Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/tests/shell/stat_metrics_cgrp.sh | 123 ++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh new file mode 100755 index 000000000000..2f74bd9e3f2e --- /dev/null +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh @@ -0,0 +1,123 @@ +#!/bin/bash +# perf stat metrics --for-each-cgroup test +# SPDX-License-Identifier: GPL-2.0 + +set -e + +test_cgroups= + +log_verbose() { + echo "$1" +} + +# skip if system-wide is not supported +check_system_wide() +{ + log_verbose "Checking system-wide..." + if ! perf stat -a -e instructions sleep 0.01 > /dev/null 2>&1 + then + log_verbose "Skipping: system-wide monitoring not supported" + exit 2 + fi +} + +# find two cgroups to measure +find_cgroups() +{ + log_verbose "Finding cgroups..." + # try usual systemd slices first + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] + then + test_cgroups="system.slice,user.slice" + log_verbose "Found cgroups: ${test_cgroups}" + return + fi + + # try root and self cgroups + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) + if [ -z "${find_cgroups_self_cgrp}" ] + then + # cgroup v2 doesn't specify perf_event + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) + fi + + if [ -z "${find_cgroups_self_cgrp}" ] + then + test_cgroups="/" + else + test_cgroups="/,${find_cgroups_self_cgrp}" + fi + log_verbose "Found cgroups: ${test_cgroups}" +} + +# Check if metric is reported for each cgroup +# $1: extra options (e.g. --bpf-counters) +check_metric_reported() +{ + local opts="$1" + local output + + log_verbose "Running check_metric_reported with opts '${opts}'..." + # Run perf stat + if ! output=$(perf stat -a ${opts} \ + --metrics=insn_per_cycle \ + --for-each-cgroup "${test_cgroups}" \ + -x, sleep 0.1 2>&1) + then + echo "FAIL: perf stat failed with exit code $?" + echo "Output: ${output}" + exit 1 + fi + + log_verbose "perf stat output:" + log_verbose "${output}" + + # Split test_cgroups by comma + IFS=',' read -r -a cgrps <<< "${test_cgroups}" + + for cgrp in "${cgrps[@]}"; do + # Find lines for this cgroup and metric + local cgrp_lines + cgrp_lines=$(echo "${output}" | grep -F "${cgrp}" | grep "insn_per_cycle" || true) + + if [ -z "${cgrp_lines}" ] + then + echo "FAIL: No metric lines found for cgroup '${cgrp}'" + exit 1 + fi + + # Check for nan or nested + if echo "${cgrp_lines}" | grep -q -i -E ",nan,|,nested," + then + echo "FAIL: Invalid metric value (nan/nested) for cgroup '${cgrp}'" + echo "Line: ${cgrp_lines}" + exit 1 + fi + # Check for empty metric value + if echo "${cgrp_lines}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" + then + echo "FAIL: Empty metric value for cgroup '${cgrp}'" + echo "Line: ${cgrp_lines}" + exit 1 + fi + done + log_verbose "check_metric_reported passed for opts '${opts}'" +} + +check_system_wide +find_cgroups + +# Test 1: Without BPF counters +check_metric_reported "" + +# Test 2: With BPF counters (if supported) +log_verbose "Checking BPF support..." +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 +then + log_verbose "BPF supported, running Test 2..." + check_metric_reported "--bpf-counters" +else + log_verbose "BPF not supported, skipping Test 2" +fi + +exit 0 -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 0/2] Fix cgroup metric association with BPF counters 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 1:51 ` [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 1:51 ` [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers @ 2026-05-19 5:01 ` Ian Rogers 2026-05-19 5:01 ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers ` (2 more replies) 2 siblings, 3 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 5:01 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev This series fixes an issue where cgroup metrics were not being correctly associated and reported (showing `nan %`) when using BPF counters (i.e., with `--bpf-counters`). The root cause is that cgroup BPF counters only open the "leader" events (for the first cgroup) and leave "follower" events unopened. Unopened events have their `supported` flag set to `false` by default. During metric calculation, `prepare_metric` checks `evsel->supported` and discards the value (setting it to `NAN`) if it is `false`, leading to `nan %` in the output. The first patch fixes this by propagating the `supported` flag from the leader events to the follower events in `bperf_load_program`. The second patch adds a new shell test (`stat_metrics_cgrp.sh`) to permanently cover this scenario (testing both with and without BPF counters) and prevent regressions. Reported-by: Svilen Kanev <skanev@google.com> Changes since v2: - Use `--metrics=insn_per_cycle` for the system-wide capability check in `check_system_wide` to ensure both cycles and instructions are supported. - Use exact cgroup field matching with surrounding commas (`,${cgrp},`) in the `grep` check to prevent false positives when matching the root cgroup (`/`). - Only enforce metric validation if both `cycles` and `instructions` counts are numeric and non-zero, preventing false failures on idle cgroups. - Wrap long lines and fix trailing whitespace in the test script to comply with style guidelines. Changes since v1: - Fix commit message for patch 2 to remove the mention of insn_per_cycle check (which was simplified out). - Quote variables in shell script to prevent word splitting. - Use grep -F in shell script for literal matching of cgroup names containing dots. Ian Rogers (2): perf stat: Propagate supported flag to follower cgroup BPF events perf test: Add stat metrics --for-each-cgroup test tools/perf/tests/shell/stat_metrics_cgrp.sh | 174 ++++++++++++++++++++ tools/perf/util/bpf_counter_cgroup.c | 15 ++ 2 files changed, 189 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers @ 2026-05-19 5:01 ` Ian Rogers 2026-05-19 5:47 ` Namhyung Kim 2026-05-19 5:01 ` [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 15:27 ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 1 reply; 18+ messages in thread From: Ian Rogers @ 2026-05-19 5:01 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev When using BPF counters with cgroups, follower events (for cgroups other than the first one) are not opened. Because they are not opened, their `supported` flag was left as `false`. During metric calculation, `prepare_metric` checks if the event is supported. If it is not supported (like the follower events), it explicitly sets the value to `NAN`, which eventually causes the metric to be reported as `nan %`. Fix this by propagating the `supported` flag from the "leader" events (the ones opened for the first cgroup) to the "follower" events. Reported-by: Svilen Kanev <skanev@google.com> Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter_cgroup.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index 519fee3dc3d0..dd1851634087 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); + if (leader) + evsel->supported = leader->supported; + } + } + /* * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check * whether the kernel support it -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events 2026-05-19 5:01 ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers @ 2026-05-19 5:47 ` Namhyung Kim 0 siblings, 0 replies; 18+ messages in thread From: Namhyung Kim @ 2026-05-19 5:47 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev Hi Ian, On Mon, May 18, 2026 at 10:01:49PM -0700, Ian Rogers wrote: > When using BPF counters with cgroups, follower events (for cgroups > other than the first one) are not opened. Because they are not opened, > their `supported` flag was left as `false`. > > During metric calculation, `prepare_metric` checks if the event is > supported. If it is not supported (like the follower events), it > explicitly sets the value to `NAN`, which eventually causes the metric > to be reported as `nan %`. > > Fix this by propagating the `supported` flag from the "leader" events > (the ones opened for the first cgroup) to the "follower" events. > > Reported-by: Svilen Kanev <skanev@google.com> > Assisted-by: Antigravity:gemini-3-flash > Signed-off-by: Ian Rogers <irogers@google.com> Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks, Namhyung > --- > tools/perf/util/bpf_counter_cgroup.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c > index 519fee3dc3d0..dd1851634087 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); > + if (leader) > + evsel->supported = leader->supported; > + } > + } > + > /* > * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check > * whether the kernel support it > -- > 2.54.0.631.ge1b05301d1-goog > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 5:01 ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers @ 2026-05-19 5:01 ` Ian Rogers 2026-05-19 5:54 ` Namhyung Kim 2026-05-19 15:27 ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2 siblings, 1 reply; 18+ messages in thread From: Ian Rogers @ 2026-05-19 5:01 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting with `--for-each-cgroup`, both with and without `--bpf-counters`. The test: - Checks if system-wide monitoring is supported (skips if not). - Finds cgroups to test. - Runs `perf stat` with `insn_per_cycle` metric and verifies that the metric is reported for each cgroup and does not contain invalid values (like `nan` or `nested`) or empty values when counts are present. - Tests both standard mode and BPF counters mode (if supported). Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/tests/shell/stat_metrics_cgrp.sh | 174 ++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh new file mode 100755 index 000000000000..b413849193a0 --- /dev/null +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh @@ -0,0 +1,174 @@ +#!/bin/bash +# perf stat metrics --for-each-cgroup test +# SPDX-License-Identifier: GPL-2.0 + +set -e + +test_cgroups= + +log_verbose() { + echo "$1" +} + +is_numeric_and_non_zero() { + local val="$1" + if [[ "${val}" =~ ^[0-9]+$ ]] && [ "${val}" -gt 0 ] + then + return 0 # True + fi + return 1 # False +} + +# skip if system-wide is not supported +check_system_wide() +{ + log_verbose "Checking system-wide..." + if ! perf stat -a --metrics=insn_per_cycle sleep 0.01 > /dev/null 2>&1 + then + log_verbose "Skipping: system-wide monitoring not supported" + exit 2 + fi +} + +# find two cgroups to measure +find_cgroups() +{ + log_verbose "Finding cgroups..." + # try usual systemd slices first + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] + then + test_cgroups="system.slice,user.slice" + log_verbose "Found cgroups: ${test_cgroups}" + return + fi + + # try root and self cgroups + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) + if [ -z "${find_cgroups_self_cgrp}" ] + then + # cgroup v2 doesn't specify perf_event + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) + fi + + if [ -z "${find_cgroups_self_cgrp}" ] + then + test_cgroups="/" + else + test_cgroups="/,${find_cgroups_self_cgrp}" + fi + log_verbose "Found cgroups: ${test_cgroups}" +} + +# Check if metric is reported for each cgroup +# $1: extra options (e.g. --bpf-counters) +check_metric_reported() +{ + local opts="$1" + local output + + log_verbose "Running check_metric_reported with opts '${opts}'..." + # Run perf stat + if ! output=$(perf stat -a ${opts} \ + --metrics=insn_per_cycle \ + --for-each-cgroup "${test_cgroups}" \ + -x, sleep 0.1 2>&1) + then + echo "FAIL: perf stat failed with exit code $?" + echo "Output: ${output}" + exit 1 + fi + + log_verbose "perf stat output:" + log_verbose "${output}" + + # Split test_cgroups by comma + IFS=',' read -r -a cgrps <<< "${test_cgroups}" + + for cgrp in "${cgrps[@]}"; do + # Find metric lines for this cgroup + # We use exact cgroup match with surrounding commas + local cgrp_lines + cgrp_lines=$(echo "${output}" | grep -F ",${cgrp}," | grep "insn_per_cycle" || true) + + if [ -z "${cgrp_lines}" ] + then + echo "FAIL: No metric lines found for cgroup '${cgrp}'" + exit 1 + fi + + # Parse each metric line + while read -r line; do + [ -z "${line}" ] && continue + + local cycles_val + cycles_val=$(echo "${line}" | cut -d, -f1) + + local event_name + event_name=$(echo "${line}" | cut -d, -f3) + + # Find corresponding instructions event line + local inst_event_name + inst_event_name="${event_name/cpu-cycles/instructions}" + inst_event_name="${inst_event_name/cycles/instructions}" + + local inst_line + inst_line=$(echo "${output}" | \ + grep -F ",${cgrp}," | \ + grep -F "${inst_event_name}" || true) + + local inst_val + inst_val=$(echo "${inst_line}" | cut -d, -f1) + + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" + + # Only enforce metric check if both cycles and + # instructions have non-zero numeric counts + if is_numeric_and_non_zero "${cycles_val}" && \ + is_numeric_and_non_zero "${inst_val}" + then + log_verbose "Enforcing metric check for cgroup '${cgrp}' \ +event '${event_name}'" + # Check for nan or nested in the metric value (7th field) + # Actually we can just check the whole line for simplicity + if echo "${line}" | grep -q -i -E ",nan,|,nested," + then + echo "FAIL: Invalid metric value (nan/nested) \ +for cgroup '${cgrp}'" + echo "Line: ${line}" + exit 1 + fi + # Check for empty metric value (2 consecutive + # commas before the unit) + if echo "${line}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" + then + echo "FAIL: Empty metric value for cgroup '${cgrp}'" + echo "Line: ${line}" + exit 1 + fi + else + log_verbose "Skipping metric check for cgroup '${cgrp}' \ +event '${event_name}' (idle or not counted)" + fi + done <<< "${cgrp_lines}" + done + log_verbose "check_metric_reported passed for opts '${opts}'" +} + +check_system_wide +find_cgroups + +# Test 1: Without BPF counters +check_metric_reported "" + +# Test 2: With BPF counters (if supported) +log_verbose "Checking BPF support..." +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 +then + log_verbose "BPF supported, running Test 2..." + check_metric_reported "--bpf-counters" +else + log_verbose "BPF not supported, skipping Test 2" +fi + +exit 0 -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 5:01 ` [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers @ 2026-05-19 5:54 ` Namhyung Kim 2026-05-19 15:13 ` Ian Rogers 0 siblings, 1 reply; 18+ messages in thread From: Namhyung Kim @ 2026-05-19 5:54 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote: > Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting > with `--for-each-cgroup`, both with and without `--bpf-counters`. > > The test: > - Checks if system-wide monitoring is supported (skips if not). > - Finds cgroups to test. > - Runs `perf stat` with `insn_per_cycle` metric and verifies that the > metric is reported for each cgroup and does not contain invalid > values (like `nan` or `nested`) or empty values when counts are > present. > - Tests both standard mode and BPF counters mode (if supported). > > Assisted-by: Antigravity:gemini-3-flash > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/tests/shell/stat_metrics_cgrp.sh | 174 ++++++++++++++++++++ > 1 file changed, 174 insertions(+) > create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh > > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh > new file mode 100755 > index 000000000000..b413849193a0 > --- /dev/null > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh > @@ -0,0 +1,174 @@ > +#!/bin/bash > +# perf stat metrics --for-each-cgroup test > +# SPDX-License-Identifier: GPL-2.0 > + > +set -e > + > +test_cgroups= > + > +log_verbose() { > + echo "$1" > +} Why is this function used? > + > +is_numeric_and_non_zero() { > + local val="$1" > + if [[ "${val}" =~ ^[0-9]+$ ]] && [ "${val}" -gt 0 ] > + then > + return 0 # True > + fi > + return 1 # False > +} > + > +# skip if system-wide is not supported > +check_system_wide() > +{ > + log_verbose "Checking system-wide..." > + if ! perf stat -a --metrics=insn_per_cycle sleep 0.01 > /dev/null 2>&1 > + then > + log_verbose "Skipping: system-wide monitoring not supported" > + exit 2 > + fi > +} > + > +# find two cgroups to measure > +find_cgroups() > +{ > + log_verbose "Finding cgroups..." > + # try usual systemd slices first > + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] > + then > + test_cgroups="system.slice,user.slice" > + log_verbose "Found cgroups: ${test_cgroups}" > + return > + fi > + > + # try root and self cgroups > + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) > + if [ -z "${find_cgroups_self_cgrp}" ] > + then > + # cgroup v2 doesn't specify perf_event > + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) > + fi > + > + if [ -z "${find_cgroups_self_cgrp}" ] > + then > + test_cgroups="/" > + else > + test_cgroups="/,${find_cgroups_self_cgrp}" > + fi > + log_verbose "Found cgroups: ${test_cgroups}" > +} > + > +# Check if metric is reported for each cgroup > +# $1: extra options (e.g. --bpf-counters) > +check_metric_reported() > +{ > + local opts="$1" > + local output > + > + log_verbose "Running check_metric_reported with opts '${opts}'..." > + # Run perf stat > + if ! output=$(perf stat -a ${opts} \ > + --metrics=insn_per_cycle \ > + --for-each-cgroup "${test_cgroups}" \ > + -x, sleep 0.1 2>&1) > + then > + echo "FAIL: perf stat failed with exit code $?" > + echo "Output: ${output}" > + exit 1 > + fi > + > + log_verbose "perf stat output:" > + log_verbose "${output}" > + > + # Split test_cgroups by comma > + IFS=',' read -r -a cgrps <<< "${test_cgroups}" > + > + for cgrp in "${cgrps[@]}"; do > + # Find metric lines for this cgroup > + # We use exact cgroup match with surrounding commas > + local cgrp_lines > + cgrp_lines=$(echo "${output}" | grep -F ",${cgrp}," | grep "insn_per_cycle" || true) > + > + if [ -z "${cgrp_lines}" ] > + then > + echo "FAIL: No metric lines found for cgroup '${cgrp}'" > + exit 1 > + fi > + > + # Parse each metric line > + while read -r line; do > + [ -z "${line}" ] && continue > + > + local cycles_val > + cycles_val=$(echo "${line}" | cut -d, -f1) > + > + local event_name > + event_name=$(echo "${line}" | cut -d, -f3) > + > + # Find corresponding instructions event line > + local inst_event_name > + inst_event_name="${event_name/cpu-cycles/instructions}" > + inst_event_name="${inst_event_name/cycles/instructions}" > + > + local inst_line > + inst_line=$(echo "${output}" | \ > + grep -F ",${cgrp}," | \ > + grep -F "${inst_event_name}" || true) > + > + local inst_val > + inst_val=$(echo "${inst_line}" | cut -d, -f1) > + > + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" Why not make log_verbose() take multiple arguments and align them properly? Maybe you can just use 'echo'? The same goes to the below lines. Thanks, Namhyung > + > + # Only enforce metric check if both cycles and > + # instructions have non-zero numeric counts > + if is_numeric_and_non_zero "${cycles_val}" && \ > + is_numeric_and_non_zero "${inst_val}" > + then > + log_verbose "Enforcing metric check for cgroup '${cgrp}' \ > +event '${event_name}'" > + # Check for nan or nested in the metric value (7th field) > + # Actually we can just check the whole line for simplicity > + if echo "${line}" | grep -q -i -E ",nan,|,nested," > + then > + echo "FAIL: Invalid metric value (nan/nested) \ > +for cgroup '${cgrp}'" > + echo "Line: ${line}" > + exit 1 > + fi > + # Check for empty metric value (2 consecutive > + # commas before the unit) > + if echo "${line}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" > + then > + echo "FAIL: Empty metric value for cgroup '${cgrp}'" > + echo "Line: ${line}" > + exit 1 > + fi > + else > + log_verbose "Skipping metric check for cgroup '${cgrp}' \ > +event '${event_name}' (idle or not counted)" > + fi > + done <<< "${cgrp_lines}" > + done > + log_verbose "check_metric_reported passed for opts '${opts}'" > +} > + > +check_system_wide > +find_cgroups > + > +# Test 1: Without BPF counters > +check_metric_reported "" > + > +# Test 2: With BPF counters (if supported) > +log_verbose "Checking BPF support..." > +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 > +then > + log_verbose "BPF supported, running Test 2..." > + check_metric_reported "--bpf-counters" > +else > + log_verbose "BPF not supported, skipping Test 2" > +fi > + > +exit 0 > -- > 2.54.0.631.ge1b05301d1-goog > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 5:54 ` Namhyung Kim @ 2026-05-19 15:13 ` Ian Rogers 2026-05-19 17:09 ` Namhyung Kim 0 siblings, 1 reply; 18+ messages in thread From: Ian Rogers @ 2026-05-19 15:13 UTC (permalink / raw) To: Namhyung Kim Cc: acme, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev On Mon, May 18, 2026 at 10:54 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote: > > Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting > > with `--for-each-cgroup`, both with and without `--bpf-counters`. > > > > The test: > > - Checks if system-wide monitoring is supported (skips if not). > > - Finds cgroups to test. > > - Runs `perf stat` with `insn_per_cycle` metric and verifies that the > > metric is reported for each cgroup and does not contain invalid > > values (like `nan` or `nested`) or empty values when counts are > > present. > > - Tests both standard mode and BPF counters mode (if supported). > > > > Assisted-by: Antigravity:gemini-3-flash > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/tests/shell/stat_metrics_cgrp.sh | 174 ++++++++++++++++++++ > > 1 file changed, 174 insertions(+) > > create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh > > > > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > new file mode 100755 > > index 000000000000..b413849193a0 > > --- /dev/null > > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > @@ -0,0 +1,174 @@ > > +#!/bin/bash > > +# perf stat metrics --for-each-cgroup test > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +set -e > > + > > +test_cgroups= > > + > > +log_verbose() { > > + echo "$1" > > +} > > Why is this function used? At some point I'd like verbose flags to be passed to tests to cut down on output. A suitable series would be the test improvements in: https://lore.kernel.org/linux-perf-users/20260513230450.529380-1-irogers@google.com/ > > + > > +is_numeric_and_non_zero() { > > + local val="$1" > > + if [[ "${val}" =~ ^[0-9]+$ ]] && [ "${val}" -gt 0 ] > > + then > > + return 0 # True > > + fi > > + return 1 # False > > +} > > + > > +# skip if system-wide is not supported > > +check_system_wide() > > +{ > > + log_verbose "Checking system-wide..." > > + if ! perf stat -a --metrics=insn_per_cycle sleep 0.01 > /dev/null 2>&1 > > + then > > + log_verbose "Skipping: system-wide monitoring not supported" > > + exit 2 > > + fi > > +} > > + > > +# find two cgroups to measure > > +find_cgroups() > > +{ > > + log_verbose "Finding cgroups..." > > + # try usual systemd slices first > > + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] > > + then > > + test_cgroups="system.slice,user.slice" > > + log_verbose "Found cgroups: ${test_cgroups}" > > + return > > + fi > > + > > + # try root and self cgroups > > + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) > > + if [ -z "${find_cgroups_self_cgrp}" ] > > + then > > + # cgroup v2 doesn't specify perf_event > > + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) > > + fi > > + > > + if [ -z "${find_cgroups_self_cgrp}" ] > > + then > > + test_cgroups="/" > > + else > > + test_cgroups="/,${find_cgroups_self_cgrp}" > > + fi > > + log_verbose "Found cgroups: ${test_cgroups}" > > +} > > + > > +# Check if metric is reported for each cgroup > > +# $1: extra options (e.g. --bpf-counters) > > +check_metric_reported() > > +{ > > + local opts="$1" > > + local output > > + > > + log_verbose "Running check_metric_reported with opts '${opts}'..." > > + # Run perf stat > > + if ! output=$(perf stat -a ${opts} \ > > + --metrics=insn_per_cycle \ > > + --for-each-cgroup "${test_cgroups}" \ > > + -x, sleep 0.1 2>&1) > > + then > > + echo "FAIL: perf stat failed with exit code $?" > > + echo "Output: ${output}" > > + exit 1 > > + fi > > + > > + log_verbose "perf stat output:" > > + log_verbose "${output}" > > + > > + # Split test_cgroups by comma > > + IFS=',' read -r -a cgrps <<< "${test_cgroups}" > > + > > + for cgrp in "${cgrps[@]}"; do > > + # Find metric lines for this cgroup > > + # We use exact cgroup match with surrounding commas > > + local cgrp_lines > > + cgrp_lines=$(echo "${output}" | grep -F ",${cgrp}," | grep "insn_per_cycle" || true) > > + > > + if [ -z "${cgrp_lines}" ] > > + then > > + echo "FAIL: No metric lines found for cgroup '${cgrp}'" > > + exit 1 > > + fi > > + > > + # Parse each metric line > > + while read -r line; do > > + [ -z "${line}" ] && continue > > + > > + local cycles_val > > + cycles_val=$(echo "${line}" | cut -d, -f1) > > + > > + local event_name > > + event_name=$(echo "${line}" | cut -d, -f3) > > + > > + # Find corresponding instructions event line > > + local inst_event_name > > + inst_event_name="${event_name/cpu-cycles/instructions}" > > + inst_event_name="${inst_event_name/cycles/instructions}" > > + > > + local inst_line > > + inst_line=$(echo "${output}" | \ > > + grep -F ",${cgrp}," | \ > > + grep -F "${inst_event_name}" || true) > > + > > + local inst_val > > + inst_val=$(echo "${inst_line}" | cut -d, -f1) > > + > > + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" > > Why not make log_verbose() take multiple arguments and align them > properly? Maybe you can just use 'echo'? The same goes to the below > lines. This fixes checkpatch warnings on line length and the odd alignment avoids spaces appearing in the output. Thanks, Ian > Thanks, > Namhyung > > > + > > + # Only enforce metric check if both cycles and > > + # instructions have non-zero numeric counts > > + if is_numeric_and_non_zero "${cycles_val}" && \ > > + is_numeric_and_non_zero "${inst_val}" > > + then > > + log_verbose "Enforcing metric check for cgroup '${cgrp}' \ > > +event '${event_name}'" > > + # Check for nan or nested in the metric value (7th field) > > + # Actually we can just check the whole line for simplicity > > + if echo "${line}" | grep -q -i -E ",nan,|,nested," > > + then > > + echo "FAIL: Invalid metric value (nan/nested) \ > > +for cgroup '${cgrp}'" > > + echo "Line: ${line}" > > + exit 1 > > + fi > > + # Check for empty metric value (2 consecutive > > + # commas before the unit) > > + if echo "${line}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" > > + then > > + echo "FAIL: Empty metric value for cgroup '${cgrp}'" > > + echo "Line: ${line}" > > + exit 1 > > + fi > > + else > > + log_verbose "Skipping metric check for cgroup '${cgrp}' \ > > +event '${event_name}' (idle or not counted)" > > + fi > > + done <<< "${cgrp_lines}" > > + done > > + log_verbose "check_metric_reported passed for opts '${opts}'" > > +} > > + > > +check_system_wide > > +find_cgroups > > + > > +# Test 1: Without BPF counters > > +check_metric_reported "" > > + > > +# Test 2: With BPF counters (if supported) > > +log_verbose "Checking BPF support..." > > +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 > > +then > > + log_verbose "BPF supported, running Test 2..." > > + check_metric_reported "--bpf-counters" > > +else > > + log_verbose "BPF not supported, skipping Test 2" > > +fi > > + > > +exit 0 > > -- > > 2.54.0.631.ge1b05301d1-goog > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 15:13 ` Ian Rogers @ 2026-05-19 17:09 ` Namhyung Kim 2026-05-25 19:49 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 18+ messages in thread From: Namhyung Kim @ 2026-05-19 17:09 UTC (permalink / raw) To: Ian Rogers Cc: acme, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev On Tue, May 19, 2026 at 08:13:02AM -0700, Ian Rogers wrote: > On Mon, May 18, 2026 at 10:54 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote: > > > Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting > > > with `--for-each-cgroup`, both with and without `--bpf-counters`. > > > > > > The test: > > > - Checks if system-wide monitoring is supported (skips if not). > > > - Finds cgroups to test. > > > - Runs `perf stat` with `insn_per_cycle` metric and verifies that the > > > metric is reported for each cgroup and does not contain invalid > > > values (like `nan` or `nested`) or empty values when counts are > > > present. > > > - Tests both standard mode and BPF counters mode (if supported). > > > > > > Assisted-by: Antigravity:gemini-3-flash > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > --- > > > tools/perf/tests/shell/stat_metrics_cgrp.sh | 174 ++++++++++++++++++++ > > > 1 file changed, 174 insertions(+) > > > create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh > > > > > > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > > new file mode 100755 > > > index 000000000000..b413849193a0 > > > --- /dev/null > > > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > > @@ -0,0 +1,174 @@ > > > +#!/bin/bash > > > +# perf stat metrics --for-each-cgroup test > > > +# SPDX-License-Identifier: GPL-2.0 > > > + > > > +set -e > > > + > > > +test_cgroups= > > > + > > > +log_verbose() { > > > + echo "$1" > > > +} > > > > Why is this function used? > > At some point I'd like verbose flags to be passed to tests to cut down > on output. A suitable series would be the test improvements in: > https://lore.kernel.org/linux-perf-users/20260513230450.529380-1-irogers@google.com/ I see. > > > > + > > > +is_numeric_and_non_zero() { > > > + local val="$1" > > > + if [[ "${val}" =~ ^[0-9]+$ ]] && [ "${val}" -gt 0 ] > > > + then > > > + return 0 # True > > > + fi > > > + return 1 # False > > > +} > > > + > > > +# skip if system-wide is not supported > > > +check_system_wide() > > > +{ > > > + log_verbose "Checking system-wide..." > > > + if ! perf stat -a --metrics=insn_per_cycle sleep 0.01 > /dev/null 2>&1 > > > + then > > > + log_verbose "Skipping: system-wide monitoring not supported" > > > + exit 2 > > > + fi > > > +} > > > + > > > +# find two cgroups to measure > > > +find_cgroups() > > > +{ > > > + log_verbose "Finding cgroups..." > > > + # try usual systemd slices first > > > + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] > > > + then > > > + test_cgroups="system.slice,user.slice" > > > + log_verbose "Found cgroups: ${test_cgroups}" > > > + return > > > + fi > > > + > > > + # try root and self cgroups > > > + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) > > > + if [ -z "${find_cgroups_self_cgrp}" ] > > > + then > > > + # cgroup v2 doesn't specify perf_event > > > + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) > > > + fi > > > + > > > + if [ -z "${find_cgroups_self_cgrp}" ] > > > + then > > > + test_cgroups="/" > > > + else > > > + test_cgroups="/,${find_cgroups_self_cgrp}" > > > + fi > > > + log_verbose "Found cgroups: ${test_cgroups}" > > > +} > > > + > > > +# Check if metric is reported for each cgroup > > > +# $1: extra options (e.g. --bpf-counters) > > > +check_metric_reported() > > > +{ > > > + local opts="$1" > > > + local output > > > + > > > + log_verbose "Running check_metric_reported with opts '${opts}'..." > > > + # Run perf stat > > > + if ! output=$(perf stat -a ${opts} \ > > > + --metrics=insn_per_cycle \ > > > + --for-each-cgroup "${test_cgroups}" \ > > > + -x, sleep 0.1 2>&1) > > > + then > > > + echo "FAIL: perf stat failed with exit code $?" > > > + echo "Output: ${output}" > > > + exit 1 > > > + fi > > > + > > > + log_verbose "perf stat output:" > > > + log_verbose "${output}" > > > + > > > + # Split test_cgroups by comma > > > + IFS=',' read -r -a cgrps <<< "${test_cgroups}" > > > + > > > + for cgrp in "${cgrps[@]}"; do > > > + # Find metric lines for this cgroup > > > + # We use exact cgroup match with surrounding commas > > > + local cgrp_lines > > > + cgrp_lines=$(echo "${output}" | grep -F ",${cgrp}," | grep "insn_per_cycle" || true) > > > + > > > + if [ -z "${cgrp_lines}" ] > > > + then > > > + echo "FAIL: No metric lines found for cgroup '${cgrp}'" > > > + exit 1 > > > + fi > > > + > > > + # Parse each metric line > > > + while read -r line; do > > > + [ -z "${line}" ] && continue > > > + > > > + local cycles_val > > > + cycles_val=$(echo "${line}" | cut -d, -f1) > > > + > > > + local event_name > > > + event_name=$(echo "${line}" | cut -d, -f3) > > > + > > > + # Find corresponding instructions event line > > > + local inst_event_name > > > + inst_event_name="${event_name/cpu-cycles/instructions}" > > > + inst_event_name="${inst_event_name/cycles/instructions}" > > > + > > > + local inst_line > > > + inst_line=$(echo "${output}" | \ > > > + grep -F ",${cgrp}," | \ > > > + grep -F "${inst_event_name}" || true) > > > + > > > + local inst_val > > > + inst_val=$(echo "${inst_line}" | cut -d, -f1) > > > + > > > + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > > > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" > > > > Why not make log_verbose() take multiple arguments and align them > > properly? Maybe you can just use 'echo'? The same goes to the below > > lines. > > This fixes checkpatch warnings on line length and the odd alignment > avoids spaces appearing in the output. How about this? Thanks, Namhyung diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh index d4226ee0ae9801cb..ecb2c053ea5158d4 100755 --- a/tools/perf/tests/shell/stat_metrics_cgrp.sh +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh @@ -7,7 +7,7 @@ set -e test_cgroups= log_verbose() { - echo "$1" + echo "$*" } is_numeric_and_non_zero() { @@ -145,22 +145,22 @@ check_metric_reported() cycles_val=$(echo "${cycles_line}" | cut -d, -f1) fi - log_verbose "Cgroup '${cgrp}': event '${event_name}' \ -val '${cycles_val}', inst val '${inst_val}'" + log_verbose "Cgroup '${cgrp}': event '${event_name}'" \ + "val '${cycles_val}', inst val '${inst_val}'" # Only enforce metric check if both cycles and # instructions have non-zero numeric counts if is_numeric_and_non_zero "${cycles_val}" && \ is_numeric_and_non_zero "${inst_val}" then - log_verbose "Enforcing metric check for cgroup '${cgrp}' \ -event '${event_name}'" + log_verbose "Enforcing metric check for cgroup '${cgrp}'" \ + "event '${event_name}'" # Check for nan or nested in the metric value (7th field) # Actually we can just check the whole line for simplicity if echo "${line}" | grep -q -i -E ",nan,|,nested," then - echo "FAIL: Invalid metric value (nan/nested) \ -for cgroup '${cgrp}'" + echo "FAIL: Invalid metric value (nan/nested)" \ + "for cgroup '${cgrp}'" echo "Line: ${line}" exit 1 fi @@ -173,8 +173,8 @@ for cgroup '${cgrp}'" exit 1 fi else - log_verbose "Skipping metric check for cgroup '${cgrp}' \ -event '${event_name}' (idle or not counted)" + log_verbose "Skipping metric check for cgroup '${cgrp}'" \ + "event '${event_name}' (idle or not counted)" fi done <<< "${cgrp_lines}" done ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 17:09 ` Namhyung Kim @ 2026-05-25 19:49 ` Arnaldo Carvalho de Melo 2026-05-25 22:46 ` Ian Rogers 0 siblings, 1 reply; 18+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-05-25 19:49 UTC (permalink / raw) To: Namhyung Kim Cc: Ian Rogers, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev On Tue, May 19, 2026 at 10:09:57AM -0700, Namhyung Kim wrote: > On Tue, May 19, 2026 at 08:13:02AM -0700, Ian Rogers wrote: > > On Mon, May 18, 2026 at 10:54 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote: > > > > + # Parse each metric line > > > > + while read -r line; do > > > > + [ -z "${line}" ] && continue > > > > + > > > > + local cycles_val > > > > + cycles_val=$(echo "${line}" | cut -d, -f1) > > > > + > > > > + local event_name > > > > + event_name=$(echo "${line}" | cut -d, -f3) > > > > + > > > > + # Find corresponding instructions event line > > > > + local inst_event_name > > > > + inst_event_name="${event_name/cpu-cycles/instructions}" > > > > + inst_event_name="${inst_event_name/cycles/instructions}" > > > > + > > > > + local inst_line > > > > + inst_line=$(echo "${output}" | \ > > > > + grep -F ",${cgrp}," | \ > > > > + grep -F "${inst_event_name}" || true) > > > > + > > > > + local inst_val > > > > + inst_val=$(echo "${inst_line}" | cut -d, -f1) > > > > + > > > > + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > > > > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" > > > Why not make log_verbose() take multiple arguments and align them > > > properly? Maybe you can just use 'echo'? The same goes to the below > > > lines. > > This fixes checkpatch warnings on line length and the odd alignment > > avoids spaces appearing in the output. > How about this? Ian posted a v4, are you ok with that now, Tested-by or Reviewed-by? - Arnaldo > Thanks, > Namhyung > > > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh > index d4226ee0ae9801cb..ecb2c053ea5158d4 100755 > --- a/tools/perf/tests/shell/stat_metrics_cgrp.sh > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh > @@ -7,7 +7,7 @@ set -e > test_cgroups= > > log_verbose() { > - echo "$1" > + echo "$*" > } > > is_numeric_and_non_zero() { > @@ -145,22 +145,22 @@ check_metric_reported() > cycles_val=$(echo "${cycles_line}" | cut -d, -f1) > fi > > - log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > -val '${cycles_val}', inst val '${inst_val}'" > + log_verbose "Cgroup '${cgrp}': event '${event_name}'" \ > + "val '${cycles_val}', inst val '${inst_val}'" > > # Only enforce metric check if both cycles and > # instructions have non-zero numeric counts > if is_numeric_and_non_zero "${cycles_val}" && \ > is_numeric_and_non_zero "${inst_val}" > then > - log_verbose "Enforcing metric check for cgroup '${cgrp}' \ > -event '${event_name}'" > + log_verbose "Enforcing metric check for cgroup '${cgrp}'" \ > + "event '${event_name}'" > # Check for nan or nested in the metric value (7th field) > # Actually we can just check the whole line for simplicity > if echo "${line}" | grep -q -i -E ",nan,|,nested," > then > - echo "FAIL: Invalid metric value (nan/nested) \ > -for cgroup '${cgrp}'" > + echo "FAIL: Invalid metric value (nan/nested)" \ > + "for cgroup '${cgrp}'" > echo "Line: ${line}" > exit 1 > fi > @@ -173,8 +173,8 @@ for cgroup '${cgrp}'" > exit 1 > fi > else > - log_verbose "Skipping metric check for cgroup '${cgrp}' \ > -event '${event_name}' (idle or not counted)" > + log_verbose "Skipping metric check for cgroup '${cgrp}'" \ > + "event '${event_name}' (idle or not counted)" > fi > done <<< "${cgrp_lines}" > done ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-25 19:49 ` Arnaldo Carvalho de Melo @ 2026-05-25 22:46 ` Ian Rogers 0 siblings, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-25 22:46 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev On Mon, May 25, 2026 at 12:50 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > On Tue, May 19, 2026 at 10:09:57AM -0700, Namhyung Kim wrote: > > On Tue, May 19, 2026 at 08:13:02AM -0700, Ian Rogers wrote: > > > On Mon, May 18, 2026 at 10:54 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Mon, May 18, 2026 at 10:01:50PM -0700, Ian Rogers wrote: > > > > > + # Parse each metric line > > > > > + while read -r line; do > > > > > + [ -z "${line}" ] && continue > > > > > + > > > > > + local cycles_val > > > > > + cycles_val=$(echo "${line}" | cut -d, -f1) > > > > > + > > > > > + local event_name > > > > > + event_name=$(echo "${line}" | cut -d, -f3) > > > > > + > > > > > + # Find corresponding instructions event line > > > > > + local inst_event_name > > > > > + inst_event_name="${event_name/cpu-cycles/instructions}" > > > > > + inst_event_name="${inst_event_name/cycles/instructions}" > > > > > + > > > > > + local inst_line > > > > > + inst_line=$(echo "${output}" | \ > > > > > + grep -F ",${cgrp}," | \ > > > > > + grep -F "${inst_event_name}" || true) > > > > > + > > > > > + local inst_val > > > > > + inst_val=$(echo "${inst_line}" | cut -d, -f1) > > > > > + > > > > > + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > > > > > +val '${cycles_val}', inst event '${inst_event_name}' val '${inst_val}'" > > > > > Why not make log_verbose() take multiple arguments and align them > > > > properly? Maybe you can just use 'echo'? The same goes to the below > > > > lines. > > > > This fixes checkpatch warnings on line length and the odd alignment > > > avoids spaces appearing in the output. > > > How about this? > > Ian posted a v4, are you ok with that now, Tested-by or Reviewed-by? I can add the log_verbose change Namhyung asked for. Thanks, Ian > - Arnaldo > > > Thanks, > > Namhyung > > > > > > diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > index d4226ee0ae9801cb..ecb2c053ea5158d4 100755 > > --- a/tools/perf/tests/shell/stat_metrics_cgrp.sh > > +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh > > @@ -7,7 +7,7 @@ set -e > > test_cgroups= > > > > log_verbose() { > > - echo "$1" > > + echo "$*" > > } > > > > is_numeric_and_non_zero() { > > @@ -145,22 +145,22 @@ check_metric_reported() > > cycles_val=$(echo "${cycles_line}" | cut -d, -f1) > > fi > > > > - log_verbose "Cgroup '${cgrp}': event '${event_name}' \ > > -val '${cycles_val}', inst val '${inst_val}'" > > + log_verbose "Cgroup '${cgrp}': event '${event_name}'" \ > > + "val '${cycles_val}', inst val '${inst_val}'" > > > > # Only enforce metric check if both cycles and > > # instructions have non-zero numeric counts > > if is_numeric_and_non_zero "${cycles_val}" && \ > > is_numeric_and_non_zero "${inst_val}" > > then > > - log_verbose "Enforcing metric check for cgroup '${cgrp}' \ > > -event '${event_name}'" > > + log_verbose "Enforcing metric check for cgroup '${cgrp}'" \ > > + "event '${event_name}'" > > # Check for nan or nested in the metric value (7th field) > > # Actually we can just check the whole line for simplicity > > if echo "${line}" | grep -q -i -E ",nan,|,nested," > > then > > - echo "FAIL: Invalid metric value (nan/nested) \ > > -for cgroup '${cgrp}'" > > + echo "FAIL: Invalid metric value (nan/nested)" \ > > + "for cgroup '${cgrp}'" > > echo "Line: ${line}" > > exit 1 > > fi > > @@ -173,8 +173,8 @@ for cgroup '${cgrp}'" > > exit 1 > > fi > > else > > - log_verbose "Skipping metric check for cgroup '${cgrp}' \ > > -event '${event_name}' (idle or not counted)" > > + log_verbose "Skipping metric check for cgroup '${cgrp}'" \ > > + "event '${event_name}' (idle or not counted)" > > fi > > done <<< "${cgrp_lines}" > > done ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 0/2] Fix cgroup metric association with BPF counters 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 5:01 ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 5:01 ` [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers @ 2026-05-19 15:27 ` Ian Rogers 2026-05-19 15:27 ` [PATCH v4 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 15:27 ` [PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2 siblings, 2 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 15:27 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev This series fixes an issue where cgroup metrics were not being correctly associated and reported (showing `nan %`) when using BPF counters (i.e., with `--bpf-counters`). The root cause is that cgroup BPF counters only open the "leader" events (for the first cgroup) and leave "follower" events unopened. Unopened events have their `supported` flag set to `false` by default. During metric calculation, `prepare_metric` checks `evsel->supported` and discards the value (setting it to `NAN`) if it is `false`, leading to `nan %` in the output. The first patch fixes this by propagating the `supported` flag from the leader events to the follower events in `bperf_load_program`. It also adds a validation check to prevent potential division-by-zero (SIGFPE) crashes. The second patch adds a new shell test (`stat_metrics_cgrp.sh`) to permanently cover this scenario (testing both with and without BPF counters) and prevent regressions. Reported-by: Svilen Kanev <skanev@google.com> Changes since v3: - Add validation check in `bperf_load_program` to prevent potential division-by-zero (SIGFPE) when `num_events` is 0 (e.g., due to a trailing comma in cgroups list). - In the test script, dynamically pair instructions and cycles events using the PMU name. This ensures that we correctly verify if the associated cycles event was counted before performing the metric check, regardless of which event (cycles or instructions) is the metric leader. - Collect Namhyung's Acked-by for patch 1. Changes since v2: - Use `--metrics=insn_per_cycle` for the system-wide capability check in `check_system_wide` to ensure both cycles and instructions are supported. - Use exact cgroup field matching with surrounding commas (`,${cgrp},`) in the `grep` check to prevent false positives when matching the root cgroup (`/`). - Only enforce metric validation if both `cycles` and `instructions` counts are numeric and non-zero, preventing false failures on idle cgroups. - Wrap long lines and fix trailing whitespace in the test script to comply with style guidelines. Changes since v1: - Fix commit message for patch 2 to remove the mention of insn_per_cycle check (which was simplified out). - Quote variables in shell script to prevent word splitting. - Use grep -F in shell script for literal matching of cgroup names containing dots. Ian Rogers (2): perf stat: Propagate supported flag to follower cgroup BPF events perf test: Add stat metrics --for-each-cgroup test tools/perf/tests/shell/stat_metrics_cgrp.sh | 200 ++++++++++++++++++++ tools/perf/util/bpf_counter_cgroup.c | 20 ++ 2 files changed, 220 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 1/2] perf stat: Propagate supported flag to follower cgroup BPF events 2026-05-19 15:27 ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers @ 2026-05-19 15:27 ` Ian Rogers 2026-05-19 15:27 ` [PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 1 sibling, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 15:27 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev When using BPF counters with cgroups, follower events (for cgroups other than the first one) are not opened. Because they are not opened, their `supported` flag was left as `false`. During metric calculation, `prepare_metric` checks if the event is supported. If it is not supported (like the follower events), it explicitly sets the value to `NAN`, which eventually causes the metric to be reported as `nan %`. Fix this by propagating the `supported` flag from the "leader" events (the ones opened for the first cgroup) to the "follower" events. Also add a validation check to `bperf_load_program` to ensure `nr_cgroups` is not zero and the number of events is a multiple of `nr_cgroups`, preventing a potential division-by-zero (SIGFPE) exception when `num_events` evaluates to 0 (e.g., with a trailing comma in cgroups list). Reported-by: Svilen Kanev <skanev@google.com> Acked-by: Namhyung Kim <namhyung@kernel.org> Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter_cgroup.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index 519fee3dc3d0..e1ce5aa3b957 100644 --- a/tools/perf/util/bpf_counter_cgroup.c +++ b/tools/perf/util/bpf_counter_cgroup.c @@ -104,6 +104,11 @@ static int bperf_load_program(struct evlist *evlist) set_max_rlimit(); + if (nr_cgroups == 0 || evlist->core.nr_entries % nr_cgroups != 0) { + pr_err("Invalid cgroup or event count\n"); + return -EINVAL; + } + test_max_events_program_load(); skel = bperf_cgroup_bpf__open(); @@ -186,6 +191,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); + if (leader) + evsel->supported = leader->supported; + } + } + /* * bperf uses BPF_PROG_TEST_RUN to get accurate reading. Check * whether the kernel support it -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test 2026-05-19 15:27 ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 15:27 ` [PATCH v4 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers @ 2026-05-19 15:27 ` Ian Rogers 1 sibling, 0 replies; 18+ messages in thread From: Ian Rogers @ 2026-05-19 15:27 UTC (permalink / raw) To: irogers, acme, namhyung Cc: adrian.hunter, james.clark, jolsa, linux-kernel, linux-perf-users, mingo, peterz, skanev Add a new shell test `stat_metrics_cgrp.sh` to verify metric reporting with `--for-each-cgroup`, both with and without `--bpf-counters`. The test: - Checks if system-wide monitoring is supported (skips if not). - Finds cgroups to test. - Runs `perf stat` with `insn_per_cycle` metric and verifies that the metric is reported for each cgroup. - Dynamically pairs and verifies instructions and cycles counts to avoid false failures on idle cgroups. - Tests both standard mode and BPF counters mode (if supported). Assisted-by: Antigravity:gemini-3-flash Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/tests/shell/stat_metrics_cgrp.sh | 200 ++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100755 tools/perf/tests/shell/stat_metrics_cgrp.sh diff --git a/tools/perf/tests/shell/stat_metrics_cgrp.sh b/tools/perf/tests/shell/stat_metrics_cgrp.sh new file mode 100755 index 000000000000..d4226ee0ae98 --- /dev/null +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh @@ -0,0 +1,200 @@ +#!/bin/bash +# perf stat metrics --for-each-cgroup test +# SPDX-License-Identifier: GPL-2.0 + +set -e + +test_cgroups= + +log_verbose() { + echo "$1" +} + +is_numeric_and_non_zero() { + local val="$1" + if [[ "${val}" =~ ^[0-9]+$ ]] && [ "${val}" -gt 0 ] + then + return 0 # True + fi + return 1 # False +} + +# skip if system-wide is not supported +check_system_wide() +{ + log_verbose "Checking system-wide..." + if ! perf stat -a --metrics=insn_per_cycle sleep 0.01 > /dev/null 2>&1 + then + log_verbose "Skipping: system-wide monitoring not supported" + exit 2 + fi +} + +# find two cgroups to measure +find_cgroups() +{ + log_verbose "Finding cgroups..." + # try usual systemd slices first + if [ -d /sys/fs/cgroup/system.slice ] && [ -d /sys/fs/cgroup/user.slice ] + then + test_cgroups="system.slice,user.slice" + log_verbose "Found cgroups: ${test_cgroups}" + return + fi + + # try root and self cgroups + find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3) + if [ -z "${find_cgroups_self_cgrp}" ] + then + # cgroup v2 doesn't specify perf_event + find_cgroups_self_cgrp=$(grep ^0: /proc/self/cgroup | cut -d: -f3) + fi + + if [ -z "${find_cgroups_self_cgrp}" ] + then + test_cgroups="/" + else + test_cgroups="/,${find_cgroups_self_cgrp}" + fi + log_verbose "Found cgroups: ${test_cgroups}" +} + +# Check if metric is reported for each cgroup +# $1: extra options (e.g. --bpf-counters) +check_metric_reported() +{ + local opts="$1" + local output + + log_verbose "Running check_metric_reported with opts '${opts}'..." + # Run perf stat + if ! output=$(perf stat -a ${opts} \ + --metrics=insn_per_cycle \ + --for-each-cgroup "${test_cgroups}" \ + -x, sleep 0.1 2>&1) + then + echo "FAIL: perf stat failed with exit code $?" + echo "Output: ${output}" + exit 1 + fi + + log_verbose "perf stat output:" + log_verbose "${output}" + + # Split test_cgroups by comma + IFS=',' read -r -a cgrps <<< "${test_cgroups}" + + for cgrp in "${cgrps[@]}"; do + # Find metric lines for this cgroup + # We use exact cgroup match with surrounding commas + local cgrp_lines + cgrp_lines=$(echo "${output}" | grep -F ",${cgrp}," | grep "insn_per_cycle" || true) + + if [ -z "${cgrp_lines}" ] + then + echo "FAIL: No metric lines found for cgroup '${cgrp}'" + exit 1 + fi + + # Parse each metric line + while read -r line; do + [ -z "${line}" ] && continue + + local val1 + val1=$(echo "${line}" | cut -d, -f1) + + local event_name + event_name=$(echo "${line}" | cut -d, -f3) + + local cycles_val="" + local inst_val="" + + if echo "${event_name}" | grep -q -i "cycles" + then + cycles_val="${val1}" + # Find corresponding instructions event + local inst_event_name + inst_event_name="${event_name/cpu-cycles/instructions}" + inst_event_name="${inst_event_name/cycles/instructions}" + + local inst_line + inst_line=$(echo "${output}" | \ + grep -F ",${cgrp}," | \ + grep -F "${inst_event_name}" || true) + inst_val=$(echo "${inst_line}" | cut -d, -f1) + elif echo "${event_name}" | grep -q -i "instructions" + then + inst_val="${val1}" + # Find corresponding cycles event (try cpu-cycles + # first, then cycles) + local cycles_event_name + cycles_event_name="${event_name/instructions/cpu-cycles}" + local cycles_line + cycles_line=$(echo "${output}" | \ + grep -F ",${cgrp}," | \ + grep -F "${cycles_event_name}" || true) + + if [ -z "${cycles_line}" ] + then + # Try "cycles" instead of "cpu-cycles" + cycles_event_name="${event_name/instructions/cycles}" + cycles_line=$(echo "${output}" | \ + grep -F ",${cgrp}," | \ + grep -F "${cycles_event_name}" || true) + fi + cycles_val=$(echo "${cycles_line}" | cut -d, -f1) + fi + + log_verbose "Cgroup '${cgrp}': event '${event_name}' \ +val '${cycles_val}', inst val '${inst_val}'" + + # Only enforce metric check if both cycles and + # instructions have non-zero numeric counts + if is_numeric_and_non_zero "${cycles_val}" && \ + is_numeric_and_non_zero "${inst_val}" + then + log_verbose "Enforcing metric check for cgroup '${cgrp}' \ +event '${event_name}'" + # Check for nan or nested in the metric value (7th field) + # Actually we can just check the whole line for simplicity + if echo "${line}" | grep -q -i -E ",nan,|,nested," + then + echo "FAIL: Invalid metric value (nan/nested) \ +for cgroup '${cgrp}'" + echo "Line: ${line}" + exit 1 + fi + # Check for empty metric value (2 consecutive + # commas before the unit) + if echo "${line}" | grep -q -E ",,[[:space:]]*[^,]*insn_per_cycle" + then + echo "FAIL: Empty metric value for cgroup '${cgrp}'" + echo "Line: ${line}" + exit 1 + fi + else + log_verbose "Skipping metric check for cgroup '${cgrp}' \ +event '${event_name}' (idle or not counted)" + fi + done <<< "${cgrp_lines}" + done + log_verbose "check_metric_reported passed for opts '${opts}'" +} + +check_system_wide +find_cgroups + +# Test 1: Without BPF counters +check_metric_reported "" + +# Test 2: With BPF counters (if supported) +log_verbose "Checking BPF support..." +if perf stat -a --bpf-counters --for-each-cgroup / true > /dev/null 2>&1 +then + log_verbose "BPF supported, running Test 2..." + check_metric_reported "--bpf-counters" +else + log_verbose "BPF not supported, skipping Test 2" +fi + +exit 0 -- 2.54.0.631.ge1b05301d1-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-05-25 22:46 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-18 22:22 [PATCH v1 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-18 22:22 ` [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-18 22:22 ` [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 1:51 ` [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 1:51 ` [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 5:01 ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 5:01 ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 5:47 ` Namhyung Kim 2026-05-19 5:01 ` [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers 2026-05-19 5:54 ` Namhyung Kim 2026-05-19 15:13 ` Ian Rogers 2026-05-19 17:09 ` Namhyung Kim 2026-05-25 19:49 ` Arnaldo Carvalho de Melo 2026-05-25 22:46 ` Ian Rogers 2026-05-19 15:27 ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers 2026-05-19 15:27 ` [PATCH v4 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers 2026-05-19 15:27 ` [PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox