Re: [PATCH 3/4] xfs/{422,517}: add missing killall to _cleanup()

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Thu, Jun 23, 2022 at 9:04 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Tue, Jun 21, 2022 at 07:02:02AM +0300, Amir Goldstein wrote:
> > On Sun, Jun 19, 2022 at 4:47 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > >
> > > Those tests failed to cleanup background jobs after test
> > > is interrupted.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> > >  tests/xfs/422 | 8 ++++++++
> > >  tests/xfs/517 | 1 +
> > >  2 files changed, 9 insertions(+)
> > >
> > > diff --git a/tests/xfs/422 b/tests/xfs/422
> > > index a83a66df..8e9a3576 100755
> > > --- a/tests/xfs/422
> > > +++ b/tests/xfs/422
> > > @@ -13,6 +13,14 @@ _begin_fstest dangerous_scrub dangerous_online_repair freeze
> > >
> > >  _register_cleanup "_cleanup" BUS
> > >
> > > +# Override the default cleanup function.
> > > +_cleanup()
> > > +{
> > > +       $KILLALL_PROG -9 $XFS_IO_PROG $FSSTRESS_PROG > /dev/null 2>&1
> > > +       cd /
> > > +       rm -rf $tmp.*
> > > +}
> > > +
> > >  # Import common functions.
> > >  . ./common/filter
> > >  . ./common/fuzzy
> > > diff --git a/tests/xfs/517 b/tests/xfs/517
> > > index 961668e3..18404248 100755
> > > --- a/tests/xfs/517
> > > +++ b/tests/xfs/517
> > > @@ -14,6 +14,7 @@ _register_cleanup "_cleanup" BUS
> > >  # Override the default cleanup function.
> > >  _cleanup()
> > >  {
> > > +       $KILLALL_PROG -9 $XFS_IO_PROG $FSSTRESS_PROG > /dev/null 2>&1
> >
> > Besides the missing wait, this cleanup is still racy, because stress_loop
> > can spawn a new fstress process after killall /proc iteration.
> >
> > Worst, freeze_loop can freeze after killall /proc iteration.
> >
> > The correct solution (I think) is to record the pid of the XXX_loop
> > sub-shells and kill those, which should also solve the "Terminated"
> > false positive error.
> >
> > Will give that a shot.
>
> FWIW I already have patches reworking 422 and all the other
> scrub-related stress tests:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=race-scrub-and-mount-state-changes
>

That looks very nice.
The cleanup looks very similar to the one I posted for v2,
but instead of "killall xfs_io fsstress" you would be better off with
"kill  $__SCRUB_STRESS_FREEZE_PID"

killing the child process and waiting for its parent bash
process has the side effect of the bash parent emitting
"Terminated" or "Killed" message with breaks golden output
if the inner loop did not already exit, which is the behavior
that got me started on doing this cleanup.

> I swear I'll send all these some day, if I ever get caught up...
> Delegating LTS maintenance is a big help, but as it is I still can't
> stay abreast of all the mainline patchsets /and/ send my own stuff. :(
>

I will respond to that in another email.

Thanks,
Amir.



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux