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

Darrick J. Wong
2025-01-22T21:46:10Z

On Wed, Jan 22, 2025 at 08:42:49PM +1100, Dave Chinner wrote:
> On Tue, Jan 21, 2025 at 11:05:20PM -0800, Darrick J. Wong wrote:
> > On Wed, Jan 22, 2025 at 05:08:17PM +1100, Dave Chinner wrote:
> > > On Tue, Jan 21, 2025 at 08:24:00PM -0800, Darrick J. Wong wrote:
> > > > On Tue, Jan 21, 2025 at 02:28:26PM +1100, Dave Chinner wrote:
> > > > > On Thu, Jan 16, 2025 at 03:27:15PM -0800, Darrick J. Wong wrote:
> > > > > > c) Putting test subprocesses in a systemd sub-scope and telling systemd
> > > > > > to kill the sub-scope could work because ./check can already use it to
> > > > > > ensure that all child processes of a test are killed. However, this is
> > > > > > an *optional* feature, which means that we'd have to require systemd.
> > > > >
> > > > > ... requiring systemd was somewhat of a show-stopper for testing
> > > > > older distros.
> > > >
> > > > Isn't RHEL7 the oldest one at this point? And it does systemd. At this
> > > > point the only reason I didn't go full systemd is out of consideration
> > > > for Devuan, since they probably need QA.
> > >
> > > I have no idea what is out there in distro land vs what fstests
> > > "supports". All I know is that there are distros out there that
> > > don't use systemd.
> > >
> > > It feels like poor form to prevent generic filesystem QA
> > > infrastructure from running on those distros by making an avoidable
> > > choice to tie the infrastructure exclusively to systemd-based
> > > functionality....
> >
> > Agreed, though at some point after these bugfixes are merged I'll see if
> > I can build on the existing "if you have systemd then ___ else here's
> > your shabby opencoded version" logic in fstests to isolate the ./checks
> > from each other a little better. It'd be kinda nice if we could
> > actually just put them in something resembling a modernish container,
> > albeit with the same underlying fs.
>
> Agreed, but I don't think we need to depend on systemd for that,
> either.
>
> > <shrug> Anyone else interested in that?
>
> check-parallel has already started down that road with the
> mount namespace isolation it uses for the runner tasks via
> src/nsexec.c.
>
> My plan has been to factor more of the check test running code
> (similar to what I did with the test list parsing) so that the
> check-parallel can iterate sections itself and runners can execute
> individual tests directly, rather than bouncing them through check
> to execute a set of tests serially. Then check-parallel could do
> whatever it needed to isolate individual tests from each other and
> nothing in check would need to change.
>
> Now I'm wondering if I can just run each runner's check instance
> in it's own private PID namespace as easily as I'm running them in
> their own private mount namespace...
>
> Hmmm - looks like src/nsexec.c can create new PID namespaces via
> the "-p" option. I haven't used that before - I wonder if that's a
> better solution that using per-test session IDs to solve the pkill
> --parent problem? Something to look into in the morning....

I tried that -- it appears to work, but then:

# ./src/nsexec -p bash
Current time: Wed Jan 22 13:43:33 PST 2025; Terminal: /dev/pts/0
# ps
fatal library error, lookup self
#

Regular processes and pkill/pgrep seem to work, but that didn't seem
especially encouraging so I stopped. :/

--D

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