diff mbox

[v2] fstest: btrfs: test single 4k extent after subpagesize buffered writes

Message ID 1458255398-26833-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Liu Bo March 17, 2016, 10:56 p.m. UTC 
This is to test if COW enabled btrfs can end up with single 4k extents
when doing subpagesize buffered writes.

The patch to fix the problem is
  https://patchwork.kernel.org/patch/8527991/

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: - Teach awk to know system's pagesize.
    - Add "Silence is golden" to output.
    - Use local variables to lower case.
    - Add comments to make code clear.

 tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/027.out |   2 ++
 tests/btrfs/group   |   1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/btrfs/027
 create mode 100644 tests/btrfs/027.out

Comments

Eryu Guan March 22, 2016, 4 a.m. UTC | #1 
On Thu, Mar 17, 2016 at 03:56:38PM -0700, Liu Bo wrote:
> This is to test if COW enabled btrfs can end up with single 4k extents
> when doing subpagesize buffered writes.

What happens if btrfs is mounted with "nodatacow" option? Does it need
to _notrun if cow is disabled?

> 
> The patch to fix the problem is
>   https://patchwork.kernel.org/patch/8527991/
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
> v2: - Teach awk to know system's pagesize.
>     - Add "Silence is golden" to output.
>     - Use local variables to lower case.
>     - Add comments to make code clear.

This should be v3, and this patch was buried in the v2 thread :)

> 
>  tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/027.out |   2 ++
>  tests/btrfs/group   |   1 +
>  3 files changed, 105 insertions(+)
>  create mode 100755 tests/btrfs/027
>  create mode 100644 tests/btrfs/027.out
> 
> diff --git a/tests/btrfs/027 b/tests/btrfs/027
> new file mode 100755
> index 0000000..19d324b
> --- /dev/null
> +++ b/tests/btrfs/027
> @@ -0,0 +1,102 @@
> +#! /bin/bash
> +# FS QA Test 027
> +#
> +# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
> +# single 4k extents.
> +# Ref:
> +#     "Stray 4k extents with slow buffered writes"
> +#     https://www.spinics.net/lists/linux-btrfs/msg52628.html

After going through this thread, my understanding is that nodatacow
btrfs should pass this test even on unpatched kernel (e.g. v4.5). But
my test on v4.5 kernel failed with nodatacow mount option, pagesize
extent is still found.

> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2016 Liu Bo.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +
> +	# restore expire
> +	echo $default_expire > /proc/sys/vm/dirty_expire_centisecs
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +echo "Silence is golden"
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_require_xfs_io_command "fiemap"
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> +# Make it flush dirty pages more frequently to make sure we reproduce the bug.
> +echo 50 > /proc/sys/vm/dirty_expire_centisecs
> +
> +tfile=$SCRATCH_MNT/testfile
> +pagesize=$(get_page_size)
> +sublen=$((RANDOM % pagesize))
> +
> +$XFS_IO_PROG -f -c "pwrite 0 $pagesize" $tfile > /dev/null 2>&1
> +# write some subpagesize data first.
> +$XFS_IO_PROG -c "pwrite $pagesize $sublen" $tfile > /dev/null 2>&1
> +
> +# Mix up "abnormal" subpagesize writes with normal pagesize based writes
> +toff=$((pagesize + sublen))
> +for ((i = 0; i < 10000; i++))
> +do
> +	tlen=$pagesize
> +	if [ $((i % 2)) = 0 ]; then
> +		tlen=$((pagesize * 3))
> +	fi
> +	if [ $((i % 1000)) = 0 ]; then
> +		tlen=$((RANDOM % pagesize))
> +	fi
> +
> +	$XFS_IO_PROG -c "pwrite $toff $tlen" $tfile > /dev/null 2>&1
> +	toff=$((toff + tlen))
> +done

fstests prefers this format:

for ...; do
	...
done

Thanks,
Eryu

> +
> +sync
> +
> +# check for single PAGESIZE extent
> +$XFS_IO_PROG -c "fiemap -v" $tfile >> $seqres.full 2>&1
> +$XFS_IO_PROG -c "fiemap -v" $tfile | \
> +awk -v pgsize=$pagesize '{n = pgsize/512; if ($4 == n) print $4}'
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/027.out b/tests/btrfs/027.out
> new file mode 100644
> index 0000000..ef78cda
> --- /dev/null
> +++ b/tests/btrfs/027.out
> @@ -0,0 +1,2 @@
> +QA output created by 027
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index a2fa412..f06d5fe 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -29,6 +29,7 @@
>  024 auto quick compress
>  025 auto quick send clone
>  026 auto quick compress prealloc
> +027 auto
>  029 auto quick clone
>  030 auto quick send
>  031 auto quick subvol clone
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Bo March 22, 2016, 10:12 p.m. UTC | #2 
On Tue, Mar 22, 2016 at 12:00:13PM +0800, Eryu Guan wrote:
> On Thu, Mar 17, 2016 at 03:56:38PM -0700, Liu Bo wrote:
> > This is to test if COW enabled btrfs can end up with single 4k extents
> > when doing subpagesize buffered writes.
> 
> What happens if btrfs is mounted with "nodatacow" option? Does it need
> to _notrun if cow is disabled?

In my test, the test passes if mounting with "nodatacow".
Yes, it makes sense to have a _notrun for nodatacow.

> 
> > 
> > The patch to fix the problem is
> >   https://patchwork.kernel.org/patch/8527991/
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> > v2: - Teach awk to know system's pagesize.
> >     - Add "Silence is golden" to output.
> >     - Use local variables to lower case.
> >     - Add comments to make code clear.
> 
> This should be v3, and this patch was buried in the v2 thread :)

Oops, thanks for pointing it out.

> 
> > 
> >  tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/027.out |   2 ++
> >  tests/btrfs/group   |   1 +
> >  3 files changed, 105 insertions(+)
> >  create mode 100755 tests/btrfs/027
> >  create mode 100644 tests/btrfs/027.out
> > 
> > diff --git a/tests/btrfs/027 b/tests/btrfs/027
> > new file mode 100755
> > index 0000000..19d324b
> > --- /dev/null
> > +++ b/tests/btrfs/027
> > @@ -0,0 +1,102 @@
> > +#! /bin/bash
> > +# FS QA Test 027
> > +#
> > +# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
> > +# single 4k extents.
> > +# Ref:
> > +#     "Stray 4k extents with slow buffered writes"
> > +#     https://www.spinics.net/lists/linux-btrfs/msg52628.html
> 
> After going through this thread, my understanding is that nodatacow
> btrfs should pass this test even on unpatched kernel (e.g. v4.5). But
> my test on v4.5 kernel failed with nodatacow mount option, pagesize
> extent is still found.
> 

I verified it again on my kvm box and it passed with a unpatched v4.5 kernel.

Can you please show me the 027.full file?

I can't think of a reason for this..

> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2016 Liu Bo.  All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > +#-----------------------------------------------------------------------
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +
> > +	# restore expire
> > +	echo $default_expire > /proc/sys/vm/dirty_expire_centisecs
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +echo "Silence is golden"
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs btrfs
> > +_supported_os Linux
> > +_require_scratch
> > +_require_xfs_io_command "fiemap"
> > +
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> > +# Make it flush dirty pages more frequently to make sure we reproduce the bug.
> > +echo 50 > /proc/sys/vm/dirty_expire_centisecs
> > +
> > +tfile=$SCRATCH_MNT/testfile
> > +pagesize=$(get_page_size)
> > +sublen=$((RANDOM % pagesize))
> > +
> > +$XFS_IO_PROG -f -c "pwrite 0 $pagesize" $tfile > /dev/null 2>&1
> > +# write some subpagesize data first.
> > +$XFS_IO_PROG -c "pwrite $pagesize $sublen" $tfile > /dev/null 2>&1
> > +
> > +# Mix up "abnormal" subpagesize writes with normal pagesize based writes
> > +toff=$((pagesize + sublen))
> > +for ((i = 0; i < 10000; i++))
> > +do
> > +	tlen=$pagesize
> > +	if [ $((i % 2)) = 0 ]; then
> > +		tlen=$((pagesize * 3))
> > +	fi
> > +	if [ $((i % 1000)) = 0 ]; then
> > +		tlen=$((RANDOM % pagesize))
> > +	fi
> > +
> > +	$XFS_IO_PROG -c "pwrite $toff $tlen" $tfile > /dev/null 2>&1
> > +	toff=$((toff + tlen))
> > +done
> 
> fstests prefers this format:
> 
> for ...; do
> 	...
> done

OK, thank you very much, Eryu!

Thanks,

-liubo

> 
> Thanks,
> Eryu
> 
> > +
> > +sync
> > +
> > +# check for single PAGESIZE extent
> > +$XFS_IO_PROG -c "fiemap -v" $tfile >> $seqres.full 2>&1
> > +$XFS_IO_PROG -c "fiemap -v" $tfile | \
> > +awk -v pgsize=$pagesize '{n = pgsize/512; if ($4 == n) print $4}'
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/027.out b/tests/btrfs/027.out
> > new file mode 100644
> > index 0000000..ef78cda
> > --- /dev/null
> > +++ b/tests/btrfs/027.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 027
> > +Silence is golden
> > diff --git a/tests/btrfs/group b/tests/btrfs/group
> > index a2fa412..f06d5fe 100644
> > --- a/tests/btrfs/group
> > +++ b/tests/btrfs/group
> > @@ -29,6 +29,7 @@
> >  024 auto quick compress
> >  025 auto quick send clone
> >  026 auto quick compress prealloc
> > +027 auto
> >  029 auto quick clone
> >  030 auto quick send
> >  031 auto quick subvol clone
> > -- 
> > 2.5.0
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan March 23, 2016, 11:53 a.m. UTC | #3 
On Tue, Mar 22, 2016 at 03:12:25PM -0700, Liu Bo wrote:
> On Tue, Mar 22, 2016 at 12:00:13PM +0800, Eryu Guan wrote:
> > On Thu, Mar 17, 2016 at 03:56:38PM -0700, Liu Bo wrote:
> > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > when doing subpagesize buffered writes.
> > 
> > What happens if btrfs is mounted with "nodatacow" option? Does it need
> > to _notrun if cow is disabled?
> 
> In my test, the test passes if mounting with "nodatacow".
> Yes, it makes sense to have a _notrun for nodatacow.

If "nodatacow" btrfs should pass the test as well, then I don't think
_notrun is needed, so when it failed, something went wrong.

> 
> > 
> > > 
> > > The patch to fix the problem is
> > >   https://patchwork.kernel.org/patch/8527991/
> > > 
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > ---
> > > v2: - Teach awk to know system's pagesize.
> > >     - Add "Silence is golden" to output.
> > >     - Use local variables to lower case.
> > >     - Add comments to make code clear.
> > 
> > This should be v3, and this patch was buried in the v2 thread :)
> 
> Oops, thanks for pointing it out.
> 
> > 
> > > 
> > >  tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/027.out |   2 ++
> > >  tests/btrfs/group   |   1 +
> > >  3 files changed, 105 insertions(+)
> > >  create mode 100755 tests/btrfs/027
> > >  create mode 100644 tests/btrfs/027.out
> > > 
> > > diff --git a/tests/btrfs/027 b/tests/btrfs/027
> > > new file mode 100755
> > > index 0000000..19d324b
> > > --- /dev/null
> > > +++ b/tests/btrfs/027
> > > @@ -0,0 +1,102 @@
> > > +#! /bin/bash
> > > +# FS QA Test 027
> > > +#
> > > +# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
> > > +# single 4k extents.
> > > +# Ref:
> > > +#     "Stray 4k extents with slow buffered writes"
> > > +#     https://www.spinics.net/lists/linux-btrfs/msg52628.html
> > 
> > After going through this thread, my understanding is that nodatacow
> > btrfs should pass this test even on unpatched kernel (e.g. v4.5). But
> > my test on v4.5 kernel failed with nodatacow mount option, pagesize
> > extent is still found.
> > 
> 
> I verified it again on my kvm box and it passed with a unpatched v4.5 kernel.
> 
> Can you please show me the 027.full file?
> 
> I can't think of a reason for this..

I'm using v4.5 kernel and v4.4 btrfs-progs, and it's not reproduced
everytime.

SECTION       -- btrfs_nodatacow
RECREATING    -- btrfs on /dev/sda5
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.5.0
MKFS_OPTIONS  -- /dev/sda6
MOUNT_OPTIONS -- -o nodatacow -o context=system_u:object_r:nfs_t:s0 /dev/sda6 /mnt/testarea/scratch

btrfs/027 28s ... - output mismatch (see /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad)
    --- tests/btrfs/027.out     2016-03-23 15:39:41.562000000 +0800
    +++ /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad       2016-03-23 19:37:38.962000000 +0800
    @@ -1,2 +1,3 @@
     QA output created by 027
     Silence is golden
    +8
    ...
    (Run 'diff -u tests/btrfs/027.out /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad'  to see the entire diff)
Ran: btrfs/027
Failures: btrfs/027
Failed 1 of 1 tests

And btrfs/027.full shows:

/mnt/testarea/scratch/testfile:
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
   0: [0..28863]:      2154496..2183359 28864   0x0
   1: [28864..57751]:  2183360..2212247 28888   0x0
   2: [57752..85543]:  2212248..2240039 27792   0x0
   3: [85544..113239]: 2240040..2267735 27696   0x0
   4: [113240..113247]: 2267736..2267743     8   0x0
   5: [113248..141999]: 2267744..2296495 28752   0x0
   6: [142000..142023]: 2296496..2296519    24   0x0
   7: [142024..159799]: 2296520..2314295 17776   0x1

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Bo March 24, 2016, 4:52 a.m. UTC | #4 
On Wed, Mar 23, 2016 at 07:53:38PM +0800, Eryu Guan wrote:
> On Tue, Mar 22, 2016 at 03:12:25PM -0700, Liu Bo wrote:
> > On Tue, Mar 22, 2016 at 12:00:13PM +0800, Eryu Guan wrote:
> > > On Thu, Mar 17, 2016 at 03:56:38PM -0700, Liu Bo wrote:
> > > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > > when doing subpagesize buffered writes.
> > > 
> > > What happens if btrfs is mounted with "nodatacow" option? Does it need
> > > to _notrun if cow is disabled?
> > 
> > In my test, the test passes if mounting with "nodatacow".
> > Yes, it makes sense to have a _notrun for nodatacow.
> 
> If "nodatacow" btrfs should pass the test as well, then I don't think
> _notrun is needed, so when it failed, something went wrong.

Ok, and it should pass in theory.

> 
> > 
> > > 
> > > > 
> > > > The patch to fix the problem is
> > > >   https://patchwork.kernel.org/patch/8527991/
> > > > 
> > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > > ---
> > > > v2: - Teach awk to know system's pagesize.
> > > >     - Add "Silence is golden" to output.
> > > >     - Use local variables to lower case.
> > > >     - Add comments to make code clear.
> > > 
> > > This should be v3, and this patch was buried in the v2 thread :)
> > 
> > Oops, thanks for pointing it out.
> > 
> > > 
> > > > 
> > > >  tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/btrfs/027.out |   2 ++
> > > >  tests/btrfs/group   |   1 +
> > > >  3 files changed, 105 insertions(+)
> > > >  create mode 100755 tests/btrfs/027
> > > >  create mode 100644 tests/btrfs/027.out
> > > > 
> > > > diff --git a/tests/btrfs/027 b/tests/btrfs/027
> > > > new file mode 100755
> > > > index 0000000..19d324b
> > > > --- /dev/null
> > > > +++ b/tests/btrfs/027
> > > > @@ -0,0 +1,102 @@
> > > > +#! /bin/bash
> > > > +# FS QA Test 027
> > > > +#
> > > > +# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
> > > > +# single 4k extents.
> > > > +# Ref:
> > > > +#     "Stray 4k extents with slow buffered writes"
> > > > +#     https://www.spinics.net/lists/linux-btrfs/msg52628.html
> > > 
> > > After going through this thread, my understanding is that nodatacow
> > > btrfs should pass this test even on unpatched kernel (e.g. v4.5). But
> > > my test on v4.5 kernel failed with nodatacow mount option, pagesize
> > > extent is still found.
> > > 
> > 
> > I verified it again on my kvm box and it passed with a unpatched v4.5 kernel.
> > 
> > Can you please show me the 027.full file?
> > 
> > I can't think of a reason for this..
> 
> I'm using v4.5 kernel and v4.4 btrfs-progs, and it's not reproduced
> everytime.
> 
> SECTION       -- btrfs_nodatacow
> RECREATING    -- btrfs on /dev/sda5
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.5.0
> MKFS_OPTIONS  -- /dev/sda6
> MOUNT_OPTIONS -- -o nodatacow -o context=system_u:object_r:nfs_t:s0 /dev/sda6 /mnt/testarea/scratch
> 
> btrfs/027 28s ... - output mismatch (see /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad)
>     --- tests/btrfs/027.out     2016-03-23 15:39:41.562000000 +0800
>     +++ /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad       2016-03-23 19:37:38.962000000 +0800
>     @@ -1,2 +1,3 @@
>      QA output created by 027
>      Silence is golden
>     +8
>     ...
>     (Run 'diff -u tests/btrfs/027.out /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad'  to see the entire diff)
> Ran: btrfs/027
> Failures: btrfs/027
> Failed 1 of 1 tests
> 
> And btrfs/027.full shows:
> 
> /mnt/testarea/scratch/testfile:
>  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>    0: [0..28863]:      2154496..2183359 28864   0x0
>    1: [28864..57751]:  2183360..2212247 28888   0x0
>    2: [57752..85543]:  2212248..2240039 27792   0x0
>    3: [85544..113239]: 2240040..2267735 27696   0x0
>    4: [113240..113247]: 2267736..2267743     8   0x0
>    5: [113248..141999]: 2267744..2296495 28752   0x0
>    6: [142000..142023]: 2296496..2296519    24   0x0
>    7: [142024..159799]: 2296520..2314295 17776   0x1

I can barely reproduce one in 100 runs... but anyway if it is a bug,
it's not a problem in this test case, I'll send a v3 version patch and
work on this nocow case.

Thanks,

-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Bo March 29, 2016, 2:13 a.m. UTC | #5 
On Wed, Mar 23, 2016 at 09:52:21PM -0700, Liu Bo wrote:
> On Wed, Mar 23, 2016 at 07:53:38PM +0800, Eryu Guan wrote:
> > On Tue, Mar 22, 2016 at 03:12:25PM -0700, Liu Bo wrote:
> > > On Tue, Mar 22, 2016 at 12:00:13PM +0800, Eryu Guan wrote:
> > > > On Thu, Mar 17, 2016 at 03:56:38PM -0700, Liu Bo wrote:
> > > > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > > > when doing subpagesize buffered writes.
> > > > 
> > > > What happens if btrfs is mounted with "nodatacow" option? Does it need
> > > > to _notrun if cow is disabled?
> > > 
> > > In my test, the test passes if mounting with "nodatacow".
> > > Yes, it makes sense to have a _notrun for nodatacow.
> > 
> > If "nodatacow" btrfs should pass the test as well, then I don't think
> > _notrun is needed, so when it failed, something went wrong.
> 
> Ok, and it should pass in theory.
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > The patch to fix the problem is
> > > > >   https://patchwork.kernel.org/patch/8527991/
> > > > > 
> > > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > > > ---
> > > > > v2: - Teach awk to know system's pagesize.
> > > > >     - Add "Silence is golden" to output.
> > > > >     - Use local variables to lower case.
> > > > >     - Add comments to make code clear.
> > > > 
> > > > This should be v3, and this patch was buried in the v2 thread :)
> > > 
> > > Oops, thanks for pointing it out.
> > > 
> > > > 
> > > > > 
> > > > >  tests/btrfs/027     | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  tests/btrfs/027.out |   2 ++
> > > > >  tests/btrfs/group   |   1 +
> > > > >  3 files changed, 105 insertions(+)
> > > > >  create mode 100755 tests/btrfs/027
> > > > >  create mode 100644 tests/btrfs/027.out
> > > > > 
> > > > > diff --git a/tests/btrfs/027 b/tests/btrfs/027
> > > > > new file mode 100755
> > > > > index 0000000..19d324b
> > > > > --- /dev/null
> > > > > +++ b/tests/btrfs/027
> > > > > @@ -0,0 +1,102 @@
> > > > > +#! /bin/bash
> > > > > +# FS QA Test 027
> > > > > +#
> > > > > +# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
> > > > > +# single 4k extents.
> > > > > +# Ref:
> > > > > +#     "Stray 4k extents with slow buffered writes"
> > > > > +#     https://www.spinics.net/lists/linux-btrfs/msg52628.html
> > > > 
> > > > After going through this thread, my understanding is that nodatacow
> > > > btrfs should pass this test even on unpatched kernel (e.g. v4.5). But
> > > > my test on v4.5 kernel failed with nodatacow mount option, pagesize
> > > > extent is still found.
> > > > 
> > > 
> > > I verified it again on my kvm box and it passed with a unpatched v4.5 kernel.
> > > 
> > > Can you please show me the 027.full file?
> > > 
> > > I can't think of a reason for this..
> > 
> > I'm using v4.5 kernel and v4.4 btrfs-progs, and it's not reproduced
> > everytime.
> > 
> > SECTION       -- btrfs_nodatacow
> > RECREATING    -- btrfs on /dev/sda5
> > FSTYP         -- btrfs
> > PLATFORM      -- Linux/x86_64 dhcp-66-86-11 4.5.0
> > MKFS_OPTIONS  -- /dev/sda6
> > MOUNT_OPTIONS -- -o nodatacow -o context=system_u:object_r:nfs_t:s0 /dev/sda6 /mnt/testarea/scratch
> > 
> > btrfs/027 28s ... - output mismatch (see /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad)
> >     --- tests/btrfs/027.out     2016-03-23 15:39:41.562000000 +0800
> >     +++ /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad       2016-03-23 19:37:38.962000000 +0800
> >     @@ -1,2 +1,3 @@
> >      QA output created by 027
> >      Silence is golden
> >     +8
> >     ...
> >     (Run 'diff -u tests/btrfs/027.out /root/xfstests/results//btrfs_nodatacow/btrfs/027.out.bad'  to see the entire diff)
> > Ran: btrfs/027
> > Failures: btrfs/027
> > Failed 1 of 1 tests
> > 
> > And btrfs/027.full shows:
> > 
> > /mnt/testarea/scratch/testfile:
> >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
> >    0: [0..28863]:      2154496..2183359 28864   0x0
> >    1: [28864..57751]:  2183360..2212247 28888   0x0
> >    2: [57752..85543]:  2212248..2240039 27792   0x0
> >    3: [85544..113239]: 2240040..2267735 27696   0x0
> >    4: [113240..113247]: 2267736..2267743     8   0x0
> >    5: [113248..141999]: 2267744..2296495 28752   0x0
> >    6: [142000..142023]: 2296496..2296519    24   0x0
> >    7: [142024..159799]: 2296520..2314295 17776   0x1
> 
> I can barely reproduce one in 100 runs... but anyway if it is a bug,
> it's not a problem in this test case, I'll send a v3 version patch and
> work on this nocow case.

My trace results show that it's not a bug.

[0, 4096]
[4096, 8192]
...
[N-4096, N]
[N, N+4096]
[N+4096, N+8192]
...

There could be some latencies between writes against [N, N+4096] and writes against [N+4096, N+8192],
so when writeback starts between [N-4096, N] and [N, N+4096], btrfs will
find delayed allocation range ending at extent [N-4096, N], and then it
creates a extent to cover that range.  Later [N, N+4096] is dirtied and
writeback thread will continue pick up the newly dirtied page and
allocate an extent for it, at which time the next [N+4096, N+8192] write
has not performed.  This is how we get the single 4k extent for
nodatacow, but it's rare.

So I assume that even after fixing writeback index, we can still experience
this case in case of cow, but that should be quite rare, too.

Thanks,

-liubo

> 
> Thanks,
> 
> -liubo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/btrfs/027 b/tests/btrfs/027
new file mode 100755
index 0000000..19d324b
--- /dev/null
+++ b/tests/btrfs/027
@@ -0,0 +1,102 @@ 
+#! /bin/bash
+# FS QA Test 027
+#
+# When btrfs is using cow mode, buffered writes of sub-pagesize can end up with
+# single 4k extents.
+# Ref:
+#     "Stray 4k extents with slow buffered writes"
+#     https://www.spinics.net/lists/linux-btrfs/msg52628.html
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016 Liu Bo.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+
+	# restore expire
+	echo $default_expire > /proc/sys/vm/dirty_expire_centisecs
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+echo "Silence is golden"
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "fiemap"
+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount
+
+default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
+# Make it flush dirty pages more frequently to make sure we reproduce the bug.
+echo 50 > /proc/sys/vm/dirty_expire_centisecs
+
+tfile=$SCRATCH_MNT/testfile
+pagesize=$(get_page_size)
+sublen=$((RANDOM % pagesize))
+
+$XFS_IO_PROG -f -c "pwrite 0 $pagesize" $tfile > /dev/null 2>&1
+# write some subpagesize data first.
+$XFS_IO_PROG -c "pwrite $pagesize $sublen" $tfile > /dev/null 2>&1
+
+# Mix up "abnormal" subpagesize writes with normal pagesize based writes
+toff=$((pagesize + sublen))
+for ((i = 0; i < 10000; i++))
+do
+	tlen=$pagesize
+	if [ $((i % 2)) = 0 ]; then
+		tlen=$((pagesize * 3))
+	fi
+	if [ $((i % 1000)) = 0 ]; then
+		tlen=$((RANDOM % pagesize))
+	fi
+
+	$XFS_IO_PROG -c "pwrite $toff $tlen" $tfile > /dev/null 2>&1
+	toff=$((toff + tlen))
+done
+
+sync
+
+# check for single PAGESIZE extent
+$XFS_IO_PROG -c "fiemap -v" $tfile >> $seqres.full 2>&1
+$XFS_IO_PROG -c "fiemap -v" $tfile | \
+awk -v pgsize=$pagesize '{n = pgsize/512; if ($4 == n) print $4}'
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/027.out b/tests/btrfs/027.out
new file mode 100644
index 0000000..ef78cda
--- /dev/null
+++ b/tests/btrfs/027.out
@@ -0,0 +1,2 @@ 
+QA output created by 027
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index a2fa412..f06d5fe 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -29,6 +29,7 @@ 
 024 auto quick compress
 025 auto quick send clone
 026 auto quick compress prealloc
+027 auto
 029 auto quick clone
 030 auto quick send
 031 auto quick subvol clone