Re: [PATCH] perf test: Make test_arm_callgraph_fp.sh more robust

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Jun 12, 2024 at 03:03:14PM +0100, James Clark wrote:
> The 2 second sleep can cause the test to fail on very slow network file
> systems because Perf ends up being killed before it finishes starting
> up.
> 
> Fix it by making the leafloop workload end after a fixed time like the
> other workloads so there is no need to kill it after 2 seconds.
> 
> Also remove the 1 second start sampling delay because it is similarly
> fragile. Instead, search through all samples for a matching one, rather
> than just checking the first sample and hoping it's in the right place.
> 
> Fixes: cd6382d82752 ("perf test arm64: Test unwinding using fame-pointer (fp) mode")
> Signed-off-by: James Clark <james.clark@arm.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  .../perf/tests/shell/test_arm_callgraph_fp.sh | 27 +++++++------------
>  tools/perf/tests/workloads/leafloop.c         | 20 +++++++++++---
>  2 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> index 61898e256616..9caa36130175 100755
> --- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> +++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
> @@ -28,28 +28,21 @@ cleanup_files()
>  
>  trap cleanup_files EXIT TERM INT
>  
> -# Add a 1 second delay to skip samples that are not in the leaf() function
>  # shellcheck disable=SC2086
> -perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- $TEST_PROGRAM 2> /dev/null &
> -PID=$!
> +perf record -o "$PERF_DATA" --call-graph fp -e cycles//u --user-callchains -- $TEST_PROGRAM
>  
> -echo " + Recording (PID=$PID)..."
> -sleep 2
> -echo " + Stopping perf-record..."
> -
> -kill $PID
> -wait $PID
> +# Try opening the file so any immediate errors are visible in the log
> +perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4
>  
> -# expected perf-script output:
> +# expected perf-script output if 'leaf' has been inserted correctly:
>  #
> -# program
> +# perf
>  # 	728 leaf
>  # 	753 parent
>  # 	76c leafloop
> -# ...
> +# ... remaining stack to main() ...
>  
> -perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4
> -perf script -i "$PERF_DATA" -F comm,ip,sym | head -n4 | \
> -	awk '{ if ($2 != "") sym[i++] = $2 } END { if (sym[0] != "leaf" ||
> -						       sym[1] != "parent" ||
> -						       sym[2] != "leafloop") exit 1 }'
> +# Each frame is separated by a tab, some spaces and an address
> +SEP="[[:space:]]+ [[:xdigit:]]+"
> +perf script -i "$PERF_DATA" -F comm,ip,sym | tr '\n' ' ' | \
> +	grep -E -q "perf $SEP leaf $SEP parent $SEP leafloop"
> diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> index 1bf5cc97649b..f7561767e32c 100644
> --- a/tools/perf/tests/workloads/leafloop.c
> +++ b/tools/perf/tests/workloads/leafloop.c
> @@ -1,6 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +#include <signal.h>
>  #include <stdlib.h>
>  #include <linux/compiler.h>
> +#include <unistd.h>
>  #include "../tests.h"
>  
>  /* We want to check these symbols in perf script */
> @@ -8,10 +10,16 @@ noinline void leaf(volatile int b);
>  noinline void parent(volatile int b);
>  
>  static volatile int a;
> +static volatile sig_atomic_t done;
> +
> +static void sighandler(int sig __maybe_unused)
> +{
> +	done = 1;
> +}
>  
>  noinline void leaf(volatile int b)
>  {
> -	for (;;)
> +	while (!done)
>  		a += b;
>  }
>  
> @@ -22,12 +30,16 @@ noinline void parent(volatile int b)
>  
>  static int leafloop(int argc, const char **argv)
>  {
> -	int c = 1;
> +	int sec = 1;
>  
>  	if (argc > 0)
> -		c = atoi(argv[0]);
> +		sec = atoi(argv[0]);
> +
> +	signal(SIGINT, sighandler);
> +	signal(SIGALRM, sighandler);
> +	alarm(sec);
>  
> -	parent(c);
> +	parent(sec);
>  	return 0;
>  }
>  
> -- 
> 2.34.1
>