| Message ID | 1457396879-30572-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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.
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 linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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 --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
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