On 2025-03-31, Nam Cao <nam...@linutronix.de> wrote:
> The test waits for coredump to finish by busy-waiting for the
> stackdump_values file to be created. The maximum wait time is 10 seconds.
>
> This doesn't work for slow machine (qemu-system-riscv64), because coredump
> takes longer.
>
> Switch to use waitpid().
Note that you are now assuming that returning from waitpid() means that:
1. the coredumping has completed
and
2. the STACKDUMP_FILE with all its contents are visible to the parent
process
> With this, the stack_values file doesn't need to be atomically written by
> coredump anymore, therefore simplify the stackdump script.
>
> Fixes: 15858da53542 ("selftests: coredump: Add stackdump test")
> Signed-off-by: Nam Cao <nam...@linutronix.de>
> ---
> tools/testing/selftests/coredump/stackdump | 6 +-----
> tools/testing/selftests/coredump/stackdump_test.c | 13 ++++++-------
> 2 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/coredump/stackdump
> b/tools/testing/selftests/coredump/stackdump
> index 96714ce42d12..ad487fd5ff15 100755
> --- a/tools/testing/selftests/coredump/stackdump
> +++ b/tools/testing/selftests/coredump/stackdump
> @@ -4,11 +4,7 @@
> CRASH_PROGRAM_ID=$1
> STACKDUMP_FILE=$2
>
> -TMP=$(mktemp)
> -
> for t in /proc/$CRASH_PROGRAM_ID/task/*; do
> tid=$(basename $t)
> - cat /proc/$tid/stat | awk '{print $29}' >> $TMP
> + cat /proc/$tid/stat | awk '{print $29}' >> $STACKDUMP_FILE
> done
> -
> -mv $TMP $STACKDUMP_FILE
I would leave this as it was. Then the availability of STACKDUMP_FILE
means the full contents are available.
> diff --git a/tools/testing/selftests/coredump/stackdump_test.c
> b/tools/testing/selftests/coredump/stackdump_test.c
> index 1dc54e128586..733feaa0f895 100644
> --- a/tools/testing/selftests/coredump/stackdump_test.c
> +++ b/tools/testing/selftests/coredump/stackdump_test.c
> @@ -96,7 +96,7 @@ TEST_F(coredump, stackdump)
> char *test_dir, *line;
> size_t line_length;
> char buf[PATH_MAX];
> - int ret, i;
> + int ret, i, status;
> FILE *file;
> pid_t pid;
>
> @@ -131,12 +131,11 @@ TEST_F(coredump, stackdump)
> /*
> * Step 3: Wait for the stackdump script to write the stack pointers to
> the stackdump file
> */
> - for (i = 0; i < 10; ++i) {
> - file = fopen(STACKDUMP_FILE, "r");
> - if (file)
> - break;
> - sleep(1);
> - }
> + waitpid(pid, &status, 0);
> + ASSERT_TRUE(WIFSIGNALED(status));
> + ASSERT_TRUE(WCOREDUMP(status));
Why not just put these 3 lines above the for-loop? So you would wait for
the process to end, then go into the 20-second timeout loop waiting for
STACKDUMP_FILE to show up.
John Ogness
