linux-kernel - Re: [PATCH] perf test: Make test_arm_callgraph

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 21 Jun 2024 16:57:33 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: James Clark <james.clark@....com>
Cc: linux-perf-users@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"Liang, Kan" <kan.liang@...ux.intel.com>,
	Kajol Jain <kjain@...ux.ibm.com>, Spoorthy S <spoorts2@...ibm.com>,
	German Gomez <german.gomez@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf test: Make test_arm_callgraph_fp.sh more robust

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@....com>

Acked-by: Namhyung Kim <namhyung@...nel.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
>