diff mbox series

[v1,5/5] perf test: Make daemon signal test less racy

Message ID 20240123000604.1211486-6-irogers@google.com
State New
Headers
Series Fixes for 6.8 PR1 |

Commit Message

Ian Rogers Jan. 23, 2024, 12:06 a.m. UTC 
  The daemon signal test sends signals and then expects files to be
written. It was observed on an Intel Alderlake that the signals were
sent too quickly leading to the 3 expected files not appearing. To
avoid this send the next signal only after the expected previous file
has appeared. To avoid an infinite loop the number of retries is
limited.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/daemon.sh | 34 ++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

Comments

Namhyung Kim Jan. 24, 2024, 12:02 a.m. UTC | #1 
On Mon, Jan 22, 2024 at 4:06 PM Ian Rogers <irogers@google.com> wrote:
>
> The daemon signal test sends signals and then expects files to be
> written. It was observed on an Intel Alderlake that the signals were
> sent too quickly leading to the 3 expected files not appearing. To
> avoid this send the next signal only after the expected previous file
> has appeared. To avoid an infinite loop the number of retries is
> limited.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/daemon.sh | 34 ++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
> index 4c598cfc5afa..de61e7898578 100755
> --- a/tools/perf/tests/shell/daemon.sh
> +++ b/tools/perf/tests/shell/daemon.sh
> @@ -414,16 +414,30 @@ EOF
>         # start daemon
>         daemon_start ${config} test
>
> -       # send 2 signals
> -       perf daemon signal --config ${config} --session test
> -       perf daemon signal --config ${config}
> -
> -       # stop daemon
> -       daemon_exit ${config}
> -
> -       # count is 2 perf.data for signals and 1 for perf record finished
> -       count=`ls ${base}/session-test/*perf.data* | wc -l`
> -       if [ ${count} -ne 3 ]; then
> +        # send 2 signals then exit. Do this in a loop watching the number of
> +        # files to avoid races. If the loop retries more than 600 times then
> +        # give up.
> +       local retries=0
> +       local signals=0
> +       local success=0
> +       while [ ${retries} -lt 600 ] && [ ${success} -eq 0 ]; do
> +               local files
> +               files=`ls ${base}/session-test/*perf.data* | wc -l`

Wouldn't it show error messages for 'file not found' for the first
round?  I think we can add '2> /dev/null' to suppress that.

Thanks,
Namhyung


> +               if [ ${signals} -eq 0 ]; then
> +                       perf daemon signal --config ${config} --session test
> +                       signals=1
> +               elif [ ${signals} -eq 1 ] && [ $files -ge 1 ]; then
> +                       perf daemon signal --config ${config}
> +                       signals=2
> +               elif [ ${signals} -eq 2 ] && [ $files -ge 2 ]; then
> +                       daemon_exit ${config}
> +                       signals=3
> +               elif [ ${signals} -eq 3 ] && [ $files -ge 3 ]; then
> +                       success=1
> +               fi
> +               retries=$((${retries} +1))
> +       done
> +       if [ ${success} -eq 0 ]; then
>                 error=1
>                 echo "FAILED: perf data no generated"
>         fi
> --
> 2.43.0.429.g432eaa2c6b-goog
>
diff mbox series

Patch

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 4c598cfc5afa..de61e7898578 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -414,16 +414,30 @@  EOF
 	# start daemon
 	daemon_start ${config} test
 
-	# send 2 signals
-	perf daemon signal --config ${config} --session test
-	perf daemon signal --config ${config}
-
-	# stop daemon
-	daemon_exit ${config}
-
-	# count is 2 perf.data for signals and 1 for perf record finished
-	count=`ls ${base}/session-test/*perf.data* | wc -l`
-	if [ ${count} -ne 3 ]; then
+        # send 2 signals then exit. Do this in a loop watching the number of
+        # files to avoid races. If the loop retries more than 600 times then
+        # give up.
+	local retries=0
+	local signals=0
+	local success=0
+	while [ ${retries} -lt 600 ] && [ ${success} -eq 0 ]; do
+		local files
+		files=`ls ${base}/session-test/*perf.data* | wc -l`
+		if [ ${signals} -eq 0 ]; then
+			perf daemon signal --config ${config} --session test
+			signals=1
+		elif [ ${signals} -eq 1 ] && [ $files -ge 1 ]; then
+			perf daemon signal --config ${config}
+			signals=2
+		elif [ ${signals} -eq 2 ] && [ $files -ge 2 ]; then
+			daemon_exit ${config}
+			signals=3
+		elif [ ${signals} -eq 3 ] && [ $files -ge 3 ]; then
+			success=1
+		fi
+		retries=$((${retries} +1))
+	done
+	if [ ${success} -eq 0 ]; then
 		error=1
 		echo "FAILED: perf data no generated"
 	fi