diff mbox series

generic/new: drop caches while freeze

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

Commit Message

Murphy Zhou Dec. 13, 2023, 9:20 a.m. UTC
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

Comments

Zorro Lang Dec. 13, 2023, 3:34 p.m. UTC | #1
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
> 
>
Murphy Zhou Dec. 15, 2023, 6:21 a.m. UTC | #2
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 mbox series

Patch

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