On Sun, Mar 7, 2021 at 3:24 PM Eryu Guan <guan@xxxxxxx> wrote:
>
> On Sun, Mar 07, 2021 at 03:07:43PM +0000, Filipe Manana wrote:
> > On Sun, Mar 7, 2021 at 2:41 PM Eryu Guan <guan@xxxxxxx> wrote:
> > >
> > > On Thu, Feb 11, 2021 at 05:01:18PM +0000, fdmanana@xxxxxxxxxx wrote:
> > > > From: Filipe Manana <fdmanana@xxxxxxxx>
> > > >
> > > > Test cases where a direct IO write, with O_DSYNC, can not be done and has
> > > > to fallback to a buffered write.
> > > >
> > > > This is motivated by a regression that was introduced in kernel 5.10 by
> > > > commit 0eb79294dbe328 ("btrfs: dio iomap DSYNC workaround")) and was
> > > > fixed in kernel 5.11 by commit ecfdc08b8cc65d ("btrfs: remove dio iomap
> > > > DSYNC workaround").
> > > >
> > > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> > >
> > > Sorry for the late review..
> > >
> > > So this is supposed to fail with v5.10 kernel, right? But I got it
> > > passed
> >
> > Because either you are testing with a patched 5.10.x kernel, or you
> > don't have CONFIG_BTRFS_ASSERT=y in your config.
> > The fix landed in 5.10.18:
>
> You're right, I don't have CONFIG_BTRFS_ASSERT=y. As the test dumps the
> od output of the file content, so I thought the failure would be a data
> corruption, and expected a od output diff failure.
I see the test was not merged yet, do you expect me to update anything
in the patch?
Thanks.
>
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.10.18&id=a6703c71153438d3ebdf58a75d53dd5e57b33095
> >
> > >
> > > [root@fedoravm xfstests]# ./check -s btrfs btrfs/231
> > > SECTION -- btrfs
> > > RECREATING -- btrfs on /dev/mapper/testvg-lv1
> > > FSTYP -- btrfs
> > > PLATFORM -- Linux/x86_64 fedoravm 5.10.0 #6 SMP Sun Mar 7 22:25:35 CST 2021
> > > MKFS_OPTIONS -- /dev/mapper/testvg-lv2
> > > MOUNT_OPTIONS -- /dev/mapper/testvg-lv2 /mnt/scratch
> > >
> > > btrfs/231 13s ... 8s
> > > Ran: btrfs/231
> > > Passed all 1 tests
> > >
> > > SECTION -- btrfs
> > > =========================
> > > Ran: btrfs/231
> > > Passed all 1 tests
> > >
> > > > ---
> > > > tests/btrfs/231 | 61 +++++++++++++++++++++++++++++++++++++++++++++
> > > > tests/btrfs/231.out | 21 ++++++++++++++++
> > > > tests/btrfs/group | 1 +
> > > > 3 files changed, 83 insertions(+)
> > > > create mode 100755 tests/btrfs/231
> > > > create mode 100644 tests/btrfs/231.out
> > > >
> > > > diff --git a/tests/btrfs/231 b/tests/btrfs/231
> > > > new file mode 100755
> > > > index 00000000..9a404f57
> > > > --- /dev/null
> > > > +++ b/tests/btrfs/231
> > > > @@ -0,0 +1,61 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (C) 2021 SUSE Linux Products GmbH. All Rights Reserved.
> > > > +#
> > > > +# FS QA Test No. btrfs/231
> > > > +#
> > > > +# Test cases where a direct IO write, with O_DSYNC, can not be done and has to
> > > > +# fallback to a buffered write.
> > > > +#
> > > > +seq=`basename $0`
> > > > +seqres=$RESULT_DIR/$seq
> > > > +echo "QA output created by $seq"
> > > > +
> > > > +tmp=/tmp/$$
> > > > +status=1 # failure is the default!
> > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > +
> > > > +_cleanup()
> > > > +{
> > > > + cd /
> > > > + rm -f $tmp.*
> > > > +}
> > > > +
> > > > +# get standard environment, filters and checks
> > > > +. ./common/rc
> > > > +. ./common/filter
> > > > +. ./common/attr
> > > > +
> > > > +# real QA test starts here
> > > > +_supported_fs btrfs
> > > > +_require_scratch
> > > > +_require_odirect
> > > > +_require_chattr c
> > > > +
> > > > +rm -f $seqres.full
> > > > +
> > > > +_scratch_mkfs >>$seqres.full 2>&1
> > > > +_scratch_mount
> > > > +
> > > > +# First lets test with an attempt to write into a file range with compressed
> > > > +# extents.
> > > > +touch $SCRATCH_MNT/foo
> > > > +$CHATTR_PROG +c $SCRATCH_MNT/foo
> > >
> > > It's not so clear to me why writing into a compressed file is required,
> > > would you please add more comments?
> >
> > The test is meant to test cases where we can deterministically make a
> > direct IO write fallback to buffered IO.
> > There are 2 such cases:
> >
> > 1) Attempting to write to an unaligned offset - this was the bug in
> > 5.10 that resulted in a crash when CONFIG_BTRFS_ASSERT=y (default in
> > many distros, such as openSUSE).
> >
> > 2) Writing to a range that has compressed extents. This has nothing to
> > do with the 5.10 regression, I just added it since there's no existing
> > test that explicitly and deterministically triggers this.
> > So yes, I decided to add a test case for all possible cases of
> > direct IO falling back to buffered instead of adding one just to test
> > a regression (and help detecting any possible future regressions).
>
> Sounds good, thanks!
>
> Eryu
