Re: [PATCH 14/34] common: fix pkill by running test program in a separate session

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Feb 05, 2025 at 11:23:10AM +1100, Dave Chinner wrote:
> On Tue, Feb 04, 2025 at 01:25:57PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Run each test program with a separate session id so that we can tell
> > pkill to kill all processes of a given name, but only within our own
> > session id.  This /should/ suffice to run multiple fstests on the same
> > machine without one instance shooting down processes of another
> > instance.
> 
> I thought you were going to drop this because pidns stuff available.
> Also, because it is only check parallel that needs the pidns
> isolation, and I'm not doing that external to check. Hence we can
> just get rid of the 'pkill --parent' requirement because the
> concurrent tests are already being run in isolated PID namespaces...
> 
> Regardless, if ppl still want to both pid session and pidns directly
> into check, the code is fine.

I'll make that clearer in the commit message.

> Just one little nit:
> 
> > diff --git a/tools/run_seq_setsid b/tools/run_seq_setsid
> > new file mode 100755
> > index 00000000000000..5938f80e689255
> > --- /dev/null
> > +++ b/tools/run_seq_setsid
> > @@ -0,0 +1,22 @@
> > +#!/bin/bash
> > +
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 Oracle.  All Rights Reserved.
> > +#
> > +# Try starting things in a new process session so that test processes have
> > +# something with which to filter only their own subprocesses.
> > +
> > +if [ -n "${FSTESTS_ISOL}" ]; then
> > +	# Allow the test to become a target of the oom killer
> > +	oom_knob="/proc/self/oom_score_adj"
> > +	test -w "${oom_knob}" && echo 250 > "${oom_knob}"
> > +
> > +	exec "$@"
> > +fi
> > +
> > +if [ -z "$1" ] || [ "$1" = "--help" ]; then
> > +	echo "Usage: $0 command [args...]"
> > +	exit 1
> > +fi
> > +
> > +FSTESTS_ISOL=setsid exec setsid "$0" "$@"
> 
> The wrapper should be called 'run_setsid' because what check is
> using it for has nothing to do with what the script actually does.

Done.

> With that change:
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Thank you!

--D

> -- 
> Dave Chinner
> david@fromorbit.com
>