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 Fri, Jun 24, 2022 at 07:36:51AM +0300, Amir Goldstein wrote:
> 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.

Hi Amir,

So ... will you singly resend a V3 patch to remove the xfs/422 changes in this patch?

Thanks,
Zorro

> 
> 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