| Message ID | 1432055622-8638-1-git-send-email-jbacik@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 19, 2015 at 01:13:42PM -0400, Josef Bacik wrote: > I noticed that btrfs wasn't setting unwritten on prealloc test, and then > subsequently noticed that we weren't testing fiemap on prealloc extents with the > fiemap-tester. This patch adds another test that does the same as generic/225 > only with prealloc enabled. Thanks, Fails on XFS, but that's because XFS is doing things fiemap-tester does not understand (speculative prealloc beyond EOF when appending writes occur on files larger than 64k) and hence XFS ends up with zeroed data on disk in places where fiemaptester is expecting preallocated blocks. i.e. there's not data corruption, and nothing wrong with XFS's behaioviour, just a broken assumption that the filesystem will lay out data due to buffered writes and delayed allocation exactly as it expects. IOWs, i think that fiemap-tester might need to direct IO to get the results it expects, not buffered IO... > +_scratch_mount > /dev/null 2>&1 > + > +rm -f $seqres.full > +fiemapfile=$SCRATCH_MNT/$seq.fiemap > +fiemaplog=$SCRATCH_MNT/$seq.log The log file to use for debug information is $seqres.full. Cheers, Dave.
On 05/25/2015 09:31 PM, Dave Chinner wrote: > On Tue, May 19, 2015 at 01:13:42PM -0400, Josef Bacik wrote: >> I noticed that btrfs wasn't setting unwritten on prealloc test, and then >> subsequently noticed that we weren't testing fiemap on prealloc extents with the >> fiemap-tester. This patch adds another test that does the same as generic/225 >> only with prealloc enabled. Thanks, > > Fails on XFS, but that's because XFS is doing things fiemap-tester > does not understand (speculative prealloc beyond EOF when appending > writes occur on files larger than 64k) and hence XFS ends up with > zeroed data on disk in places where fiemaptester is expecting > preallocated blocks. > > i.e. there's not data corruption, and nothing wrong with XFS's > behaioviour, just a broken assumption that the filesystem will lay > out data due to buffered writes and delayed allocation exactly as it > expects. > > IOWs, i think that fiemap-tester might need to direct IO to get the > results it expects, not buffered IO... > Huh crap I thought I caught this case, but now I look at it I only caught it for the HOLE case not the PREALLOC case, so I'll fix up the tester to check if there's 0's where there should be a prealloc. >> +_scratch_mount > /dev/null 2>&1 >> + >> +rm -f $seqres.full >> +fiemapfile=$SCRATCH_MNT/$seq.fiemap >> +fiemaplog=$SCRATCH_MNT/$seq.log > > The log file to use for debug information is $seqres.full. > I'll fix it up, thanks, Josef -- 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 --git a/tests/generic/328 b/tests/generic/328 new file mode 100644 index 0000000..b6d6917 --- /dev/null +++ b/tests/generic/328 @@ -0,0 +1,78 @@ +#! /bin/bash +# FS QA Test No. 328 +# +# Run the fiemap (file extent mapping) tester with preallocation enabled +# +#----------------------------------------------------------------------- +# Copyright (c) 2009 Eric Sandeen. 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 + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "fiemap" +_require_xfs_io_command "falloc" + +_scratch_mkfs > /dev/null 2>&1 +_scratch_mount > /dev/null 2>&1 + +rm -f $seqres.full +fiemapfile=$SCRATCH_MNT/$seq.fiemap +fiemaplog=$SCRATCH_MNT/$seq.log + +[ -x $here/src/fiemap-tester ] || _notrun "fiemap-tester not built" + +seed=`date +%s` + +echo "using seed $seed" >> $fiemaplog + +echo "fiemap run with sync" +$here/src/fiemap-tester -q -S -s $seed -r 200 $fiemapfile 2>&1 | tee -a $fiemaplog + +if [ $? -ne 0 ]; then + if grep -q "Operation not supported" $fiemaplog; then + _notrun "FIEMAP not supported by this filesystem type: $FSTYP" + fi + status=$? + exit +fi + +echo "fiemap run without sync" +$here/src/fiemap-tester -q -s $seed -r 200 $fiemapfile 2>&1 | tee -a $fiemaplog + +status=0 +exit diff --git a/tests/generic/328.out b/tests/generic/328.out new file mode 100644 index 0000000..a8f052a --- /dev/null +++ b/tests/generic/328.out @@ -0,0 +1,3 @@ +QA output created by 328 +fiemap run with sync +fiemap run without sync diff --git a/tests/generic/group b/tests/generic/group index 09bf1a5..3024cd5 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -194,3 +194,4 @@ 325 auto quick data log 326 auto log 327 auto quick +328 auto quick prealloc
I noticed that btrfs wasn't setting unwritten on prealloc test, and then subsequently noticed that we weren't testing fiemap on prealloc extents with the fiemap-tester. This patch adds another test that does the same as generic/225 only with prealloc enabled. Thanks, Signed-off-by: Josef Bacik <jbacik@fb.com> --- tests/generic/328 | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/328.out | 3 ++ tests/generic/group | 1 + 3 files changed, 82 insertions(+) create mode 100644 tests/generic/328 create mode 100644 tests/generic/328.out