* [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
@ 2019-10-02 18:41 Josef Bacik
2019-10-03 10:59 ` Filipe Manana
2019-10-05 17:44 ` Eryu Guan
0 siblings, 2 replies; 5+ messages in thread
From: Josef Bacik @ 2019-10-02 18:41 UTC (permalink / raw)
To: fstests, linux-btrfs, kernel-team
I discovered a problem in btrfs where we'd end up pointing at a block we
hadn't written out yet. This is triggered by a race when two different
files on two different subvolumes fsync. This test exercises this path
with dm-log-writes, and then replays the log at every FUA to verify the
file system is still mountable and the log is replayable.
This test is to verify the fix
btrfs: fix incorrect updating of log root tree
actually fixed the problem.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
v1->v2:
- added the patchname related to this test in the comments and changelog.
- running fio makes it use 400mib of shared memory, so running 50 of them is
impossible on boxes that don't have hundreds of gib of RAM. Fixed this to
just generate a fio config so we can run 1 fio instance with 50 threads which
makes it not OOM boxes with tiny amounts of RAM.
- fixed some formatting things that Filipe pointed out.
tests/btrfs/194 | 111 ++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/194.out | 2 +
tests/btrfs/group | 1 +
3 files changed, 114 insertions(+)
create mode 100755 tests/btrfs/194
create mode 100644 tests/btrfs/194.out
diff --git a/tests/btrfs/194 b/tests/btrfs/194
new file mode 100755
index 00000000..b98064e2
--- /dev/null
+++ b/tests/btrfs/194
@@ -0,0 +1,111 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2019 Facebook. All Rights Reserved.
+#
+# FS QA Test 194
+#
+# Test multi subvolume fsync to test a bug where we'd end up pointing at a block
+# we haven't written. This was fixed by the patch
+#
+# btrfs: fix incorrect updating of log root tree
+#
+# Will do log replay and check the filesystem.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+fio_config=$tmp.fio
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ _log_writes_cleanup &> /dev/null
+ _dmthin_cleanup
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmthin
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+
+# Use thin device as replay device, which requires $SCRATCH_DEV
+_require_scratch_nocheck
+# and we need extra device as log device
+_require_log_writes
+_require_dm_target thin-pool
+
+cat >$fio_config <<EOF
+[global]
+readwrite=write
+fallocate=none
+bs=4k
+fsync=1
+size=128k
+EOF
+
+for i in $(seq 0 49); do
+ echo "[foo$i]" >> $fio_config
+ echo "filename=$SCRATCH_MNT/$i/file" >> $fio_config
+done
+
+_require_fio $fio_config
+
+cat $fio_config >> $seqres.full
+
+# Use a thin device to provide deterministic discard behavior. Discards are used
+# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
+_test_unmount
+_dmthin_init $devsize $devsize $csize $lowspace
+_log_writes_init $DMTHIN_VOL_DEV
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mark mkfs
+
+_log_writes_mount
+
+# First create all the subvolumes
+for i in $(seq 0 49); do
+ $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/$i" > /dev/null
+done
+
+$FIO_PROG $fio_config > /dev/null 2>&1
+_log_writes_unmount
+
+_log_writes_remove
+prev=$(_log_writes_mark_to_entry_number mkfs)
+[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
+cur=$(_log_writes_find_next_fua $prev)
+[ -z "$cur" ] && _fail "failed to locate next FUA write"
+
+while [ ! -z "$cur" ]; do
+ _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full
+
+ # We need to mount the fs because btrfsck won't bother checking the log.
+ _dmthin_mount
+ _dmthin_check_fs
+
+ prev=$cur
+ cur=$(_log_writes_find_next_fua $(($cur + 1)))
+ [ -z "$cur" ] && break
+done
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out
new file mode 100644
index 00000000..7bfd50ff
--- /dev/null
+++ b/tests/btrfs/194.out
@@ -0,0 +1,2 @@
+QA output created by 194
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index b92cb12c..0d0e1bba 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -196,3 +196,4 @@
191 auto quick send dedupe
192 auto replay snapshot stress
193 auto quick qgroup enospc limit
+194 auto metadata log volume
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
2019-10-02 18:41 [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing Josef Bacik
@ 2019-10-03 10:59 ` Filipe Manana
2019-10-03 11:12 ` Filipe Manana
2019-10-05 17:44 ` Eryu Guan
1 sibling, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2019-10-03 10:59 UTC (permalink / raw)
To: Josef Bacik; +Cc: fstests, linux-btrfs, kernel-team
On Wed, Oct 2, 2019 at 7:44 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> I discovered a problem in btrfs where we'd end up pointing at a block we
> hadn't written out yet. This is triggered by a race when two different
> files on two different subvolumes fsync. This test exercises this path
> with dm-log-writes, and then replays the log at every FUA to verify the
> file system is still mountable and the log is replayable.
>
> This test is to verify the fix
>
> btrfs: fix incorrect updating of log root tree
>
> actually fixed the problem.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
It's working now.
Confirmed this triggers the bug, and after about 4 hours of this
running with the btrfs patch, it doesn't trigger the bug anymore.
Thanks!
> ---
> v1->v2:
> - added the patchname related to this test in the comments and changelog.
> - running fio makes it use 400mib of shared memory, so running 50 of them is
> impossible on boxes that don't have hundreds of gib of RAM. Fixed this to
> just generate a fio config so we can run 1 fio instance with 50 threads which
> makes it not OOM boxes with tiny amounts of RAM.
> - fixed some formatting things that Filipe pointed out.
>
> tests/btrfs/194 | 111 ++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/194.out | 2 +
> tests/btrfs/group | 1 +
> 3 files changed, 114 insertions(+)
> create mode 100755 tests/btrfs/194
> create mode 100644 tests/btrfs/194.out
>
> diff --git a/tests/btrfs/194 b/tests/btrfs/194
> new file mode 100755
> index 00000000..b98064e2
> --- /dev/null
> +++ b/tests/btrfs/194
> @@ -0,0 +1,111 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2019 Facebook. All Rights Reserved.
> +#
> +# FS QA Test 194
> +#
> +# Test multi subvolume fsync to test a bug where we'd end up pointing at a block
> +# we haven't written. This was fixed by the patch
> +#
> +# btrfs: fix incorrect updating of log root tree
> +#
> +# Will do log replay and check the filesystem.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +fio_config=$tmp.fio
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + _log_writes_cleanup &> /dev/null
> + _dmthin_cleanup
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmthin
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +
> +# Use thin device as replay device, which requires $SCRATCH_DEV
> +_require_scratch_nocheck
> +# and we need extra device as log device
> +_require_log_writes
> +_require_dm_target thin-pool
> +
> +cat >$fio_config <<EOF
> +[global]
> +readwrite=write
> +fallocate=none
> +bs=4k
> +fsync=1
> +size=128k
> +EOF
> +
> +for i in $(seq 0 49); do
> + echo "[foo$i]" >> $fio_config
> + echo "filename=$SCRATCH_MNT/$i/file" >> $fio_config
> +done
> +
> +_require_fio $fio_config
> +
> +cat $fio_config >> $seqres.full
> +
> +# Use a thin device to provide deterministic discard behavior. Discards are used
> +# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
> +_test_unmount
> +_dmthin_init $devsize $devsize $csize $lowspace
> +_log_writes_init $DMTHIN_VOL_DEV
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mark mkfs
> +
> +_log_writes_mount
> +
> +# First create all the subvolumes
> +for i in $(seq 0 49); do
> + $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/$i" > /dev/null
> +done
> +
> +$FIO_PROG $fio_config > /dev/null 2>&1
> +_log_writes_unmount
> +
> +_log_writes_remove
> +prev=$(_log_writes_mark_to_entry_number mkfs)
> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
> +cur=$(_log_writes_find_next_fua $prev)
> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
> +
> +while [ ! -z "$cur" ]; do
> + _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full
> +
> + # We need to mount the fs because btrfsck won't bother checking the log.
> + _dmthin_mount
> + _dmthin_check_fs
> +
> + prev=$cur
> + cur=$(_log_writes_find_next_fua $(($cur + 1)))
> + [ -z "$cur" ] && break
> +done
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out
> new file mode 100644
> index 00000000..7bfd50ff
> --- /dev/null
> +++ b/tests/btrfs/194.out
> @@ -0,0 +1,2 @@
> +QA output created by 194
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index b92cb12c..0d0e1bba 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -196,3 +196,4 @@
> 191 auto quick send dedupe
> 192 auto replay snapshot stress
> 193 auto quick qgroup enospc limit
> +194 auto metadata log volume
> --
> 2.21.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
2019-10-03 10:59 ` Filipe Manana
@ 2019-10-03 11:12 ` Filipe Manana
2019-10-05 17:26 ` Eryu Guan
0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2019-10-03 11:12 UTC (permalink / raw)
To: Josef Bacik; +Cc: fstests, linux-btrfs, kernel-team
On Thu, Oct 3, 2019 at 11:59 AM Filipe Manana <fdmanana@gmail.com> wrote:
>
> On Wed, Oct 2, 2019 at 7:44 PM Josef Bacik <josef@toxicpanda.com> wrote:
> >
> > I discovered a problem in btrfs where we'd end up pointing at a block we
> > hadn't written out yet. This is triggered by a race when two different
> > files on two different subvolumes fsync. This test exercises this path
> > with dm-log-writes, and then replays the log at every FUA to verify the
> > file system is still mountable and the log is replayable.
> >
> > This test is to verify the fix
> >
> > btrfs: fix incorrect updating of log root tree
> >
> > actually fixed the problem.
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> It's working now.
> Confirmed this triggers the bug, and after about 4 hours of this
> running with the btrfs patch, it doesn't trigger the bug anymore.
>
> Thanks!
>
> > ---
> > v1->v2:
> > - added the patchname related to this test in the comments and changelog.
> > - running fio makes it use 400mib of shared memory, so running 50 of them is
> > impossible on boxes that don't have hundreds of gib of RAM. Fixed this to
> > just generate a fio config so we can run 1 fio instance with 50 threads which
> > makes it not OOM boxes with tiny amounts of RAM.
> > - fixed some formatting things that Filipe pointed out.
> >
> > tests/btrfs/194 | 111 ++++++++++++++++++++++++++++++++++++++++++++
> > tests/btrfs/194.out | 2 +
> > tests/btrfs/group | 1 +
> > 3 files changed, 114 insertions(+)
> > create mode 100755 tests/btrfs/194
> > create mode 100644 tests/btrfs/194.out
> >
> > diff --git a/tests/btrfs/194 b/tests/btrfs/194
> > new file mode 100755
> > index 00000000..b98064e2
> > --- /dev/null
> > +++ b/tests/btrfs/194
> > @@ -0,0 +1,111 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2019 Facebook. All Rights Reserved.
> > +#
> > +# FS QA Test 194
> > +#
> > +# Test multi subvolume fsync to test a bug where we'd end up pointing at a block
> > +# we haven't written. This was fixed by the patch
> > +#
> > +# btrfs: fix incorrect updating of log root tree
> > +#
> > +# Will do log replay and check the filesystem.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +fio_config=$tmp.fio
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + cd /
> > + _log_writes_cleanup &> /dev/null
> > + _dmthin_cleanup
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/dmthin
> > +. ./common/dmlogwrites
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
Btw, only forgot about this.
Should be: _supported_fs btrfs
Eryu can probably fix that at commit time.
Thanks.
> > +_supported_os Linux
> > +
> > +# Use thin device as replay device, which requires $SCRATCH_DEV
> > +_require_scratch_nocheck
> > +# and we need extra device as log device
> > +_require_log_writes
> > +_require_dm_target thin-pool
> > +
> > +cat >$fio_config <<EOF
> > +[global]
> > +readwrite=write
> > +fallocate=none
> > +bs=4k
> > +fsync=1
> > +size=128k
> > +EOF
> > +
> > +for i in $(seq 0 49); do
> > + echo "[foo$i]" >> $fio_config
> > + echo "filename=$SCRATCH_MNT/$i/file" >> $fio_config
> > +done
> > +
> > +_require_fio $fio_config
> > +
> > +cat $fio_config >> $seqres.full
> > +
> > +# Use a thin device to provide deterministic discard behavior. Discards are used
> > +# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
> > +_test_unmount
> > +_dmthin_init $devsize $devsize $csize $lowspace
> > +_log_writes_init $DMTHIN_VOL_DEV
> > +_log_writes_mkfs >> $seqres.full 2>&1
> > +_log_writes_mark mkfs
> > +
> > +_log_writes_mount
> > +
> > +# First create all the subvolumes
> > +for i in $(seq 0 49); do
> > + $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/$i" > /dev/null
> > +done
> > +
> > +$FIO_PROG $fio_config > /dev/null 2>&1
> > +_log_writes_unmount
> > +
> > +_log_writes_remove
> > +prev=$(_log_writes_mark_to_entry_number mkfs)
> > +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
> > +cur=$(_log_writes_find_next_fua $prev)
> > +[ -z "$cur" ] && _fail "failed to locate next FUA write"
> > +
> > +while [ ! -z "$cur" ]; do
> > + _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full
> > +
> > + # We need to mount the fs because btrfsck won't bother checking the log.
> > + _dmthin_mount
> > + _dmthin_check_fs
> > +
> > + prev=$cur
> > + cur=$(_log_writes_find_next_fua $(($cur + 1)))
> > + [ -z "$cur" ] && break
> > +done
> > +
> > +echo "Silence is golden"
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out
> > new file mode 100644
> > index 00000000..7bfd50ff
> > --- /dev/null
> > +++ b/tests/btrfs/194.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 194
> > +Silence is golden
> > diff --git a/tests/btrfs/group b/tests/btrfs/group
> > index b92cb12c..0d0e1bba 100644
> > --- a/tests/btrfs/group
> > +++ b/tests/btrfs/group
> > @@ -196,3 +196,4 @@
> > 191 auto quick send dedupe
> > 192 auto replay snapshot stress
> > 193 auto quick qgroup enospc limit
> > +194 auto metadata log volume
> > --
> > 2.21.0
> >
>
>
> --
> Filipe David Manana,
>
> “Whether you think you can, or you think you can't — you're right.”
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
2019-10-03 11:12 ` Filipe Manana
@ 2019-10-05 17:26 ` Eryu Guan
0 siblings, 0 replies; 5+ messages in thread
From: Eryu Guan @ 2019-10-05 17:26 UTC (permalink / raw)
To: Filipe Manana; +Cc: Josef Bacik, fstests, linux-btrfs, kernel-team
On Thu, Oct 03, 2019 at 12:12:36PM +0100, Filipe Manana wrote:
> On Thu, Oct 3, 2019 at 11:59 AM Filipe Manana <fdmanana@gmail.com> wrote:
> >
> > On Wed, Oct 2, 2019 at 7:44 PM Josef Bacik <josef@toxicpanda.com> wrote:
> > >
> > > I discovered a problem in btrfs where we'd end up pointing at a block we
> > > hadn't written out yet. This is triggered by a race when two different
> > > files on two different subvolumes fsync. This test exercises this path
> > > with dm-log-writes, and then replays the log at every FUA to verify the
> > > file system is still mountable and the log is replayable.
> > >
> > > This test is to verify the fix
> > >
> > > btrfs: fix incorrect updating of log root tree
> > >
> > > actually fixed the problem.
> > >
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> >
> > Reviewed-by: Filipe Manana <fdmanana@suse.com>
> >
> > It's working now.
> > Confirmed this triggers the bug, and after about 4 hours of this
> > running with the btrfs patch, it doesn't trigger the bug anymore.
> >
> > Thanks!
> >
> > > ---
> > > v1->v2:
> > > - added the patchname related to this test in the comments and changelog.
> > > - running fio makes it use 400mib of shared memory, so running 50 of them is
> > > impossible on boxes that don't have hundreds of gib of RAM. Fixed this to
> > > just generate a fio config so we can run 1 fio instance with 50 threads which
> > > makes it not OOM boxes with tiny amounts of RAM.
> > > - fixed some formatting things that Filipe pointed out.
> > >
> > > tests/btrfs/194 | 111 ++++++++++++++++++++++++++++++++++++++++++++
> > > tests/btrfs/194.out | 2 +
> > > tests/btrfs/group | 1 +
> > > 3 files changed, 114 insertions(+)
> > > create mode 100755 tests/btrfs/194
> > > create mode 100644 tests/btrfs/194.out
> > >
> > > diff --git a/tests/btrfs/194 b/tests/btrfs/194
> > > new file mode 100755
> > > index 00000000..b98064e2
> > > --- /dev/null
> > > +++ b/tests/btrfs/194
> > > @@ -0,0 +1,111 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2019 Facebook. All Rights Reserved.
> > > +#
> > > +# FS QA Test 194
> > > +#
> > > +# Test multi subvolume fsync to test a bug where we'd end up pointing at a block
> > > +# we haven't written. This was fixed by the patch
> > > +#
> > > +# btrfs: fix incorrect updating of log root tree
> > > +#
> > > +# Will do log replay and check the filesystem.
> > > +#
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +fio_config=$tmp.fio
> > > +status=1 # failure is the default!
> > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > +
> > > +_cleanup()
> > > +{
> > > + cd /
> > > + _log_writes_cleanup &> /dev/null
> > > + _dmthin_cleanup
> > > + rm -f $tmp.*
> > > +}
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +. ./common/dmthin
> > > +. ./common/dmlogwrites
> > > +
> > > +# remove previous $seqres.full before test
> > > +rm -f $seqres.full
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs generic
>
> Btw, only forgot about this.
> Should be: _supported_fs btrfs
>
> Eryu can probably fix that at commit time.
> Thanks.
Sure. Thanks for the review!
Eryu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
2019-10-02 18:41 [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing Josef Bacik
2019-10-03 10:59 ` Filipe Manana
@ 2019-10-05 17:44 ` Eryu Guan
1 sibling, 0 replies; 5+ messages in thread
From: Eryu Guan @ 2019-10-05 17:44 UTC (permalink / raw)
To: Josef Bacik; +Cc: fstests, linux-btrfs, kernel-team
On Wed, Oct 02, 2019 at 02:41:33PM -0400, Josef Bacik wrote:
> I discovered a problem in btrfs where we'd end up pointing at a block we
> hadn't written out yet. This is triggered by a race when two different
> files on two different subvolumes fsync. This test exercises this path
> with dm-log-writes, and then replays the log at every FUA to verify the
> file system is still mountable and the log is replayable.
>
> This test is to verify the fix
>
> btrfs: fix incorrect updating of log root tree
>
> actually fixed the problem.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
> v1->v2:
> - added the patchname related to this test in the comments and changelog.
> - running fio makes it use 400mib of shared memory, so running 50 of them is
> impossible on boxes that don't have hundreds of gib of RAM. Fixed this to
> just generate a fio config so we can run 1 fio instance with 50 threads which
> makes it not OOM boxes with tiny amounts of RAM.
> - fixed some formatting things that Filipe pointed out.
>
> tests/btrfs/194 | 111 ++++++++++++++++++++++++++++++++++++++++++++
> tests/btrfs/194.out | 2 +
> tests/btrfs/group | 1 +
> 3 files changed, 114 insertions(+)
> create mode 100755 tests/btrfs/194
> create mode 100644 tests/btrfs/194.out
>
> diff --git a/tests/btrfs/194 b/tests/btrfs/194
> new file mode 100755
> index 00000000..b98064e2
> --- /dev/null
> +++ b/tests/btrfs/194
> @@ -0,0 +1,111 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2019 Facebook. All Rights Reserved.
> +#
> +# FS QA Test 194
> +#
> +# Test multi subvolume fsync to test a bug where we'd end up pointing at a block
> +# we haven't written. This was fixed by the patch
> +#
> +# btrfs: fix incorrect updating of log root tree
> +#
> +# Will do log replay and check the filesystem.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +fio_config=$tmp.fio
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + _log_writes_cleanup &> /dev/null
> + _dmthin_cleanup
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmthin
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +
> +# Use thin device as replay device, which requires $SCRATCH_DEV
> +_require_scratch_nocheck
> +# and we need extra device as log device
> +_require_log_writes
> +_require_dm_target thin-pool
> +
> +cat >$fio_config <<EOF
> +[global]
> +readwrite=write
> +fallocate=none
> +bs=4k
> +fsync=1
> +size=128k
> +EOF
> +
> +for i in $(seq 0 49); do
> + echo "[foo$i]" >> $fio_config
> + echo "filename=$SCRATCH_MNT/$i/file" >> $fio_config
> +done
> +
> +_require_fio $fio_config
> +
> +cat $fio_config >> $seqres.full
> +
> +# Use a thin device to provide deterministic discard behavior. Discards are used
> +# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
> +_test_unmount
Why umount $TEST_DEV here?
> +_dmthin_init $devsize $devsize $csize $lowspace
'devsize' 'csize' and 'lowspace' are not defined, and _dmthin_init uses
all defaults. Define them or just use the defaults?
Thanks,
Eryu
> +_log_writes_init $DMTHIN_VOL_DEV
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mark mkfs
> +
> +_log_writes_mount
> +
> +# First create all the subvolumes
> +for i in $(seq 0 49); do
> + $BTRFS_UTIL_PROG subvolume create "$SCRATCH_MNT/$i" > /dev/null
> +done
> +
> +$FIO_PROG $fio_config > /dev/null 2>&1
> +_log_writes_unmount
> +
> +_log_writes_remove
> +prev=$(_log_writes_mark_to_entry_number mkfs)
> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
> +cur=$(_log_writes_find_next_fua $prev)
> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
> +
> +while [ ! -z "$cur" ]; do
> + _log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full
> +
> + # We need to mount the fs because btrfsck won't bother checking the log.
> + _dmthin_mount
> + _dmthin_check_fs
> +
> + prev=$cur
> + cur=$(_log_writes_find_next_fua $(($cur + 1)))
> + [ -z "$cur" ] && break
> +done
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out
> new file mode 100644
> index 00000000..7bfd50ff
> --- /dev/null
> +++ b/tests/btrfs/194.out
> @@ -0,0 +1,2 @@
> +QA output created by 194
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index b92cb12c..0d0e1bba 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -196,3 +196,4 @@
> 191 auto quick send dedupe
> 192 auto replay snapshot stress
> 193 auto quick qgroup enospc limit
> +194 auto metadata log volume
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-05 17:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 18:41 [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing Josef Bacik
2019-10-03 10:59 ` Filipe Manana
2019-10-03 11:12 ` Filipe Manana
2019-10-05 17:26 ` Eryu Guan
2019-10-05 17:44 ` Eryu Guan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).