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@xxxxxxxxx> wrote:
> >
> > On Wed, Oct 2, 2019 at 7:44 PM Josef Bacik <josef@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> >
> > Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > 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
