[PATCH v1 0/2] Fix cgroup metric association with BPF counters

[PATCH v1 0/2] Fix cgroup metric association with BPF counters

[Ian Rogers]

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

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

[Ian Rogers]

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

[PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

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

Re: [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Missing 'insn_per_cycle' metric support check contradicts the commit message and causes false-positive test failures.
- [Low] Unquoted variables in bash script can cause syntax errors and command execution failures.
- [Low] Unsafe regex matching with `grep` for cgroup paths.
--

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

perf test: Add stat metrics --for-each-cgroup test

This commit adds a new shell test stat_metrics_cgrp.sh to verify metric
reporting when using --for-each-cgroup. It tests metric outputs for cgroups
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.

The commit message mentions checking if the insn_per_cycle metric is
supported, but the script only seems to check for system-wide support
using 'perf stat -a -e instructions'.

If the metric is unavailable on the test architecture, will the script fail
with exit code 1 ("No metric lines found") instead of gracefully skipping?

> 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 0000000000000..4a29eb9a90674
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh
[ ... ]
> +	# try root and self cgroups
> +	find_cgroups_self_cgrp=$(grep perf_event /proc/self/cgroup | cut -d: -f3)
> +	if [ -z ${find_cgroups_self_cgrp} ]

This isn't a bug, but could this cause a bash syntax error ("too many
arguments") if the cgroup path happens to contain spaces? Enclosing the
variable in double quotes would prevent word splitting.

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

Are double quotes also needed here to prevent word splitting?

> +	then
> +		test_cgroups="/"
> +	else
> +		test_cgroups="/,${find_cgroups_self_cgrp}"
> +	fi
[ ... ]
> +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} \

Similarly, should ${test_cgroups} be quoted to ensure perf stat doesn't
misinterpret space-separated cgroup paths as commands?

> +			-x, sleep 0.1 2>&1)
> +	then
> +		echo "FAIL: perf stat failed with exit code $?"
> +		echo "Output: ${output}"
> +		exit 1
> +	fi
[ ... ]
> +	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)

Since cgroup names can contain dots (like "system.slice"), will grep
interpret the dot as a regex wildcard?

Using 'grep -F "${cgrp}"' might be safer to ensure a literal string match.

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

[PATCH v2 0/2] Fix cgroup metric association with BPF counters

[Ian Rogers]

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

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

[Ian Rogers]

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

[PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

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

Re: [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The test will falsely fail on healthy systems if the tested cgroups are idle.
- [Medium] The capability checks verify unrelated events instead of the tested metric, risking false failures on unsupported hardware.
- [Medium] A loose substring match causes the test to falsely pass if metrics for the root cgroup are missing.
--

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

perf test: Add stat metrics --for-each-cgroup test

This commit adds a new shell test stat_metrics_cgrp.sh to verify metric
reporting with --for-each-cgroup. It checks if system-wide monitoring is
supported and tests both standard mode and BPF counters mode.

> 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 0000000000000..2f74bd9e3f2ee
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh
[ ... ]
> +# 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
> +}

This check verifies the instructions event, but the test measures
insn_per_cycle which also requires cycles. If a test environment supports
instructions but lacks hardware support for cycles (such as in some virtual
environments), will this preliminary check pass but cause the actual test
to fail? Could this capability check use --metrics=insn_per_cycle instead?

[ ... ]
> +	# 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)

When cgrp is / (the root cgroup), won't grep -F "/" match any sub-cgroup
containing a slash, like /user.slice? If perf stat fails to output metrics
for the root cgroup, it seems cgrp_lines might still be populated by the
child's metrics, potentially causing the test to pass and hide a failure.

> +
> +		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}'"

If a tested cgroup like user.slice is completely idle during the 0.1 second
sleep, its cycles and instructions counts will be 0. Would the insn_per_cycle
metric evaluate to NaN and be output as an empty field in CSV mode? 

If so, will this check cause the test to falsely fail on healthy systems
where the tested cgroups happen to be idle?

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

[PATCH v3 0/2] Fix cgroup metric association with BPF counters

[Ian Rogers]

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

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

[Ian Rogers]

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

[PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

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

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

[sashiko-bot]

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

Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test incorrectly reads the `instructions` event count for both `cycles_val` and `inst_val`, failing to verify if the `cycles` event was actually counted before enforcing the metric check.
--

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

perf test: Add stat metrics --for-each-cgroup test

This commit adds a new shell test to verify metric reporting with
the --for-each-cgroup flag in perf stat. It checks that metrics like
insn_per_cycle are properly reported per-cgroup and do not contain
invalid values when events are actively counted.

> 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 0000000000000..b413849193a0b
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat_metrics_cgrp.sh
[ ... ]
> +	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)
[ ... ]
> +		# 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)

Since cgrp_lines is filtered by insn_per_cycle earlier, the event_name
extracted from the third field will typically be instructions rather than
cycles.

Does this cause both cycles_val and inst_val to read the exact same
instructions count? If event_name is already instructions, the string
replacement leaves it unchanged, and inst_line will match the original line.

This could mean the script never actually verifies if the cycles event
itself was successfully counted before enforcing the metric check.

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

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

[Namhyung Kim]

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
> 

Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[Namhyung Kim]

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
> 

Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

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

[PATCH v4 0/2] Fix cgroup metric association with BPF counters

[Ian Rogers]

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

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

[Ian Rogers]

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

[PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test

[Ian Rogers]

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

Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test

[Namhyung Kim]

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