On 2026-04-28 16:41, Mike Rapoport wrote:
From: "Mike Rapoport (Microsoft)" <[email protected]>
Several migration test use fork() to create worker processes. These
processes are later killed, but nothing collects their exit status and they
remain as zombies in the system.
Add a helper function that kills the worker processes, waitpid()s for
them and verifies the exit status.
Replace the loops that call kill() for each process with a call to that
helper.
The addition of waitpid() calls also makes sure the migrating process never
exits before the processes accessing the memory, so it's possible to drop
the call to prctl(PR_SET_PDEATHSIG, SIGHUP).
Reported-by: Luiz Capitulino <[email protected]>
Signed-off-by: Mike Rapoport (Microsoft) <[email protected]>
---
tools/testing/selftests/mm/migration.c | 54 +++++++++++++-------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/mm/migration.c
b/tools/testing/selftests/mm/migration.c
index 3630f2fb0800..76e380b74658 100644
--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c
@@ -67,6 +67,24 @@ FIXTURE_TEARDOWN(migration)
free(self->pids);
}
+static bool kill_children(FIXTURE_DATA(migration) * self)
+{
+ int i, status;
+ pid_t pid;
+
+ for (i = 0; i < self->nthreads; i++) {
+ pid = self->pids[i];
+ if (kill(pid, SIGTERM))
+ return false;
+ if (pid != waitpid(pid, &status, 0))
+ return false;
+ if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGTERM)
+ return false;
+ }
Why do we stop at the first error? Wouldn't it be better to kill &
collect all of them and then return false if at least one fails?
+
+ return true;
+}
+
int migrate(uint64_t *ptr, int n1, int n2)
{
int ret, tmp;
@@ -160,20 +178,14 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
memset(ptr, 0xde, TWOMEG);
for (i = 0; i < self->nthreads; i++) {
pid = fork();
- if (!pid) {
- prctl(PR_SET_PDEATHSIG, SIGHUP);
- /* Parent may have died before prctl so check now. */
- if (getppid() == 1)
- kill(getpid(), SIGHUP);
This goes beyond this series, but I wonder if we should have this check
in access_mem() in case the parent dies in migrate().
+ if (!pid)
access_mem(ptr);
- } else {
+ else
self->pids[i] = pid;
- }
}
ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
- for (i = 0; i < self->nthreads; i++)
- ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+ ASSERT_EQ(kill_children(self), true);
}
/*
@@ -236,20 +248,14 @@ TEST_F_TIMEOUT(migration, shared_anon_thp, 2*RUNTIME)
memset(ptr, 0xde, pmdsize);
for (i = 0; i < self->nthreads; i++) {
pid = fork();
- if (!pid) {
- prctl(PR_SET_PDEATHSIG, SIGHUP);
- /* Parent may have died before prctl so check now. */
- if (getppid() == 1)
- kill(getpid(), SIGHUP);
+ if (!pid)
access_mem(ptr);
- } else {
+ else
self->pids[i] = pid;
- }
}
ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
- for (i = 0; i < self->nthreads; i++)
- ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+ ASSERT_EQ(kill_children(self), true);
}
/*
@@ -300,20 +306,14 @@ TEST_F_TIMEOUT(migration, shared_anon_htlb, 2*RUNTIME)
memset(ptr, 0xde, hugepage_size);
for (i = 0; i < self->nthreads; i++) {
pid = fork();
- if (!pid) {
- prctl(PR_SET_PDEATHSIG, SIGHUP);
- /* Parent may have died before prctl so check now. */
- if (getppid() == 1)
- kill(getpid(), SIGHUP);
+ if (!pid)
access_mem(ptr);
- } else {
+ else
self->pids[i] = pid;
- }
}
ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
- for (i = 0; i < self->nthreads; i++)
- ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+ ASSERT_EQ(kill_children(self), true);
}
TEST_HARNESS_MAIN
