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

[Namhyung Kim <namhyung@kernel.org>]

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