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

[Mike Rapoport <rppt@kernel.org>]

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.

-- 
Sincerely yours,
Mike.