Re: [PATCH v3 04/54] selftests/mm: migration: properly cleanup fork()ed processes

Luiz Capitulino
2026-04-30T14:54:29Z

On 2026-04-30 10:40, Mike Rapoport wrote:
> On Thu, Apr 30, 2026 at 09:37:43AM -0400, Luiz Capitulino wrote:
>> On 2026-04-28 16:41, Mike Rapoport wrote:
>>> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>>>
>>> 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 <luizcap@redhat.com>
>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>> ---
>>> 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?
>
> Yep, makes sense.
>
>>> +
>>> + 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().
>
> With added kill() + waitpid() this should not happen.

I was thinking in the case of a crash, but looking at it again it seems
to be impossible so it's fine.