fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eryu Guan <guaneryu@gmail.com>
To: Filipe Manana <fdmanana@gmail.com>
Cc: Josef Bacik <josef@toxicpanda.com>,
	fstests <fstests@vger.kernel.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	kernel-team@fb.com
Subject: Re: [PATCH][v2] btrfs/194: add a test for multi-subvolume fsyncing
Date: Sun, 6 Oct 2019 01:26:27 +0800	[thread overview]
Message-ID: <20191005172625.GB2622@desktop> (raw)
In-Reply-To: <CAL3q7H47wKpDPGDd4v5-xYsbsSVug3tEeWNxJYSUJ1Cr-8FtTg@mail.gmail.com>

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

  reply	other threads:[~2019-10-05 17:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-10-05 17:44 ` Eryu Guan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191005172625.GB2622@desktop \
    --to=guaneryu@gmail.com \
    --cc=fdmanana@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).