On Mon, Oct 07, 2013 at 05:14:06AM +0900, Namjae Jeon wrote: > From: Namjae Jeon <namjae.jeon@xxxxxxxxxxx> > > We execute collapse range multiple times on same file. > Each collapse range call collapses a single alternate block. > After the test execution, file will be left with 80 blocks and > as much number of extents. > We also check for file system consistency after the completion. ..... > +. ./common/rc > +. ./common/filter > + > +# real QA test starts here > +_supported_fs xfs ext4 > +_supported_os Linux > + > +_require_scratch > +_require_xfs_io_fiemap > +_require_xfs_io_falloc_collapse > +_do_die_on_error=y > +test=$SCRATCH_MNT/test Not used. > +testfile=$SCRATCH_MNT/317.$$ > +BSIZE=4096 > +BLOCKS=10240 > + > +# Filters fiemap output > +_filter_fiemap() > +{ > + awk --posix ' > + $3 ~ /hole/ { > + print $1, $2, $3; > + next; > + } > + $5 ~ /0x[[:xdigit:]]+/ { > + print $1, $2, "extent"; > + }' > +} There's already a function in common/punch of this name, and it does pretty much the same thing. Why not use that? > + > +case $FSTYP in > + ext4) > + export MKFS_OPTIONS="-F -b $BSIZE" > + ;; > + xfs) > + export MKFS_OPTIONS="-f -b size=$BSIZE" > + ;; > +esac _scratch_mkfs takes options on the command line - there is no need to do this. In fact, this test needs to run on all block sizes that filesystems are capable of using, not just 4k and different architectures exercise different code paths and so we must be able to test the case where block size is smaller than page size on x86-64 so when the code is run on an ia64 or ppc64 box with a 64k page size we know that it's not completely broken... Anyway, if you really need to make a 4k block size filesystem, then _scratch_mkfs_sized() is the generic way of doing this. > +# make filesystem on scratch with 4KB blocksize > +_do 'make filesystem on $SCRATCH_DEV' '_scratch_mkfs' > +_do 'mount filesytem' '_scratch_mount' I really dislike this "_do" wrapper. The text does not add anything to the test, and it makes it hard to see the command being run and harder to modify it when necessary. It is used only by a couple of old tests, and we'd do better to remove it than to propagate it further. This: _scratch_mkfs >> $seqres.full 2>&1 || _fail "scratch_mkfs failed." _scratch_mount >> $seqres.full 2>&1 || _fail "scratch_mount failed." does everything that the _do wrapper does. > + > +# Write file > +length=$(($BLOCKS*$BSIZE)) > +$XFS_IO_PROG -f -c "pwrite 0 $length" -c fsync $testfile > /dev/null > + > +# Collapse alternate blocks > +for (( i = 1; i <= 7; i++ )); do > + for(( j=0 ; j < $(($BLOCKS/(2**$i))) ; j++ )); do > + offset=$(($j*$BSIZE)) > + $XFS_IO_PROG -c "fcollapse $offset $BSIZE" $testfile > /dev/null > + done > +done > + > +# Check if 80 extents are present > +$XFS_IO_PROG -c "fiemap -v" $testfile | _filter_fiemap If all you care about is that there are 80 extents, then why not just something like: $XFS_IO_PROG -c "fiemap -v" $testfile |grep "^ *[0-9]*:" |wc -l > + > +_do 'unmount $SCRATCH_DEV' 'umount $SCRATCH_DEV' > +_do 'repair filesystem' '_check_scratch_fs' _check_scratch_fs is all you need to call here. > index 3a69294..80ff7ec 100644 > --- a/tests/shared/group > +++ b/tests/shared/group > @@ -12,3 +12,4 @@ > 298 auto trim > 305 aio dangerous enospc rw stress > 316 auto quick collapse > +317 auto collapse Again, I think the prealloc group is better for this. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html