Re: [PATCH 02/40] fstests: cleanup fsstress process management

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 05, 2024 at 01:57:15AM +0800, Zorro Lang wrote:
> On Wed, Nov 27, 2024 at 03:51:32PM +1100, Dave Chinner wrote:
> > +# Common execution handling for fsstress invocation.
> > +#
> > +# We need per-test fsstress binaries because of the way fsstress forks and
> > +# tests run it in the background and/or nest it. Trying to kill fsstress
> > +# tasks is unreliable because killing parent fsstress task does not guarantee
> > +# that the children get killed. Hence the historic use of killall for stopping
> > +# execution.
> > +#
> > +# However, we can't just kill all fsstress binaries as multiple tests might be
> > +# running fsstress at the same time. Hence copy the fsstress binary to a test
> > +# specific binary on the test device and use pkill to select that only that
> > +# task name to kill.
> > +#
> > +# If tasks want to start fsstress themselves (e.g. under a different uid) then
> > +# they can set up _FSSTRESS_BIN and record _FSSTRESS_PID themselves. Then if the
> > +# test is killed then it will get cleaned up automatically.
> > +
> > +_FSSTRESS_BIN="$seq.fsstress"
> > +_FSSTRESS_PROG="$TEST_DIR/$seq.fsstress"
> 
> Hi Dave,
> 
> I'm wondering if the "$seq.fsstress" can be unique name? For example, if generic/561,
> xfs/561, btrfs/561, ext4/561 run fsstress in parallel, won't they have same
> "561.fsstress", then "pkill -PIPE $_FSSTRESS_BIN" kills all these cases' fsstress
> processes?

Yeah, it's not entirely unique, but it was "good enough" to solve
the problem and move on to the next one. It should be easy enough to
change; the reason I used $seq was because it is short, and the
pkill man page says:

NOTES
       The  process  name  used  for  matching is limited to the 15
       characters present in the output of /proc/<pid>/stat.

It turns out that this 15 character limit doesn't appear to exist
anymore.  I just checked that file on a 6.10.7 kernel, and it
appears that there are full process names in there. pgrep (at least)
matches on process names longer than 15 character, so I suspect that
the man page is simply out of date here.

Hence we might be able to use something longer and more unique along
with 'pkill -x' to match the full name. i.e. this is a solvable
problem, but not critical to the effective working of concurrent
test running.

-Dave.

-- 
Dave Chinner
david@fromorbit.com