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

Ian Rogers posted 2 patches 6 days, 5 hours ago
There is a newer version of this series
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
[PATCH v1 0/2] Fix cgroup metric association with BPF counters
Posted by Ian Rogers 6 days, 5 hours ago
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 v2 0/2] Fix cgroup metric association with BPF counters
Posted by Ian Rogers 6 days, 2 hours ago
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 v3 0/2] Fix cgroup metric association with BPF counters
Posted by Ian Rogers 5 days, 22 hours ago
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 v4 0/2] Fix cgroup metric association with BPF counters
Posted by Ian Rogers 5 days, 12 hours ago
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
Posted by Ian Rogers 5 days, 12 hours ago
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
Posted by Ian Rogers 5 days, 12 hours ago
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
[PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events
Posted by Ian Rogers 5 days, 22 hours ago
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
Re: [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events
Posted by Namhyung Kim 5 days, 22 hours ago
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
>
[PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test
Posted by Ian Rogers 5 days, 22 hours ago
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 2/2] perf test: Add stat metrics --for-each-cgroup test
Posted by Namhyung Kim 5 days, 21 hours ago
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
Posted by Ian Rogers 5 days, 12 hours ago
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
> >
Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test
Posted by Namhyung Kim 5 days, 10 hours ago
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
[PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events
Posted by Ian Rogers 6 days, 2 hours ago
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
Posted by Ian Rogers 6 days, 2 hours ago
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