Hi Zorro, On Wed, Dec 13, 2023 at 11:34 PM Zorro Lang <zlang@xxxxxxxxxx> wrote: > > On Wed, Dec 13, 2023 at 05:20:43PM +0800, Murphy Zhou wrote: > > Signed-off-by: Murphy Zhou <jencce.kernel@xxxxxxxxx> > > --- > > tests/generic/734 | 34 ++++++++++++++++++++++++++++++++++ > > Hi Murphy, > > > Thanks for this new test. Sorry that g/734 has been taken last weekend, > please rebase to the latest for-next branch. Got it. > > > tests/generic/734.out | 2 ++ > > 2 files changed, 36 insertions(+) > > create mode 100755 tests/generic/734 > > create mode 100644 tests/generic/734.out > > > > diff --git a/tests/generic/734 b/tests/generic/734 > > new file mode 100755 > > index 00000000..8ca91930 > > --- /dev/null > > +++ b/tests/generic/734 > > @@ -0,0 +1,34 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# FS QA Test 734 > > +# > > +# Test possible deadlock of umount and reclaim memory > > +# when there are EOF blocks in files. > > Is there a known bug fix for this case? If yes, please specify that by: > "_fixed_by_kernel_commit xxxxxxxxxx xxxxxxxxxxxxxxxx". Maybe a patchset but I'm not sure of it. In this situation I think we can skip this part? > > > +# > > +. ./common/preamble > > +_begin_fstest freeze auto quick > > + > > +_supported_fs generic > > +_require_scratch > > This's a geneirc case, not all fs support fsfreeze, so ... > _require_freeze Got it. > > And ... > > _scratch_mkfs >> $seqres.full ?? > _scratch_mount ?? OK. > > > + > > +$XFS_IO_PROG -fc "pwrite 0 64k" $SCRATCH_MNT/testfile >> $seqres.full > > Could you explain why there're two duplicated lines below? If it's intended, > better to have a comment to explain why do it twice (not once or more times). OK. > > > +cat $SCRATCH_MNT/testfile >> $SCRATCH_MNT/testfile1 > > +cat $SCRATCH_MNT/testfile >> $SCRATCH_MNT/testfile1 > > + > > +sync > > + > > +fsfreeze -f $SCRATCH_MNT > > All cases in fstests do fs freeze by xfs_freeze. If there's not special > reason, better to keep consistency. Due to the _require_freeze checks > that too. OK. > > > + > > +# This will hang if bug reproduces > > +echo 3 > /proc/sys/vm/drop_caches & > > I'm wondering, if the data in cache has been written back and been cleared > before fsfreeze or this step, what will happen? If the cache data is necessary, > should we load/read more data/files to provide that? Agree. I'll add more files. > > > + > > +# Manually unfreeze after a while > > +sleep 5 > > +fsfreeze -u $SCRATCH_MNT > > + > > +wait > > As there's a fs freeze in this case, so we'd better to do a "forced" unfreeze > in _cleanup, to make sure the fs will be unfreezed 100%, no matter what. So > you can move these two lines to _cleanup. Good idea! Thanks for reviewing! Murphy > > > +# success, all done > > +echo "Silence is golden" > > +status=0 > > +exit > > diff --git a/tests/generic/734.out b/tests/generic/734.out > > new file mode 100644 > > index 00000000..4299839b > > --- /dev/null > > +++ b/tests/generic/734.out > > @@ -0,0 +1,2 @@ > > +QA output created by 734 > > +Silence is golden > > -- > > 2.31.1 > > > > >