public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: eugene.loh@oracle.com
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH] test: Clean up stack_fbt test
Date: Fri, 17 Oct 2025 02:18:59 -0400	[thread overview]
Message-ID: <aPHf07N4ISwnhK7K@oracle.com> (raw)
In-Reply-To: <20251016205641.15446-1-eugene.loh@oracle.com>

On Thu, Oct 16, 2025 at 04:56:41PM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> The idea behind the test is to check the stack() output for some fbt
> probe for specific expected frames.  An attempt was made to specify the
> exact stack that was expected, but there are too many variations among
> kernel versions and so maintaining the test was difficult.  Loosen the
> test to check for only a few expected frames.
> 
> The test was also checking that stack()'s first 3 frames matched
> stack(3), but such a test is already provided by, for example,
>     test/unittest/printf/tst.stack.d
>     test/unittest/stack/tst.stack.d
> So, drop the stack(3) stuff.

Since the kernel implementation can change between versions and architectures,
why not just verify that the function we're probing is the top frame, and then
perhaps for sanity ensure that the rest of the frames are resolved addresses
(i.e. symbolic nmes rather than hex addresses)?  Although that perhaps might be
a bit strict because I could see some architectures implementing the syscall
stuff somehow with dynamically generated trampolines or something that would
not have addr-to-symbol information.  Or ksplice might even cause that if the
splicing took place while tracing was going on because we never re-read the list
of kernel symbols after dtarce started.

> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  test/unittest/stack/tst.stack_fbt.r  |  1 +
>  test/unittest/stack/tst.stack_fbt.sh | 89 +++++++---------------------
>  2 files changed, 23 insertions(+), 67 deletions(-)
>  create mode 100644 test/unittest/stack/tst.stack_fbt.r
> 
> diff --git a/test/unittest/stack/tst.stack_fbt.r b/test/unittest/stack/tst.stack_fbt.r
> new file mode 100644
> index 000000000..2e9ba477f
> --- /dev/null
> +++ b/test/unittest/stack/tst.stack_fbt.r
> @@ -0,0 +1 @@
> +success
> diff --git a/test/unittest/stack/tst.stack_fbt.sh b/test/unittest/stack/tst.stack_fbt.sh
> index 15b85be13..f3d321e7f 100755
> --- a/test/unittest/stack/tst.stack_fbt.sh
> +++ b/test/unittest/stack/tst.stack_fbt.sh
> @@ -5,7 +5,7 @@
>  # Licensed under the Universal Permissive License v 1.0 as shown at
>  # http://oss.oracle.com/licenses/upl.
>  #
> -# Test the stack action with default stack depth and depth 3.
> +# Check the stack action for expected frames.
>  
>  dtrace=$1
>  
> @@ -26,8 +26,6 @@ BEGIN
>  fbt::vfs_write:entry
>  {
>  	stack();
> -	printf("first 3 frames\n");
> -	stack(3);
>  	exit(0);
>  }' >& dtrace.out
>  
> @@ -37,17 +35,16 @@ if [ $? -ne 0 ]; then
>  	exit 1
>  fi
>  
> -# Strip out
> -# - blank lines
> -# - "constprop"
> -# - "isra"
> +# Ignore blank lines and strip out
> +# - ".constprop.[0-9]"
>  # - "_after_hwframe"    (x86 starting with UEK8)
> -# - pointer values
> +# - "+0x[0-9a-f]*$"
> +# - leading spaces
>  
>  awk 'NF != 0 { sub("\\.constprop\\.[0-9]", "");
> -               sub("\\.isra\\.[0-9]", "");
>                 sub("_after_hwframe\\+", "+");
> -               sub(/+0x[0-9a-f]*$/, "+{ptr}");
> +               sub(/+0x[0-9a-f]*$/, "");
> +               sub(/^ */, "");
>                 print }' dtrace.out > dtrace.post
>  if [ $? -ne 0 ]; then
>  	echo ERROR: awk failed
> @@ -55,77 +52,35 @@ if [ $? -ne 0 ]; then
>  	exit 1
>  fi
>  
> -# Figure out what stack to expect.
> +# Identify, in order, a few frames we expect to see.
>  
> -read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
> -
> -if [ $MAJOR -eq 5 -a $MINOR -lt 8 ]; then
> -	# up to 5.8
> -	KERVER="A"
> -else
> -	# starting at 5.8
> -	KERVER="B"
> -fi
> -
> -if [ $(uname -m) == "x86_64" -a $KERVER == "A" ]; then
> -cat << EOF > dtrace.cmp
> -              vmlinux\`vfs_write+{ptr}
> -              vmlinux\`__x64_sys_write+{ptr}
> -              vmlinux\`x64_sys_call+{ptr}
> -              vmlinux\`do_syscall_64+{ptr}
> -              vmlinux\`entry_SYSCALL_64+{ptr}
> -EOF
> -elif [ $(uname -m) == "aarch64" -a $KERVER == "A" ]; then
> -cat << EOF > dtrace.cmp
> -              vmlinux\`vfs_write
> -              vmlinux\`__arm64_sys_write+{ptr}
> -              vmlinux\`el0_svc_common+{ptr}
> -              vmlinux\`el0_svc_handler+{ptr}
> -              vmlinux\`el0_svc+{ptr}
> -EOF
> -elif [ $(uname -m) == "x86_64" -a $KERVER == "B" ]; then
> -cat << EOF > dtrace.cmp
> -              vmlinux\`vfs_write+{ptr}
> -              vmlinux\`ksys_write+{ptr}
> -              vmlinux\`do_syscall_64+{ptr}
> -              vmlinux\`entry_SYSCALL_64+{ptr}
> -EOF
> -elif [ $(uname -m) == "aarch64" -a $KERVER == "B" ]; then
> -cat << EOF > dtrace.cmp
> -              vmlinux\`vfs_write
> -              vmlinux\`__arm64_sys_write+{ptr}
> -              vmlinux\`invoke_syscall+{ptr}
> -              vmlinux\`el0_svc_common+{ptr}
> -              vmlinux\`do_el0_svc+{ptr}
> -              vmlinux\`el0_svc+{ptr}
> -              vmlinux\`el0t_64_sync_handler+{ptr}
> -              vmlinux\`el0t_64_sync+{ptr}
> -EOF
> +if [ $(uname -m) == "x86_64" ]; then
> +	frames="vfs_write do_syscall_64 entry_SYSCALL_64"
> +elif [ $(uname -m) == "aarch64" ]; then
> +	frames="vfs_write __arm64_sys_write el0_svc_common el0_svc"
>  else
>  	echo ERROR: unrecognized platform
>  	uname -r
>  	uname -m
>  	exit 1
>  fi
> -
> -# Add the first 3 frames a second time.
> -
> -head -3 dtrace.cmp > dtrace.tmp
> -echo first 3 frames >> dtrace.cmp
> -cat dtrace.tmp >> dtrace.cmp
> +for frame in $frames; do
> +	echo 'vmlinux`'$frame >> dtrace.cmp
> +done
>  
>  # Compare results.
>  
> -if ! diff -q dtrace.cmp dtrace.post; then
> -	echo ERROR: results do not match
> -	diff dtrace.cmp dtrace.post
> -	echo "==== expect"
> +diff dtrace.cmp dtrace.post | grep '^<' > missing.frames
> +if [ `cat missing.frames | wc -l` -ne 0 ]; then
> +	echo ERROR: missing some expected frames
> +	echo === expected frames include:
>  	cat dtrace.cmp
> -	echo "==== actual"
> +	echo === actual frames are:
>  	cat dtrace.out
> +	echo === missing expected frames:
> +	cat missing.frames
>  	exit 1
>  fi
>  
>  echo success
> -
>  exit 0
> -- 
> 2.47.3
> 

  reply	other threads:[~2025-10-17  6:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-16 20:56 [PATCH] test: Clean up stack_fbt test eugene.loh
2025-10-17  6:18 ` Kris Van Hees [this message]
2025-10-17 17:32   ` Eugene Loh
2025-10-17 17:50     ` Kris Van Hees
2025-10-17 19:26       ` Eugene Loh
2025-10-17 20:19         ` Kris Van Hees
2025-10-20 18:56 ` Kris Van Hees

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=aPHf07N4ISwnhK7K@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.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