| Message ID | 133d0dc62cebb6fde2764af384b0166d98755a3c.1743438749.git.namcao@linutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: coredump: Some bug fixes | expand |
On 2025-03-31, Nam Cao <namcao@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 <namcao@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
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 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)); + + file = fopen(STACKDUMP_FILE, "r"); ASSERT_NE(file, NULL); /* Step 4: Make sure all stack pointer values are non-zero */
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(). 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 <namcao@linutronix.de> --- tools/testing/selftests/coredump/stackdump | 6 +----- tools/testing/selftests/coredump/stackdump_test.c | 13 ++++++------- 2 files changed, 7 insertions(+), 12 deletions(-)