diff mbox

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

Message ID 1457396879-30572-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo March 8, 2016, 12:27 a.m. UTC
This is to test if COW enabled btrfs can end up with single 4k extents
when doing subpagesize buffered writes.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: - Fix 027.out to make sure we don't get single 4k extents.
    - Add the original mail list discussion as the reference.

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

Comments

Dave Chinner March 15, 2016, 3:39 a.m. UTC | #1
On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> This is to test if COW enabled btrfs can end up with single 4k extents
> when doing subpagesize buffered writes.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
....
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> +echo 50 > /proc/sys/vm/dirty_expire_centisecs

why?

> +tfile=$SCRATCH_MNT/testfile
> +
> +PAGE_SIZE=$(get_page_size)
> +SUBLEN=$((RANDOM % PAGE_SIZE))

local variables are lower case.

> +$XFS_IO_PROG -f -c "pwrite 0 $PAGE_SIZE" $tfile > /dev/null 2>&1
> +$XFS_IO_PROG -c "pwrite $PAGE_SIZE $SUBLEN" $tfile > /dev/null 2>&1
> +
> +toff=$((PAGE_SIZE + SUBLEN))
> +for ((i=0; i<10000; i++))
> +do

# Add comment on what the loop is trying to do to the layout of the
# file being written.
for ((i = 0; i < 10000; i++)); do

> +	tlen=$PAGE_SIZE
> +	if [ $((i % 2)) = 0 ]; then
> +		tlen=$((PAGE_SIZE * 3))
> +	fi
> +	if [ $((i % 1000)) = 0 ]; then
> +		tlen=$((RANDOM % PAGE_SIZE))
> +	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 '{if ($4 == 8) print $4}'

Assumes page size is 4k. Also assumes that no individual extent in
the file is 4k. Likely broken.

> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/027.out b/tests/btrfs/027.out
> new file mode 100644
> index 0000000..e8291ab
> --- /dev/null
> +++ b/tests/btrfs/027.out
> @@ -0,0 +1 @@
> +QA output created by 027

So are we expecting no output or not?

Cheers,

Dave.
Liu Bo March 17, 2016, 4:13 a.m. UTC | #2
On Tue, Mar 15, 2016 at 02:39:41PM +1100, Dave Chinner wrote:
> On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> > This is to test if COW enabled btrfs can end up with single 4k extents
> > when doing subpagesize buffered writes.
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > ---
> ....
> > +_scratch_mkfs >> $seqres.full 2>&1
> > +_scratch_mount
> > +
> > +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> > +echo 50 > /proc/sys/vm/dirty_expire_centisecs
> 
> why?

Setting it to 50 is to flush dirty pages more frequently so that it's
more likely to reproduce this bug.

But I just realized that this may not be a good testcase for xfstests as
there're many factors impacting writeback flushing, which means this may
work well on my machine but may not trigger the bug on others' machines.

> 
> > +tfile=$SCRATCH_MNT/testfile
> > +
> > +PAGE_SIZE=$(get_page_size)
> > +SUBLEN=$((RANDOM % PAGE_SIZE))
> 
> local variables are lower case.

OK.

> 
> > +$XFS_IO_PROG -f -c "pwrite 0 $PAGE_SIZE" $tfile > /dev/null 2>&1
> > +$XFS_IO_PROG -c "pwrite $PAGE_SIZE $SUBLEN" $tfile > /dev/null 2>&1
> > +
> > +toff=$((PAGE_SIZE + SUBLEN))
> > +for ((i=0; i<10000; i++))
> > +do
> 
> # Add comment on what the loop is trying to do to the layout of the
> # file being written.
> for ((i = 0; i < 10000; i++)); do

OK.

> 
> > +	tlen=$PAGE_SIZE
> > +	if [ $((i % 2)) = 0 ]; then
> > +		tlen=$((PAGE_SIZE * 3))
> > +	fi
> > +	if [ $((i % 1000)) = 0 ]; then
> > +		tlen=$((RANDOM % PAGE_SIZE))
> > +	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 '{if ($4 == 8) print $4}'
> 
> Assumes page size is 4k. Also assumes that no individual extent in
> the file is 4k. Likely broken.

My miss, will try to come up with a way to tell awk PAGE_SIZE, maybe call getpagesize() ?
(if you already how to do it, please do me a favor :-) )

> 
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/027.out b/tests/btrfs/027.out
> > new file mode 100644
> > index 0000000..e8291ab
> > --- /dev/null
> > +++ b/tests/btrfs/027.out
> > @@ -0,0 +1 @@
> > +QA output created by 027
> 
> So are we expecting no output or not?

We don't expect any 4k single extent if btrfs is doing correctly, so I leave the output empty.

Thanks for the comments, Dave!

Thanks,

-liubo
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe fstests" 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 17, 2016, 4:24 a.m. UTC | #3
On Wed, Mar 16, 2016 at 09:13:57PM -0700, Liu Bo wrote:
> On Tue, Mar 15, 2016 at 02:39:41PM +1100, Dave Chinner wrote:
> > On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > when doing subpagesize buffered writes.
> > > 
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
...
> > > +# check for single PAGESIZE extent
> > > +$XFS_IO_PROG -c "fiemap -v" $tfile >> $seqres.full 2>&1
> > > +$XFS_IO_PROG -c "fiemap -v" $tfile | awk '{if ($4 == 8) print $4}'
> > 
> > Assumes page size is 4k. Also assumes that no individual extent in
> > the file is 4k. Likely broken.
> 
> My miss, will try to come up with a way to tell awk PAGE_SIZE, maybe call getpagesize() ?
> (if you already how to do it, please do me a favor :-) )

./src/feature is able to do this, e.g. pagesize=`$here/src/feature -s`

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner March 17, 2016, 9:07 p.m. UTC | #4
On Wed, Mar 16, 2016 at 09:13:57PM -0700, Liu Bo wrote:
> On Tue, Mar 15, 2016 at 02:39:41PM +1100, Dave Chinner wrote:
> > On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > when doing subpagesize buffered writes.
> > > 
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > ---
> > ....
> > > +_scratch_mkfs >> $seqres.full 2>&1
> > > +_scratch_mount
> > > +
> > > +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> > > +echo 50 > /proc/sys/vm/dirty_expire_centisecs
> > 
> > why?
> 
> Setting it to 50 is to flush dirty pages more frequently so that it's
> more likely to reproduce this bug.

Explainations should be in comments.

....
> > > +	$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 '{if ($4 == 8) print $4}'
> > 
> > Assumes page size is 4k. Also assumes that no individual extent in
> > the file is 4k. Likely broken.
> 
> My miss, will try to come up with a way to tell awk PAGE_SIZE, maybe call getpagesize() ?
> (if you already how to do it, please do me a favor :-) )

awk -v pgsize=$page_size \
	'{ cnt = pgsize/512; if ($4 == pgsize) print $4; }

Assuming that fiemap is reporting in 512 byte block size, not sector
sizes...

> > > --- /dev/null
> > > +++ b/tests/btrfs/027.out
> > > @@ -0,0 +1 @@
> > > +QA output created by 027
> > 
> > So are we expecting no output or not?
> 
> We don't expect any 4k single extent if btrfs is doing correctly, so I leave the output empty.

Convention is that we echo a line to the golden output file to
document the test is expected to give no output. That's why you see

echo "Silence is golden"

in many tests...

Cheers,

Dave.
Liu Bo March 17, 2016, 9:31 p.m. UTC | #5
On Fri, Mar 18, 2016 at 08:07:15AM +1100, Dave Chinner wrote:
> On Wed, Mar 16, 2016 at 09:13:57PM -0700, Liu Bo wrote:
> > On Tue, Mar 15, 2016 at 02:39:41PM +1100, Dave Chinner wrote:
> > > On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> > > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > > when doing subpagesize buffered writes.
> > > > 
> > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > > > ---
> > > ....
> > > > +_scratch_mkfs >> $seqres.full 2>&1
> > > > +_scratch_mount
> > > > +
> > > > +default_expire=`cat /proc/sys/vm/dirty_expire_centisecs`
> > > > +echo 50 > /proc/sys/vm/dirty_expire_centisecs
> > > 
> > > why?
> > 
> > Setting it to 50 is to flush dirty pages more frequently so that it's
> > more likely to reproduce this bug.
> 
> Explainations should be in comments.

Got it, that makes sense.

> 
> ....
> > > > +	$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 '{if ($4 == 8) print $4}'
> > > 
> > > Assumes page size is 4k. Also assumes that no individual extent in
> > > the file is 4k. Likely broken.
> > 
> > My miss, will try to come up with a way to tell awk PAGE_SIZE, maybe call getpagesize() ?
> > (if you already how to do it, please do me a favor :-) )
> 
> awk -v pgsize=$page_size \
> 	'{ cnt = pgsize/512; if ($4 == pgsize) print $4; }
> 
> Assuming that fiemap is reporting in 512 byte block size, not sector
> sizes...

Yeah, I noticed that.

> 
> > > > --- /dev/null
> > > > +++ b/tests/btrfs/027.out
> > > > @@ -0,0 +1 @@
> > > > +QA output created by 027
> > > 
> > > So are we expecting no output or not?
> > 
> > We don't expect any 4k single extent if btrfs is doing correctly, so I leave the output empty.
> 
> Convention is that we echo a line to the golden output file to
> document the test is expected to give no output. That's why you see
> 
> echo "Silence is golden"
> 
> in many tests...

I see, will update it.

Thank you for the nice explanation.

Thanks,

-liubo
--
To unsubscribe from this list: send the line "unsubscribe fstests" 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 17, 2016, 10:23 p.m. UTC | #6
On Thu, Mar 17, 2016 at 12:24:35PM +0800, Eryu Guan wrote:
> On Wed, Mar 16, 2016 at 09:13:57PM -0700, Liu Bo wrote:
> > On Tue, Mar 15, 2016 at 02:39:41PM +1100, Dave Chinner wrote:
> > > On Mon, Mar 07, 2016 at 04:27:59PM -0800, Liu Bo wrote:
> > > > This is to test if COW enabled btrfs can end up with single 4k extents
> > > > when doing subpagesize buffered writes.
> > > > 
> > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ...
> > > > +# check for single PAGESIZE extent
> > > > +$XFS_IO_PROG -c "fiemap -v" $tfile >> $seqres.full 2>&1
> > > > +$XFS_IO_PROG -c "fiemap -v" $tfile | awk '{if ($4 == 8) print $4}'
> > > 
> > > Assumes page size is 4k. Also assumes that no individual extent in
> > > the file is 4k. Likely broken.
> > 
> > My miss, will try to come up with a way to tell awk PAGE_SIZE, maybe call getpagesize() ?
> > (if you already how to do it, please do me a favor :-) )
> 
> ./src/feature is able to do this, e.g. pagesize=`$here/src/feature -s`

It works, thanks for reminding me of that.

Thanks,

-liubo
--
To unsubscribe from this list: send the line "unsubscribe fstests" 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..820973d
--- /dev/null
+++ b/tests/btrfs/027
@@ -0,0 +1,97 @@ 
+#! /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
+
+# 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`
+echo 50 > /proc/sys/vm/dirty_expire_centisecs
+tfile=$SCRATCH_MNT/testfile
+
+PAGE_SIZE=$(get_page_size)
+SUBLEN=$((RANDOM % PAGE_SIZE))
+
+$XFS_IO_PROG -f -c "pwrite 0 $PAGE_SIZE" $tfile > /dev/null 2>&1
+$XFS_IO_PROG -c "pwrite $PAGE_SIZE $SUBLEN" $tfile > /dev/null 2>&1
+
+toff=$((PAGE_SIZE + SUBLEN))
+for ((i=0; i<10000; i++))
+do
+	tlen=$PAGE_SIZE
+	if [ $((i % 2)) = 0 ]; then
+		tlen=$((PAGE_SIZE * 3))
+	fi
+	if [ $((i % 1000)) = 0 ]; then
+		tlen=$((RANDOM % PAGE_SIZE))
+	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 '{if ($4 == 8) 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..e8291ab
--- /dev/null
+++ b/tests/btrfs/027.out
@@ -0,0 +1 @@ 
+QA output created by 027
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