Message ID | 20231213092043.1621684-1-jencce.kernel@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | generic/new: drop caches while freeze | expand |
On Wed, Dec 13, 2023 at 05:20:43PM +0800, Murphy Zhou wrote: > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> > --- > 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. > 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". > +# > +. ./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 And ... _scratch_mkfs >> $seqres.full ?? _scratch_mount ?? > + > +$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). > +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. > + > +# 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? > + > +# 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. > +# 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 > >
Hi Zorro, On Wed, Dec 13, 2023 at 11:34 PM Zorro Lang <zlang@redhat.com> wrote: > > On Wed, Dec 13, 2023 at 05:20:43PM +0800, Murphy Zhou wrote: > > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> > > --- > > 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 > > > > >
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. +# +. ./common/preamble +_begin_fstest freeze auto quick + +_supported_fs generic +_require_scratch + +$XFS_IO_PROG -fc "pwrite 0 64k" $SCRATCH_MNT/testfile >> $seqres.full +cat $SCRATCH_MNT/testfile >> $SCRATCH_MNT/testfile1 +cat $SCRATCH_MNT/testfile >> $SCRATCH_MNT/testfile1 + +sync + +fsfreeze -f $SCRATCH_MNT + +# This will hang if bug reproduces +echo 3 > /proc/sys/vm/drop_caches & + +# Manually unfreeze after a while +sleep 5 +fsfreeze -u $SCRATCH_MNT + +wait +# 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
Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> --- tests/generic/734 | 34 ++++++++++++++++++++++++++++++++++ tests/generic/734.out | 2 ++ 2 files changed, 36 insertions(+) create mode 100755 tests/generic/734 create mode 100644 tests/generic/734.out