| Message ID | 20170403070923.18518-1-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Mon, Apr 03, 2017 at 03:09:23PM +0800, Qu Wenruo wrote: > As long as we don't modify the on-disk data, fiemap result should always > be constant. > > Operation like cycle mount and sleep should not affect fiemap result. > While unfortunately, btrfs doesn't follow that behavior. > > Btrfs fiemap sometimes return merged result, while after cycle mount, it > returns split result. Furthermore after a snap, btrfs returns merged > result again. > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > tests/generic/422 | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/422.out | 2 + > tests/generic/group | 1 + > 3 files changed, 130 insertions(+) > create mode 100755 tests/generic/422 > create mode 100644 tests/generic/422.out > > diff --git a/tests/generic/422 b/tests/generic/422 > new file mode 100755 > index 0000000..4ca4476 > --- /dev/null > +++ b/tests/generic/422 > @@ -0,0 +1,127 @@ > +#! /bin/bash > +# FS QA Test 422 > +# > +# Test if a file system returns constant fiemap result after remount and > +# fiemap. > +# Unfortunately, btrfs doesn't follow this behavior. Is this test a goalpost for btrfs bugfixes you're going to post? > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Fujitsu. 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.* > +} > + > +# 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 generic > +_supported_os IRIX Linux I don't think we need to support IRIX... --D > +_require_scratch > + > +block_size=$((64 * 1024)) > +block_count=32 > +dst=$SCRATCH_MNT/file > +sleeptime=3 > + > +# It's almost 100% for btrfs to trigger inconstant fiemap result > +# just in case > +runtime=$((2 * $LOAD_FACTOR)) > + > +# record fiemap as checkpoint, and output the hash of fiemap result > +# to stdout > +fiemap_checkpoint() > +{ > + local number=$1 > + local message=$2 > + > + echo "=== $message ===" >> $seqres.full > + $XFS_IO_PROG -c "fiemap -v" $dst > ${tmp}.cp$number > + cat ${tmp}.cp${number} >> $seqres.full > + > + md5sum ${tmp}.cp${number} | cut -d ' ' -f 1 > +} > + > +do_test() > +{ > + local number=$1 > + > + # Use 16 times of file size to ensure we have enough space > + _scratch_mkfs_sized $((16 * $block_size * $block_count)) \ > + > /dev/null 2>&1 > + _scratch_mount > + > + echo "====== Loop $number ======" >> $seqres.full > + touch $dst > + # Xfsprogs 4.9.0 still has a bug that xfs_io "open" with O_SYNC command > + # doesn't work well with "pwrite", although it gets fixed in v4.10.0, > + # use dd here to avoid it won't hurt for non-xfs developers > + dd if=/dev/zero of=$dst bs=$block_size count=$block_count oflag=dsync \ > + status=none 2>&1 > + > + hash1=$(fiemap_checkpoint 1 "Fiemap just after dsync write") > + > + # Sleep should not modify fiemap result > + sleep $sleeptime > + > + hash2=$(fiemap_checkpoint 2 "Fiemap after dsync write and sleep") > + > + # cycle mount should not modify fiemap result > + _scratch_cycle_mount > + > + hash3=$(fiemap_checkpoint 3 "Fiemap after cycle mount") > + > + # Sleep should not modify fiemap result > + sleep $sleeptime > + > + hash4=$(fiemap_checkpoint 4 "Fiemap after cycle mount and sleep") > + > + _scratch_unmount > + > + if [ $hash1 != $hash2 -o $hash2 != $hash3 -o $hash3 != $hash4 ]; then > + echo "Inconstant fiemap result detected" > + fi > + echo >> $seqres.full > +} > + > +for i in $(seq 1 $runtime); do > + do_test $i > +done > + > +echo "Silence is golden" > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/422.out b/tests/generic/422.out > new file mode 100644 > index 0000000..f70693f > --- /dev/null > +++ b/tests/generic/422.out > @@ -0,0 +1,2 @@ > +QA output created by 422 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index 3c7c5e4..86d2325 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -424,3 +424,4 @@ > 419 auto quick encrypt > 420 auto quick punch > 421 auto quick encrypt dangerous > +422 auto > -- > 2.9.3 > > > > -- > 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 -- 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
At 04/04/2017 12:31 AM, Darrick J. Wong wrote: > On Mon, Apr 03, 2017 at 03:09:23PM +0800, Qu Wenruo wrote: >> As long as we don't modify the on-disk data, fiemap result should always >> be constant. >> >> Operation like cycle mount and sleep should not affect fiemap result. >> While unfortunately, btrfs doesn't follow that behavior. >> >> Btrfs fiemap sometimes return merged result, while after cycle mount, it >> returns split result. Furthermore after a snap, btrfs returns merged >> result again. >> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> --- >> tests/generic/422 | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/422.out | 2 + >> tests/generic/group | 1 + >> 3 files changed, 130 insertions(+) >> create mode 100755 tests/generic/422 >> create mode 100644 tests/generic/422.out >> >> diff --git a/tests/generic/422 b/tests/generic/422 >> new file mode 100755 >> index 0000000..4ca4476 >> --- /dev/null >> +++ b/tests/generic/422 >> @@ -0,0 +1,127 @@ >> +#! /bin/bash >> +# FS QA Test 422 >> +# >> +# Test if a file system returns constant fiemap result after remount and >> +# fiemap. >> +# Unfortunately, btrfs doesn't follow this behavior. > > Is this test a goalpost for btrfs bugfixes you're going to post? Yes. > >> +# >> +#----------------------------------------------------------------------- >> +# Copyright (c) 2017 Fujitsu. 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.* >> +} >> + >> +# 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 generic >> +_supported_os IRIX Linux > > I don't think we need to support IRIX... Could we just remove IRIX from the template? Thanks, Qu > > --D > >> +_require_scratch >> + >> +block_size=$((64 * 1024)) >> +block_count=32 >> +dst=$SCRATCH_MNT/file >> +sleeptime=3 >> + >> +# It's almost 100% for btrfs to trigger inconstant fiemap result >> +# just in case >> +runtime=$((2 * $LOAD_FACTOR)) >> + >> +# record fiemap as checkpoint, and output the hash of fiemap result >> +# to stdout >> +fiemap_checkpoint() >> +{ >> + local number=$1 >> + local message=$2 >> + >> + echo "=== $message ===" >> $seqres.full >> + $XFS_IO_PROG -c "fiemap -v" $dst > ${tmp}.cp$number >> + cat ${tmp}.cp${number} >> $seqres.full >> + >> + md5sum ${tmp}.cp${number} | cut -d ' ' -f 1 >> +} >> + >> +do_test() >> +{ >> + local number=$1 >> + >> + # Use 16 times of file size to ensure we have enough space >> + _scratch_mkfs_sized $((16 * $block_size * $block_count)) \ >> + > /dev/null 2>&1 >> + _scratch_mount >> + >> + echo "====== Loop $number ======" >> $seqres.full >> + touch $dst >> + # Xfsprogs 4.9.0 still has a bug that xfs_io "open" with O_SYNC command >> + # doesn't work well with "pwrite", although it gets fixed in v4.10.0, >> + # use dd here to avoid it won't hurt for non-xfs developers >> + dd if=/dev/zero of=$dst bs=$block_size count=$block_count oflag=dsync \ >> + status=none 2>&1 >> + >> + hash1=$(fiemap_checkpoint 1 "Fiemap just after dsync write") >> + >> + # Sleep should not modify fiemap result >> + sleep $sleeptime >> + >> + hash2=$(fiemap_checkpoint 2 "Fiemap after dsync write and sleep") >> + >> + # cycle mount should not modify fiemap result >> + _scratch_cycle_mount >> + >> + hash3=$(fiemap_checkpoint 3 "Fiemap after cycle mount") >> + >> + # Sleep should not modify fiemap result >> + sleep $sleeptime >> + >> + hash4=$(fiemap_checkpoint 4 "Fiemap after cycle mount and sleep") >> + >> + _scratch_unmount >> + >> + if [ $hash1 != $hash2 -o $hash2 != $hash3 -o $hash3 != $hash4 ]; then >> + echo "Inconstant fiemap result detected" >> + fi >> + echo >> $seqres.full >> +} >> + >> +for i in $(seq 1 $runtime); do >> + do_test $i >> +done >> + >> +echo "Silence is golden" >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/generic/422.out b/tests/generic/422.out >> new file mode 100644 >> index 0000000..f70693f >> --- /dev/null >> +++ b/tests/generic/422.out >> @@ -0,0 +1,2 @@ >> +QA output created by 422 >> +Silence is golden >> diff --git a/tests/generic/group b/tests/generic/group >> index 3c7c5e4..86d2325 100644 >> --- a/tests/generic/group >> +++ b/tests/generic/group >> @@ -424,3 +424,4 @@ >> 419 auto quick encrypt >> 420 auto quick punch >> 421 auto quick encrypt dangerous >> +422 auto >> -- >> 2.9.3 >> >> >> >> -- >> 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 > > -- 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 Mon, Apr 03, 2017 at 03:09:23PM +0800, Qu Wenruo wrote: > As long as we don't modify the on-disk data, fiemap result should always > be constant. > > Operation like cycle mount and sleep should not affect fiemap result. > While unfortunately, btrfs doesn't follow that behavior. > > Btrfs fiemap sometimes return merged result, while after cycle mount, it > returns split result. Furthermore after a snap, btrfs returns merged > result again. > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> Test fails with ext3/2 when driving with ext4 driver, fiemap changed after umount/mount cycle, then changed back to original result after sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) === Fiemap after dsync write and sleep === /vda6/file: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..127]: 12288..12415 128 0x1000 1: [128..255]: 12160..12287 128 0x1000 2: [256..511]: 16384..16639 256 0x1000 3: [512..2047]: 16896..18431 1536 0x1000 4: [2048..4095]: 19456..21503 2048 0x1001 === Fiemap after cycle mount === /vda6/file: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..95]: 12288..12383 96 0x1000 1: [96..127]: 12384..12415 32 0x1000 [0-127] was split to two extents 2: [128..255]: 12160..12287 128 0x1000 3: [256..511]: 16384..16639 256 0x1000 4: [512..2047]: 16896..18431 1536 0x1000 5: [2048..4095]: 19456..21503 2048 0x1001 === Fiemap after cycle mount and sleep === /vda6/file: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..127]: 12288..12415 128 0x1000 1: [128..255]: 12160..12287 128 0x1000 2: [256..511]: 16384..16639 256 0x1000 3: [512..2047]: 16896..18431 1536 0x1000 4: [2048..4095]: 19456..21503 2048 0x1001 > --- > tests/generic/422 | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/422.out | 2 + > tests/generic/group | 1 + > 3 files changed, 130 insertions(+) > create mode 100755 tests/generic/422 > create mode 100644 tests/generic/422.out > > diff --git a/tests/generic/422 b/tests/generic/422 > new file mode 100755 > index 0000000..4ca4476 > --- /dev/null > +++ b/tests/generic/422 > @@ -0,0 +1,127 @@ > +#! /bin/bash > +# FS QA Test 422 > +# > +# Test if a file system returns constant fiemap result after remount and > +# fiemap. > +# Unfortunately, btrfs doesn't follow this behavior. > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 Fujitsu. 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.* > +} > + > +# 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 generic > +_supported_os IRIX Linux > +_require_scratch Need _require_fiemap > + > +block_size=$((64 * 1024)) > +block_count=32 > +dst=$SCRATCH_MNT/file > +sleeptime=3 > + > +# It's almost 100% for btrfs to trigger inconstant fiemap result Trailing whitespace in above line. > +# just in case > +runtime=$((2 * $LOAD_FACTOR)) I don't think this loop is necessary, as the comments state, it's almost 100% reproducible for btrfs (and for ext2/3 in my testing). > + > +# record fiemap as checkpoint, and output the hash of fiemap result > +# to stdout > +fiemap_checkpoint() > +{ > + local number=$1 > + local message=$2 > + > + echo "=== $message ===" >> $seqres.full > + $XFS_IO_PROG -c "fiemap -v" $dst > ${tmp}.cp$number > + cat ${tmp}.cp${number} >> $seqres.full > + > + md5sum ${tmp}.cp${number} | cut -d ' ' -f 1 > +} > + > +do_test() > +{ > + local number=$1 > + > + # Use 16 times of file size to ensure we have enough space > + _scratch_mkfs_sized $((16 * $block_size * $block_count)) \ Use _require_fs_space for required space. Not all filesystems have _scratch_mkfs_sized support so this could potentially reduce the filesystem coverage. > + > /dev/null 2>&1 > + _scratch_mount Trailing whitespace issue. Thanks, Eryu > + > + echo "====== Loop $number ======" >> $seqres.full > + touch $dst > + # Xfsprogs 4.9.0 still has a bug that xfs_io "open" with O_SYNC command > + # doesn't work well with "pwrite", although it gets fixed in v4.10.0, > + # use dd here to avoid it won't hurt for non-xfs developers > + dd if=/dev/zero of=$dst bs=$block_size count=$block_count oflag=dsync \ > + status=none 2>&1 > + > + hash1=$(fiemap_checkpoint 1 "Fiemap just after dsync write") > + > + # Sleep should not modify fiemap result > + sleep $sleeptime > + > + hash2=$(fiemap_checkpoint 2 "Fiemap after dsync write and sleep") > + > + # cycle mount should not modify fiemap result > + _scratch_cycle_mount > + > + hash3=$(fiemap_checkpoint 3 "Fiemap after cycle mount") > + > + # Sleep should not modify fiemap result > + sleep $sleeptime > + > + hash4=$(fiemap_checkpoint 4 "Fiemap after cycle mount and sleep") > + > + _scratch_unmount > + > + if [ $hash1 != $hash2 -o $hash2 != $hash3 -o $hash3 != $hash4 ]; then > + echo "Inconstant fiemap result detected" > + fi > + echo >> $seqres.full > +} > + > +for i in $(seq 1 $runtime); do > + do_test $i > +done > + > +echo "Silence is golden" > +# success, all done > +status=0 > +exit > diff --git a/tests/generic/422.out b/tests/generic/422.out > new file mode 100644 > index 0000000..f70693f > --- /dev/null > +++ b/tests/generic/422.out > @@ -0,0 +1,2 @@ > +QA output created by 422 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index 3c7c5e4..86d2325 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -424,3 +424,4 @@ > 419 auto quick encrypt > 420 auto quick punch > 421 auto quick encrypt dangerous > +422 auto > -- > 2.9.3 > > > > -- > 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 -- 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
At 04/05/2017 10:35 AM, Eryu Guan wrote: > On Mon, Apr 03, 2017 at 03:09:23PM +0800, Qu Wenruo wrote: >> As long as we don't modify the on-disk data, fiemap result should always >> be constant. >> >> Operation like cycle mount and sleep should not affect fiemap result. >> While unfortunately, btrfs doesn't follow that behavior. >> >> Btrfs fiemap sometimes return merged result, while after cycle mount, it >> returns split result. Furthermore after a snap, btrfs returns merged >> result again. >> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > > Test fails with ext3/2 when driving with ext4 driver, fiemap changed > after umount/mount cycle, then changed back to original result after > sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) It's good to know this test case can already detect hidden bug of other fs. > > === Fiemap after dsync write and sleep === > /vda6/file: > EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS > 0: [0..127]: 12288..12415 128 0x1000 > 1: [128..255]: 12160..12287 128 0x1000 > 2: [256..511]: 16384..16639 256 0x1000 > 3: [512..2047]: 16896..18431 1536 0x1000 > 4: [2048..4095]: 19456..21503 2048 0x1001 > === Fiemap after cycle mount === > /vda6/file: > EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS > 0: [0..95]: 12288..12383 96 0x1000 > 1: [96..127]: 12384..12415 32 0x1000 > > [0-127] was split to two extents > > 2: [128..255]: 12160..12287 128 0x1000 > 3: [256..511]: 16384..16639 256 0x1000 > 4: [512..2047]: 16896..18431 1536 0x1000 > 5: [2048..4095]: 19456..21503 2048 0x1001 > === Fiemap after cycle mount and sleep === > /vda6/file: > EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS > 0: [0..127]: 12288..12415 128 0x1000 > 1: [128..255]: 12160..12287 128 0x1000 > 2: [256..511]: 16384..16639 256 0x1000 > 3: [512..2047]: 16896..18431 1536 0x1000 > 4: [2048..4095]: 19456..21503 2048 0x1001 > >> --- >> tests/generic/422 | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/422.out | 2 + >> tests/generic/group | 1 + >> 3 files changed, 130 insertions(+) >> create mode 100755 tests/generic/422 >> create mode 100644 tests/generic/422.out >> >> diff --git a/tests/generic/422 b/tests/generic/422 >> new file mode 100755 >> index 0000000..4ca4476 >> --- /dev/null >> +++ b/tests/generic/422 >> @@ -0,0 +1,127 @@ >> +#! /bin/bash >> +# FS QA Test 422 >> +# >> +# Test if a file system returns constant fiemap result after remount and >> +# fiemap. >> +# Unfortunately, btrfs doesn't follow this behavior. >> +# >> +#----------------------------------------------------------------------- >> +# Copyright (c) 2017 Fujitsu. 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.* >> +} >> + >> +# 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 generic >> +_supported_os IRIX Linux >> +_require_scratch > > Need _require_fiemap > >> + >> +block_size=$((64 * 1024)) >> +block_count=32 >> +dst=$SCRATCH_MNT/file >> +sleeptime=3 >> + >> +# It's almost 100% for btrfs to trigger inconstant fiemap result > > Trailing whitespace in above line. > >> +# just in case >> +runtime=$((2 * $LOAD_FACTOR)) > > I don't think this loop is necessary, as the comments state, it's almost > 100% reproducible for btrfs (and for ext2/3 in my testing). In fact, the uncertainty lies in the time of sleep. In my test environment, if only sleeps for 1 second, the fiemap result won't change for btrfs. (although cycle mount still changes the fiemap result) So it's better to keep the runtime (will be renamed to loop_counts in next version), just as stated, in case. > >> + >> +# record fiemap as checkpoint, and output the hash of fiemap result >> +# to stdout >> +fiemap_checkpoint() >> +{ >> + local number=$1 >> + local message=$2 >> + >> + echo "=== $message ===" >> $seqres.full >> + $XFS_IO_PROG -c "fiemap -v" $dst > ${tmp}.cp$number >> + cat ${tmp}.cp${number} >> $seqres.full >> + >> + md5sum ${tmp}.cp${number} | cut -d ' ' -f 1 >> +} >> + >> +do_test() >> +{ >> + local number=$1 >> + >> + # Use 16 times of file size to ensure we have enough space >> + _scratch_mkfs_sized $((16 * $block_size * $block_count)) \ > > Use _require_fs_space for required space. Not all filesystems have > _scratch_mkfs_sized support so this could potentially reduce the > filesystem coverage. As most tester won't use a disk smaller than 32M as SCRATCH_DEV, I'm OK to remove it. Thanks, Qu > >> + > /dev/null 2>&1 >> + _scratch_mount > > Trailing whitespace issue. > > Thanks, > Eryu > >> + >> + echo "====== Loop $number ======" >> $seqres.full >> + touch $dst >> + # Xfsprogs 4.9.0 still has a bug that xfs_io "open" with O_SYNC command >> + # doesn't work well with "pwrite", although it gets fixed in v4.10.0, >> + # use dd here to avoid it won't hurt for non-xfs developers >> + dd if=/dev/zero of=$dst bs=$block_size count=$block_count oflag=dsync \ >> + status=none 2>&1 >> + >> + hash1=$(fiemap_checkpoint 1 "Fiemap just after dsync write") >> + >> + # Sleep should not modify fiemap result >> + sleep $sleeptime >> + >> + hash2=$(fiemap_checkpoint 2 "Fiemap after dsync write and sleep") >> + >> + # cycle mount should not modify fiemap result >> + _scratch_cycle_mount >> + >> + hash3=$(fiemap_checkpoint 3 "Fiemap after cycle mount") >> + >> + # Sleep should not modify fiemap result >> + sleep $sleeptime >> + >> + hash4=$(fiemap_checkpoint 4 "Fiemap after cycle mount and sleep") >> + >> + _scratch_unmount >> + >> + if [ $hash1 != $hash2 -o $hash2 != $hash3 -o $hash3 != $hash4 ]; then >> + echo "Inconstant fiemap result detected" >> + fi >> + echo >> $seqres.full >> +} >> + >> +for i in $(seq 1 $runtime); do >> + do_test $i >> +done >> + >> +echo "Silence is golden" >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/generic/422.out b/tests/generic/422.out >> new file mode 100644 >> index 0000000..f70693f >> --- /dev/null >> +++ b/tests/generic/422.out >> @@ -0,0 +1,2 @@ >> +QA output created by 422 >> +Silence is golden >> diff --git a/tests/generic/group b/tests/generic/group >> index 3c7c5e4..86d2325 100644 >> --- a/tests/generic/group >> +++ b/tests/generic/group >> @@ -424,3 +424,4 @@ >> 419 auto quick encrypt >> 420 auto quick punch >> 421 auto quick encrypt dangerous >> +422 auto >> -- >> 2.9.3 >> >> >> >> -- >> 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 > > -- 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, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: > > Test fails with ext3/2 when driving with ext4 driver, fiemap changed > after umount/mount cycle, then changed back to original result after > sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) I haven't had time to look at this, but I'm not sure this test is a reasonable one on the face of it. A file system may choose to optimize a file's extent tree for whatever reason it wants, whenever it wants, including on an unmount --- and that would not be an invalid thing to do. So to have an xfstests that causes a test failure if a file system were to, say, do some cleanup at mount or unmount time, or when the file is next opened, to merge adjacent extents together (and hence change what is returned by FIEMAP) might be strange, or even weird --- but is this any of user space's business? Or anything we want to enforce as wrong wrong wrong by xfstests? - Ted -- 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 4/6/17 11:26 AM, Theodore Ts'o wrote: > On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >> >> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >> after umount/mount cycle, then changed back to original result after >> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) > > I haven't had time to look at this, but I'm not sure this test is a > reasonable one on the face of it. > > A file system may choose to optimize a file's extent tree for whatever > reason it wants, whenever it wants, including on an unmount --- and > that would not be an invalid thing to do. So to have an xfstests that > causes a test failure if a file system were to, say, do some cleanup > at mount or unmount time, or when the file is next opened, to merge > adjacent extents together (and hence change what is returned by > FIEMAP) might be strange, or even weird --- but is this any of user > space's business? Or anything we want to enforce as wrong wrong wrong > by xfstests? I had the same question. If the exact behavior isn't defined anywhere, I don't know what we can be testing, TBH. -Eric > - Ted -- 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
At 04/07/2017 12:28 AM, Eric Sandeen wrote: > On 4/6/17 11:26 AM, Theodore Ts'o wrote: >> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>> >>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>> after umount/mount cycle, then changed back to original result after >>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >> >> I haven't had time to look at this, but I'm not sure this test is a >> reasonable one on the face of it. >> >> A file system may choose to optimize a file's extent tree for whatever >> reason it wants, whenever it wants, including on an unmount --- and >> that would not be an invalid thing to do. So to have an xfstests that >> causes a test failure if a file system were to, say, do some cleanup >> at mount or unmount time, or when the file is next opened, to merge >> adjacent extents together (and hence change what is returned by >> FIEMAP) might be strange, or even weird --- but is this any of user >> space's business? Or anything we want to enforce as wrong wrong wrong >> by xfstests? > > I had the same question. If the exact behavior isn't defined anywhere, > I don't know what we can be testing, TBH. For unmount cleanup, it's acceptable. But if we're getting different result even we didn't modify the fs during the same mount duration, fiemap still changes, then it's at least anti-instinct. And unfortunately, btrfs and ext3 shares the same problem here: === Fiemap after cycle mount === /mnt/scratch/file: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..95]: 139264..139359 96 0x1000 1: [96..127]: 139360..139391 32 0x1000 2: [128..511]: 138112..138495 384 0x1000 3: [512..1023]: 138752..139263 512 0x1000 4: [1024..2047]: 143360..144383 1024 0x1000 5: [2048..4095]: 145408..147455 2048 0x1001 === Fiemap after cycle mount and sleep === /mnt/scratch/file: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..127]: 139264..139391 128 0x1000 1: [128..511]: 138112..138495 384 0x1000 2: [512..1023]: 138752..139263 512 0x1000 3: [1024..2047]: 143360..144383 1024 0x1000 4: [2048..4095]: 145408..147455 2048 0x1001 I was using the same excuse just as you guys, until I found btrfs is merging extent maps at read time, which causes the problem. That's why the 2nd fiemap in btrfs returns merged result. We fix btrfs by caching fiemap extent result before calling fiemap_fill_next_extent(), and try to merge with cached result. Which fixes the problem quite easy. Thanks, Qu > > -Eric > >> - Ted > > -- 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, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: > On 4/6/17 11:26 AM, Theodore Ts'o wrote: > > On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: > >> > >> Test fails with ext3/2 when driving with ext4 driver, fiemap changed > >> after umount/mount cycle, then changed back to original result after > >> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) > > > > I haven't had time to look at this, but I'm not sure this test is a > > reasonable one on the face of it. > > > > A file system may choose to optimize a file's extent tree for whatever > > reason it wants, whenever it wants, including on an unmount --- and > > that would not be an invalid thing to do. So to have an xfstests that > > causes a test failure if a file system were to, say, do some cleanup > > at mount or unmount time, or when the file is next opened, to merge > > adjacent extents together (and hence change what is returned by > > FIEMAP) might be strange, or even weird --- but is this any of user > > space's business? Or anything we want to enforce as wrong wrong wrong > > by xfstests? So I was asking for a review from ext4 side instead of queuing it for next xfstests update :) > > I had the same question. If the exact behavior isn't defined anywhere, > I don't know what we can be testing, TBH. Agreed, I was about to ask for the expected behavior today if there was no new review comments on this patch. Thanks for the comments and review! 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 Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: > On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: > > On 4/6/17 11:26 AM, Theodore Ts'o wrote: > > > On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: > > >> > > >> Test fails with ext3/2 when driving with ext4 driver, fiemap changed > > >> after umount/mount cycle, then changed back to original result after > > >> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) > > > > > > I haven't had time to look at this, but I'm not sure this test is a > > > reasonable one on the face of it. > > > > > > A file system may choose to optimize a file's extent tree for whatever > > > reason it wants, whenever it wants, including on an unmount --- and > > > that would not be an invalid thing to do. So to have an xfstests that > > > causes a test failure if a file system were to, say, do some cleanup > > > at mount or unmount time, or when the file is next opened, to merge > > > adjacent extents together (and hence change what is returned by > > > FIEMAP) might be strange, or even weird --- but is this any of user > > > space's business? Or anything we want to enforce as wrong wrong wrong > > > by xfstests? > > So I was asking for a review from ext4 side instead of queuing it for > next xfstests update :) In general FIEMAP can return pretty much whatever it wants, which usually means that it won't report extents larger than the underlying block mapping extents, though as we've seen it can split a single on-disk extent into multiple FIEMAP records for the purpose of reporting sharedness. For ext3 I'm wondering if it's the case that the first time we FIEMAP an indirect map file we see a possibly-merged version of whatever's in the particular leaf node we land in; then that information gets cached & merged with other records in the extent status tree, such that subsequent FIEMAP calls see longer extents than the first time around. > > I had the same question. If the exact behavior isn't defined anywhere, > > I don't know what we can be testing, TBH. > > Agreed, I was about to ask for the expected behavior today if there was > no new review comments on this patch. I think the expected behavior is that any behavior is expected. :( --D > > Thanks for the comments and review! > > 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 -- 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 4/7/17 10:42 AM, Darrick J. Wong wrote: > On Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: >> On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: >>> On 4/6/17 11:26 AM, Theodore Ts'o wrote: >>>> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>>>> >>>>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>>>> after umount/mount cycle, then changed back to original result after >>>>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >>>> >>>> I haven't had time to look at this, but I'm not sure this test is a >>>> reasonable one on the face of it. >>>> >>>> A file system may choose to optimize a file's extent tree for whatever >>>> reason it wants, whenever it wants, including on an unmount --- and >>>> that would not be an invalid thing to do. So to have an xfstests that >>>> causes a test failure if a file system were to, say, do some cleanup >>>> at mount or unmount time, or when the file is next opened, to merge >>>> adjacent extents together (and hence change what is returned by >>>> FIEMAP) might be strange, or even weird --- but is this any of user >>>> space's business? Or anything we want to enforce as wrong wrong wrong >>>> by xfstests? >> >> So I was asking for a review from ext4 side instead of queuing it for >> next xfstests update :) > > In general FIEMAP can return pretty much whatever it wants, which > usually means that it won't report extents larger than the underlying > block mapping extents, though as we've seen it can split a single > on-disk extent into multiple FIEMAP records for the purpose of reporting > sharedness. > > For ext3 I'm wondering if it's the case that the first time we FIEMAP an > indirect map file we see a possibly-merged version of whatever's in the > particular leaf node we land in; then that information gets cached & > merged with other records in the extent status tree, such that > subsequent FIEMAP calls see longer extents than the first time around. > >>> I had the same question. If the exact behavior isn't defined anywhere, >>> I don't know what we can be testing, TBH. >> >> Agreed, I was about to ask for the expected behavior today if there was >> no new review comments on this patch. > > I think the expected behavior is that any behavior is expected. :( I suppose that if any particular filesystem wants to enforce stricter rules for its own fiemap interface, that could be done in a filesystem-specific test... -Eric -- 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
At 04/07/2017 11:48 PM, Eric Sandeen wrote: > On 4/7/17 10:42 AM, Darrick J. Wong wrote: >> On Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: >>> On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: >>>> On 4/6/17 11:26 AM, Theodore Ts'o wrote: >>>>> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>>>>> >>>>>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>>>>> after umount/mount cycle, then changed back to original result after >>>>>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >>>>> >>>>> I haven't had time to look at this, but I'm not sure this test is a >>>>> reasonable one on the face of it. >>>>> >>>>> A file system may choose to optimize a file's extent tree for whatever >>>>> reason it wants, whenever it wants, including on an unmount --- and >>>>> that would not be an invalid thing to do. So to have an xfstests that >>>>> causes a test failure if a file system were to, say, do some cleanup >>>>> at mount or unmount time, or when the file is next opened, to merge >>>>> adjacent extents together (and hence change what is returned by >>>>> FIEMAP) might be strange, or even weird --- but is this any of user >>>>> space's business? Or anything we want to enforce as wrong wrong wrong >>>>> by xfstests? >>> >>> So I was asking for a review from ext4 side instead of queuing it for >>> next xfstests update :) >> >> In general FIEMAP can return pretty much whatever it wants, which >> usually means that it won't report extents larger than the underlying >> block mapping extents, though as we've seen it can split a single >> on-disk extent into multiple FIEMAP records for the purpose of reporting >> sharedness. >> >> For ext3 I'm wondering if it's the case that the first time we FIEMAP an >> indirect map file we see a possibly-merged version of whatever's in the >> particular leaf node we land in; then that information gets cached & >> merged with other records in the extent status tree, such that >> subsequent FIEMAP calls see longer extents than the first time around. >> >>>> I had the same question. If the exact behavior isn't defined anywhere, >>>> I don't know what we can be testing, TBH. >>> >>> Agreed, I was about to ask for the expected behavior today if there was >>> no new review comments on this patch. >> >> I think the expected behavior is that any behavior is expected. :( > > I suppose that if any particular filesystem wants to enforce stricter > rules for its own fiemap interface, that could be done in a > filesystem-specific test... Well, then some test cases need to be moved from generic to filesystem specified test, and some filsystems will have similar test cases. I think it's a hell for maintainer to keep all these test cases consistent. This is not the first time Btrfs failed to pass reflink and fiemap test designed for xfs though. Btrfs does skip some generic test case originally designed for xfs, like generic/372. But most of the time, xfs and btrfs can get consistent result. And for btrfs side, we already have such fix now: https://patchwork.kernel.org/patch/9668721/ It's OK that any fs can have either own fiemap behavior, but if it's a not big fix and the behavior itself is somewhat wired, why not joining the common behavior? Thanks, Qu > > -Eric > -- > 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 --git a/tests/generic/422 b/tests/generic/422 new file mode 100755 index 0000000..4ca4476 --- /dev/null +++ b/tests/generic/422 @@ -0,0 +1,127 @@ +#! /bin/bash +# FS QA Test 422 +# +# Test if a file system returns constant fiemap result after remount and +# fiemap. +# Unfortunately, btrfs doesn't follow this behavior. +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 Fujitsu. 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.* +} + +# 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 generic +_supported_os IRIX Linux +_require_scratch + +block_size=$((64 * 1024)) +block_count=32 +dst=$SCRATCH_MNT/file +sleeptime=3 + +# It's almost 100% for btrfs to trigger inconstant fiemap result +# just in case +runtime=$((2 * $LOAD_FACTOR)) + +# record fiemap as checkpoint, and output the hash of fiemap result +# to stdout +fiemap_checkpoint() +{ + local number=$1 + local message=$2 + + echo "=== $message ===" >> $seqres.full + $XFS_IO_PROG -c "fiemap -v" $dst > ${tmp}.cp$number + cat ${tmp}.cp${number} >> $seqres.full + + md5sum ${tmp}.cp${number} | cut -d ' ' -f 1 +} + +do_test() +{ + local number=$1 + + # Use 16 times of file size to ensure we have enough space + _scratch_mkfs_sized $((16 * $block_size * $block_count)) \ + > /dev/null 2>&1 + _scratch_mount + + echo "====== Loop $number ======" >> $seqres.full + touch $dst + # Xfsprogs 4.9.0 still has a bug that xfs_io "open" with O_SYNC command + # doesn't work well with "pwrite", although it gets fixed in v4.10.0, + # use dd here to avoid it won't hurt for non-xfs developers + dd if=/dev/zero of=$dst bs=$block_size count=$block_count oflag=dsync \ + status=none 2>&1 + + hash1=$(fiemap_checkpoint 1 "Fiemap just after dsync write") + + # Sleep should not modify fiemap result + sleep $sleeptime + + hash2=$(fiemap_checkpoint 2 "Fiemap after dsync write and sleep") + + # cycle mount should not modify fiemap result + _scratch_cycle_mount + + hash3=$(fiemap_checkpoint 3 "Fiemap after cycle mount") + + # Sleep should not modify fiemap result + sleep $sleeptime + + hash4=$(fiemap_checkpoint 4 "Fiemap after cycle mount and sleep") + + _scratch_unmount + + if [ $hash1 != $hash2 -o $hash2 != $hash3 -o $hash3 != $hash4 ]; then + echo "Inconstant fiemap result detected" + fi + echo >> $seqres.full +} + +for i in $(seq 1 $runtime); do + do_test $i +done + +echo "Silence is golden" +# success, all done +status=0 +exit diff --git a/tests/generic/422.out b/tests/generic/422.out new file mode 100644 index 0000000..f70693f --- /dev/null +++ b/tests/generic/422.out @@ -0,0 +1,2 @@ +QA output created by 422 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index 3c7c5e4..86d2325 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -424,3 +424,4 @@ 419 auto quick encrypt 420 auto quick punch 421 auto quick encrypt dangerous +422 auto
As long as we don't modify the on-disk data, fiemap result should always be constant. Operation like cycle mount and sleep should not affect fiemap result. While unfortunately, btrfs doesn't follow that behavior. Btrfs fiemap sometimes return merged result, while after cycle mount, it returns split result. Furthermore after a snap, btrfs returns merged result again. Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- tests/generic/422 | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/422.out | 2 + tests/generic/group | 1 + 3 files changed, 130 insertions(+) create mode 100755 tests/generic/422 create mode 100644 tests/generic/422.out