mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: James Clark <james.clark@linaro.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com,
	alexander.shishkin@linux.intel.com, german.gomez@arm.com,
	jolsa@kernel.org, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, mingo@redhat.com,
	namhyung@kernel.org, peterz@infradead.org
Subject: Re: [PATCH v4] perf test: Fixes for check branch stack sampling
Date: Thu, 9 Apr 2026 09:23:56 +0100	[thread overview]
Message-ID: <1d6da8c9-c8e2-4edf-adee-7eac2f832c51@linaro.org> (raw)
In-Reply-To: <20260409000216.196083-1-irogers@google.com>



On 09/04/2026 1:02 am, Ian Rogers wrote:
> When filtering branch stack samples on user events they sample in user
> land but may have come from the kernel. Aarch64 avoids leaking the
> kernel address for kaslr reasons but other platforms, for now,
> don't. Be more permissive in allowing kernel addresses in the source
> of user branch stacks.
> 
> When filtering branch stack samples on kernel events they sample in
> kernel land but may have come from user land. Avoid the target being a
> user address but allow the source to be in user land. Aarch64 may not
> leak the user land addresses (making them 0) but other platforms
> do. As the kernel address sampling implies privelege, just allow this.
> 
> Increase the duration of the system call sampling test to make the
> likelihood of sampling a system call higher (increased from 1000 to
> 8000 loops - a number found through experimentation on an Intel
> Tigerlake laptop), also make the period of the event a prime number.
> 
> Put unneeded perf record output into a temporary file so that the test
> output isn't cluttered. More clearly state which test is running and
> the pass, fail or skipped result of the test.
> 
> These changes make the test on an Intel tigerlake laptop reliably pass
> rather than reliably fail.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

Reviewed-by: James Clark <james.clark@linaro.org>

> ---
> v4: Address feedback from James Clark.
> v2,v3: Address Sashiko feedback.
> ---
>   tools/perf/tests/shell/test_brstack.sh | 146 ++++++++++++++++---------
>   1 file changed, 96 insertions(+), 50 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
> index 85233d435be6..eb5837f82e39 100755
> --- a/tools/perf/tests/shell/test_brstack.sh
> +++ b/tools/perf/tests/shell/test_brstack.sh
> @@ -38,9 +38,13 @@ is_arm64() {
>   	[ "$(uname -m)" = "aarch64" ];
>   }
>   
> +has_kaslr_bug() {
> +	[ "$(uname -m)" != "aarch64" ];
> +}
> +
>   check_branches() {
>   	if ! tr -s ' ' '\n' < "$TMPDIR/perf.script" | grep -E -m1 -q "$1"; then
> -		echo "Branches missing $1"
> +		echo "ERROR: Branches missing $1"
>   		err=1
>   	fi
>   }
> @@ -48,6 +52,8 @@ check_branches() {
>   test_user_branches() {
>   	echo "Testing user branch stack sampling"
>   
> +	start_err=$err
> +	err=0
>   	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"
>   
> @@ -73,59 +79,88 @@ test_user_branches() {
>   	perf script -i "$TMPDIR/perf.data" --fields brstack | \
>   		tr ' ' '\n' > "$TMPDIR/perf.script"
>   
> -	# There should be no kernel addresses with the u option, in either
> -	# source or target addresses.
> -	if grep -E -m1 "0x[89a-f][0-9a-f]{15}" $TMPDIR/perf.script; then
> -		echo "ERROR: Kernel address found in user mode"
> +	# There should be no kernel addresses in the target with the u option.
> +	local regex="0x[89a-f][0-9a-f]{15}"
> +	if has_kaslr_bug; then
> +		# If the system has a kaslr bug that may leak kernel addresses
> +		# in the source of something like an ERET/SYSRET. Make the regex
> +		# more specific and just check the target address is in user
> +		# code.
> +		regex="^0x[0-9a-f]{0,16}/0x[89a-f][0-9a-f]{15}/"
> +	fi
> +	if grep -q -E -m1 "$regex" $TMPDIR/perf.script; then
> +		echo "Testing user branch stack sampling [Failed kernel address found in user mode]"
>   		err=1
>   	fi
>   	# some branch types are still not being tested:
>   	# IND COND_CALL COND_RET SYSRET SERROR NO_TX
> +	if [ $err -eq 0 ]; then
> +		echo "Testing user branch stack sampling [Passed]"
> +		err=$start_err
> +	else
> +		echo "Testing user branch stack sampling [Failed]"
> +	fi
>   }
>   
>   test_trap_eret_branches() {
>   	echo "Testing trap & eret branches"
> +
>   	if ! is_arm64; then
> -		echo "skip: not arm64"
> +		echo "Testing trap & eret branches [Skipped not arm64]"
> +		return
> +	fi
> +	start_err=$err
> +	err=0
> +	perf record -o $TMPDIR/perf.data --branch-filter any,save_type,u,k -- \
> +		perf test -w traploop 1000 > "$TMPDIR/record.txt" 2>&1
> +	perf script -i $TMPDIR/perf.data --fields brstacksym | \
> +		tr ' ' '\n' > $TMPDIR/perf.script
> +
> +	# BRBINF<n>.TYPE == TRAP are mapped to PERF_BR_IRQ by the BRBE driver
> +	check_branches "^trap_bench\+[^ ]+/[^ ]/IRQ/"
> +	check_branches "^[^ ]+/trap_bench\+[^ ]+/ERET/"
> +	if [ $err -eq 0 ]; then
> +		echo "Testing trap & eret branches [Passed]"
> +		err=$start_err
>   	else
> -		perf record -o $TMPDIR/perf.data --branch-filter any,save_type,u,k -- \
> -			perf test -w traploop 1000
> -		perf script -i $TMPDIR/perf.data --fields brstacksym | \
> -			tr ' ' '\n' > $TMPDIR/perf.script
> -
> -		# BRBINF<n>.TYPE == TRAP are mapped to PERF_BR_IRQ by the BRBE driver
> -		check_branches "^trap_bench\+[^ ]+/[^ ]/IRQ/"
> -		check_branches "^[^ ]+/trap_bench\+[^ ]+/ERET/"
> +		echo "Testing trap & eret branches [Failed]"
>   	fi
>   }
>   
>   test_kernel_branches() {
> -	echo "Testing that k option only includes kernel source addresses"
> +	echo "Testing kernel branch sampling"
>   
> -	if ! perf record --branch-filter any,k -o- -- true > /dev/null; then
> -		echo "skip: not enough privileges"
> +	if ! perf record --branch-filter any,k -o- -- true > "$TMPDIR/record.txt" 2>&1; then
> +		echo "Testing that k option [Skipped not enough privileges]"
> +		return
> +	fi
> +	start_err=$err
> +	err=0
> +	perf record -o $TMPDIR/perf.data --branch-filter any,k -- \
> +		perf bench syscall basic --loop 1000 > "$TMPDIR/record.txt" 2>&1
> +	perf script -i $TMPDIR/perf.data --fields brstack | \
> +		tr ' ' '\n' > $TMPDIR/perf.script
> +
> +	# Example of branch entries:
> +	#       "0xffffffff93bda241/0xffffffff93bda20f/M/-/-/..."
> +	# Source addresses come first in user or kernel code. Next is the target
> +	# address that must be in the kernel.
> +
> +	# Look for source addresses with top bit set
> +	if ! grep -q -E -m1 "^0x[89a-f][0-9a-f]{15}" $TMPDIR/perf.script; then
> +		echo "Testing kernel branch sampling [Failed kernel branches missing]"
> +		err=1
> +	fi
> +	# Look for no target addresses without top bit set
> +	if grep -q -E -m1 "^0x[0-9a-f]{0,16}/0x[0-7][0-9a-f]{1,15}/" $TMPDIR/perf.script; then
> +		echo "Testing kernel branch sampling [Failed user branches found]"
> +		err=1
> +	fi
> +	if [ $err -eq 0 ]; then
> +		echo "Testing kernel branch sampling [Passed]"
> +		err=$start_err
>   	else
> -		perf record -o $TMPDIR/perf.data --branch-filter any,k -- \
> -			perf bench syscall basic --loop 1000
> -		perf script -i $TMPDIR/perf.data --fields brstack | \
> -			tr ' ' '\n' > $TMPDIR/perf.script
> -
> -		# Example of branch entries:
> -		#       "0xffffffff93bda241/0xffffffff93bda20f/M/-/-/..."
> -		# Source addresses come first and target address can be either
> -		# userspace or kernel even with k option, as long as the source
> -		# is in kernel.
> -
> -		#Look for source addresses with top bit set
> -		if ! grep -E -m1 "^0x[89a-f][0-9a-f]{15}" $TMPDIR/perf.script; then
> -			echo "ERROR: Kernel branches missing"
> -			err=1
> -		fi
> -		# Look for no source addresses without top bit set
> -		if grep -E -m1 "^0x[0-7][0-9a-f]{0,15}" $TMPDIR/perf.script; then
> -			echo "ERROR: User branches found with kernel filter"
> -			err=1
> -		fi
> +		echo "Testing kernel branch sampling [Failed]"
>   	fi
>   }
>   
> @@ -136,14 +171,15 @@ 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}  > "$TMPDIR/record.txt" 2>&1
> +	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 [ ! -s "$TMPDIR/perf.script" ]
>   	then
> -		echo "Empty script output"
> +		echo "Testing branch stack filtering [Failed empty script output]"
>   		err=1
>   		return
>   	fi
> @@ -154,26 +190,36 @@ test_filter() {
>   	  > "$TMPDIR/perf.script-filtered" || true
>   	if [ -s "$TMPDIR/perf.script-filtered" ]
>   	then
> -		echo "Unexpected branch filter in script output"
> +		echo "Testing branch stack filtering [Failed unexpected branch filter]"
>   		cat "$TMPDIR/perf.script"
>   		err=1
>   		return
>   	fi
> +	echo "Testing branch stack filtering [Passed]"
>   }
>   
>   test_syscall() {
>   	echo "Testing syscalls"
>   	# skip if perf doesn't have enough privileges
> -	if ! perf record --branch-filter any,k -o- -- true > /dev/null; then
> -		echo "skip: not enough privileges"
> +	if ! perf record --branch-filter any,k -o- -- true > "$TMPDIR/record.txt" 2>&1; then
> +		echo "Testing syscalls [Skipped: not enough privileges]"
> +		return
> +	fi
> +	start_err=$err
> +	err=0
> +	perf record -o $TMPDIR/perf.data --branch-filter \
> +		any_call,save_type,u,k -c 10007 -- \
> +		perf bench syscall basic --loop 8000  > "$TMPDIR/record.txt" 2>&1
> +	perf script -i $TMPDIR/perf.data --fields brstacksym | \
> +		tr ' ' '\n' > $TMPDIR/perf.script
> +
> +	check_branches "getppid[^ ]*/SYSCALL/"
> +
> +	if [ $err -eq 0 ]; then
> +		echo "Testing syscalls [Passed]"
> +		err=$start_err
>   	else
> -		perf record -o $TMPDIR/perf.data --branch-filter \
> -			any_call,save_type,u,k -c 10000 -- \
> -			perf bench syscall basic --loop 1000
> -		perf script -i $TMPDIR/perf.data --fields brstacksym | \
> -			tr ' ' '\n' > $TMPDIR/perf.script
> -
> -		check_branches "getppid[^ ]*/SYSCALL/"
> +		echo "Testing syscalls [Failed]"
>   	fi
>   }
>   set -e



  reply	other threads:[~2026-04-09  8:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260408061218.9FFAEC19424@smtp.kernel.org>
2026-04-08  6:20 ` [PATCH v2] " Ian Rogers
2026-04-08  6:58   ` [PATCH v3] " Ian Rogers
2026-04-08 12:37     ` James Clark
2026-04-08 22:02       ` Ian Rogers
2026-04-09  0:02         ` [PATCH v4] " Ian Rogers
2026-04-09  8:23           ` James Clark [this message]
2026-04-13 19:45             ` Ian Rogers
2026-04-14 17:11           ` Namhyung Kim

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=1d6da8c9-c8e2-4edf-adee-7eac2f832c51@linaro.org \
    --to=james.clark@linaro.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=german.gomez@arm.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /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

all inboxes | Powered by JetHome®