From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: Eryu Guan <eguan@redhat.com> Cc: <linux-btrfs@vger.kernel.org>, <fstests@vger.kernel.org>, <linux-ext4@vger.kernel.org> Subject: Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Date: Wed, 5 Apr 2017 10:42:45 +0800 [thread overview] Message-ID: <30a56575-ce59-a16b-c353-be1ecb6bb327@cn.fujitsu.com> (raw) In-Reply-To: <20170405023526.GS22845@eguan.usersys.redhat.com> 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 > >
WARNING: multiple messages have this Message-ID (diff)
From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: Eryu Guan <eguan@redhat.com> Cc: linux-btrfs@vger.kernel.org, fstests@vger.kernel.org, linux-ext4@vger.kernel.org Subject: Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Date: Wed, 5 Apr 2017 10:42:45 +0800 [thread overview] Message-ID: <30a56575-ce59-a16b-c353-be1ecb6bb327@cn.fujitsu.com> (raw) In-Reply-To: <20170405023526.GS22845@eguan.usersys.redhat.com> 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 > >
next prev parent reply other threads:[~2017-04-05 2:42 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-03 7:09 [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Qu Wenruo 2017-04-03 7:09 ` Qu Wenruo 2017-04-03 16:31 ` Darrick J. Wong 2017-04-05 0:51 ` Qu Wenruo 2017-04-05 0:51 ` Qu Wenruo 2017-04-05 2:35 ` Eryu Guan 2017-04-05 2:42 ` Qu Wenruo [this message] 2017-04-05 2:42 ` Qu Wenruo 2017-04-06 16:26 ` Theodore Ts'o 2017-04-06 16:28 ` Eric Sandeen 2017-04-07 0:48 ` Qu Wenruo 2017-04-07 0:48 ` Qu Wenruo 2017-04-07 5:02 ` Eryu Guan 2017-04-07 15:42 ` Darrick J. Wong 2017-04-07 15:48 ` Eric Sandeen 2017-04-10 2:07 ` Qu Wenruo 2017-04-10 2:07 ` Qu Wenruo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=30a56575-ce59-a16b-c353-be1ecb6bb327@cn.fujitsu.com \ --to=quwenruo@cn.fujitsu.com \ --cc=eguan@redhat.com \ --cc=fstests@vger.kernel.org \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.