[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZnYTbZ30Vkrzm8xI@google.com>
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
>
Powered by blists - more mailing lists