On Fri, Jun 24, 2022 at 09:41:34AM +0300, Amir Goldstein wrote:
> On Fri, Jun 24, 2022 at 9:31 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> >
> > 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?
> >
>
> I don't understand.
>
> V2 patch should be reviewed
>
> https://lore.kernel.org/fstests/20220621173729.2135249-1-amir73il@xxxxxxxxx/
>
> The fact that Darrick has future work pending to cleanup xfs/422
> should not stop a cleanup patch to be applied now.
Oh, maybe I misunderstood. I thought Darrick meant his patchset (which haven't
been sent out) contains what you did on xfs/422. If that's misunderstanding,
I'll review and merge your V2 :)
Thanks,
Zorro
>
> Am I missing something?
>
> Thanks,
> Amir.
>
