Linux Perf Users
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com, james.clark@linaro.org,
	jolsa@kernel.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, mingo@redhat.com,
	peterz@infradead.org, skanev@google.com
Subject: Re: [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test
Date: Mon, 18 May 2026 22:54:56 -0700	[thread overview]
Message-ID: <agv7MMF7AEc_mdCN@z2> (raw)
In-Reply-To: <20260519050150.3144907-3-irogers@google.com>

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
> 

  parent reply	other threads:[~2026-05-19  5:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 22:22 [PATCH v1 0/2] Fix cgroup metric association with BPF counters Ian Rogers
2026-05-18 22:22 ` [PATCH v1 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers
2026-05-18 22:22 ` [PATCH v1 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers
2026-05-18 22:43   ` sashiko-bot
2026-05-19  1:51 ` [PATCH v2 0/2] Fix cgroup metric association with BPF counters Ian Rogers
2026-05-19  1:51   ` [PATCH v2 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers
2026-05-19  1:51   ` [PATCH v2 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers
2026-05-19  2:59     ` sashiko-bot
2026-05-19  5:01   ` [PATCH v3 0/2] Fix cgroup metric association with BPF counters Ian Rogers
2026-05-19  5:01     ` [PATCH v3 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers
2026-05-19  5:39       ` sashiko-bot
2026-05-19  5:47       ` Namhyung Kim
2026-05-19  5:01     ` [PATCH v3 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers
2026-05-19  5:47       ` sashiko-bot
2026-05-19  5:54       ` Namhyung Kim [this message]
2026-05-19 15:13         ` Ian Rogers
2026-05-19 17:09           ` Namhyung Kim
2026-05-19 15:27     ` [PATCH v4 0/2] Fix cgroup metric association with BPF counters Ian Rogers
2026-05-19 15:27       ` [PATCH v4 1/2] perf stat: Propagate supported flag to follower cgroup BPF events Ian Rogers
2026-05-19 15:27       ` [PATCH v4 2/2] perf test: Add stat metrics --for-each-cgroup test Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agv7MMF7AEc_mdCN@z2 \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=skanev@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox