Message-ID: <aCI2LnmPgauls0BN@x1>
Date: Mon, 12 May 2025 14:55:58 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	James Clark <james.clark@...aro.org>,
	Anshuman Khandual <anshuman.khandual@....com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	German Gomez <german.gomez@....com>
Subject: Re: [PATCH v3] perf tests: Harden branch stack sampling test

On Tue, Mar 18, 2025 at 09:16:39AM -0700, Ian Rogers wrote:
> On continuous testing the perf script output can be empty, or nearly
> empty, causing tr/grep to exit and due to "set -e" the test traps and
> fails. Add some empty file handling that sets the test to skip and
> make grep and other text rewriting failures non-fatal by adding
> "|| true".
> 
> Signed-off-by: Ian Rogers <irogers@...gle.com>


Thanks, applied to perf-tools-next,

- Arnaldo

> ---
> v3: Drop set -x
> v2: Change skips to errors Leo Yan.
> ---
>  tools/perf/tests/shell/test_brstack.sh | 72 +++++++++++++++++++-------
>  1 file changed, 52 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
> index e01df7581393..9138fa83bf36 100755
> --- a/tools/perf/tests/shell/test_brstack.sh
> +++ b/tools/perf/tests/shell/test_brstack.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  # Check branch stack sampling
>  
>  # SPDX-License-Identifier: GPL-2.0
> @@ -17,35 +17,50 @@ fi
>  
>  skip_test_missing_symbol brstack_bench
>  
> +err=0
>  TMPDIR=$(mktemp -d /tmp/__perf_test.program.XXXXX)
>  TESTPROG="perf test -w brstack"
>  
>  cleanup() {
>  	rm -rf $TMPDIR
> +	trap - EXIT TERM INT
>  }
>  
> -trap cleanup EXIT TERM INT
> +trap_cleanup() {
> +	set +e
> +	echo "Unexpected signal in ${FUNCNAME[1]}"
> +	cleanup
> +	exit 1
> +}
> +trap trap_cleanup EXIT TERM INT
>  
>  test_user_branches() {
>  	echo "Testing user branch stack sampling"
>  
> -	perf record -o $TMPDIR/perf.data --branch-filter any,save_type,u -- ${TESTPROG} > /dev/null 2>&1
> -	perf script -i $TMPDIR/perf.data --fields brstacksym | tr -s ' ' '\n' > $TMPDIR/perf.script
> +	perf record -o "$TMPDIR/perf.data" --branch-filter any,save_type,u -- ${TESTPROG} > "$TMPDIR/record.txt" 2>&1
> +	perf script -i "$TMPDIR/perf.data" --fields brstacksym > "$TMPDIR/perf.script"
>  
>  	# example of branch entries:
>  	# 	brstack_foo+0x14/brstack_bar+0x40/P/-/-/0/CALL
>  
> -	set -x
> -	grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/IND_CALL/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/CALL/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_bar\+[^ ]*/brstack_foo\+[^ ]*/RET/.*$"		$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bench\+[^ ]*/RET/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bench\+[^ ]*/COND/.*$"	$TMPDIR/perf.script
> -	grep -E -m1 "^brstack\+[^ ]*/brstack\+[^ ]*/UNCOND/.*$"		$TMPDIR/perf.script
> -	set +x
> -
> +	expected=(
> +		"^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/IND_CALL/.*$"
> +		"^brstack_foo\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$"
> +		"^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/CALL/.*$"
> +		"^brstack_bench\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$"
> +		"^brstack_bar\+[^ ]*/brstack_foo\+[^ ]*/RET/.*$"
> +		"^brstack_foo\+[^ ]*/brstack_bench\+[^ ]*/RET/.*$"
> +		"^brstack_bench\+[^ ]*/brstack_bench\+[^ ]*/COND/.*$"
> +		"^brstack\+[^ ]*/brstack\+[^ ]*/UNCOND/.*$"
> +	)
> +	for x in "${expected[@]}"
> +	do
> +		if ! tr -s ' ' '\n' < "$TMPDIR/perf.script" | grep -E -m1 -q "$x"
> +		then
> +			echo "Branches missing $x"
> +			err=1
> +		fi
> +	done
>  	# some branch types are still not being tested:
>  	# IND COND_CALL COND_RET SYSCALL SYSRET IRQ SERROR NO_TX
>  }
> @@ -57,14 +72,28 @@ test_filter() {
>  	test_filter_expect=$2
>  
>  	echo "Testing branch stack filtering permutation ($test_filter_filter,$test_filter_expect)"
> -
> -	perf record -o $TMPDIR/perf.data --branch-filter $test_filter_filter,save_type,u -- ${TESTPROG} > /dev/null 2>&1
> -	perf script -i $TMPDIR/perf.data --fields brstack | tr -s ' ' '\n' | grep '.' > $TMPDIR/perf.script
> +	perf record -o "$TMPDIR/perf.data" --branch-filter "$test_filter_filter,save_type,u" -- ${TESTPROG}  > "$TMPDIR/record.txt" 2>&1
> +	perf script -i "$TMPDIR/perf.data" --fields brstack > "$TMPDIR/perf.script"
>  
>  	# fail if we find any branch type that doesn't match any of the expected ones
>  	# also consider UNKNOWN branch types (-)
> -	if grep -E -vm1 "^[^ ]*/($test_filter_expect|-|( *))/.*$" $TMPDIR/perf.script; then
> -		return 1
> +	if [ ! -s "$TMPDIR/perf.script" ]
> +	then
> +		echo "Empty script output"
> +		err=1
> +		return
> +	fi
> +	# Look for lines not matching test_filter_expect ignoring issues caused
> +	# by empty output
> +	tr -s ' ' '\n' < "$TMPDIR/perf.script" | grep '.' | \
> +	  grep -E -vm1 "^[^ ]*/($test_filter_expect|-|( *))/.*$" \
> +	  > "$TMPDIR/perf.script-filtered" || true
> +	if [ -s "$TMPDIR/perf.script-filtered" ]
> +	then
> +		echo "Unexpected branch filter in script output"
> +		cat "$TMPDIR/perf.script"
> +		err=1
> +		return
>  	fi
>  }
>  
> @@ -80,3 +109,6 @@ test_filter "any_ret"	"RET|COND_RET|SYSRET|ERET"
>  test_filter "call,cond"		"CALL|SYSCALL|COND"
>  test_filter "any_call,cond"		"CALL|IND_CALL|COND_CALL|IRQ|SYSCALL|COND"
>  test_filter "cond,any_call,any_ret"	"COND|CALL|IND_CALL|COND_CALL|SYSCALL|IRQ|RET|COND_RET|SYSRET|ERET"
> +
> +cleanup
> +exit $err
> -- 
> 2.49.0.rc1.451.g8f38331e32-goog
> 

Powered by blists - more mailing lists