On Thu, Nov 08, 2012 at 03:18:09PM -0500, Brian Foster wrote:
> The speculative preallocation trimming test verifies that files
> with post-EOF blocks are trimmed when the scan is invoked.
> Background scans and the various scan filters are tested as well.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>
> Hi all,
>
> This is my first stab at an xfstests test for the eofblocks patchset.
A good start!
> This
> depends on the xfs_spaceman tool and associated 'prealloc' command. The bits to
> check the latter command are included for brevity, but I could certainly break
> that stuff into a 1/2 patch in subsequent posts if desired. This has been lightly
> tested against the v7 eofblocks set running in a VM. Thoughts appreciated.
I'll have to get a basic version of the spacemen patchset I have
sent out again so that we can get this into xfsprogs ASAP. I think
we might be waiting until after the release is done before that
happens, though....
>
> 290 | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 290.out | 55 ++++++++++++++++++
> common.config | 1 +
> common.rc | 10 +++
> group | 1 +
> 5 files changed, 243 insertions(+), 0 deletions(-)
> create mode 100755 290
> create mode 100644 290.out
>
> diff --git a/290 b/290
> new file mode 100755
> index 0000000..12ce9c2
> --- /dev/null
> +++ b/290
> @@ -0,0 +1,176 @@
> +#! /bin/bash
> +# FS QA Test No. 290
> +#
> +# Verify speculative preallocation trimming functionality.
A more verbose description of the test is a good habit to get into.
> +# Write a file of specified size after sending a couple 1 byte writes. The
> +# repeated writes triggers the open-write-close optimization that keeps post-EOF
> +# preallocated space around.
> +_write_prealloc_file()
FWIW, the leading _ in the function names is typically used to
indicate a library function. i.e. something sourced from the
common.* includes. That's to avoid namespace clashes with local
functions that otherwise might have the same name. e.g.
filter_xfs_io vs _filter_xfs_io - the former is a local function,
the latter is defined in common.filter....
> +{
> + file=$1
> + size=$2
> +
> + for i in "1" "1" "$size"
> + do
> + $XFS_IO_PROG -f -c "pwrite -b 4k 0 $i" $file \
> + >> $seq.full 2>&1 || _fail "failed to write file"
Just dump the xfs_io output into $seq.out though the io filter.
There's no need to check for failure to write - the golden output
check will catch that as the first point of failure in the test.
i.e
$XFS_IO_PROG -f -c "pwrite -b 4k 0 $i" $file | _filter_xfs_io
As it is, it's good habit to let a test continue to run even if
something fails - the test should continue to completion gracefully
and not trigger corruption or kernel panics, etc. If the failure
mode is a shutdown, for example, the test then exercises behaviour
on a shutdown filesystem. We don't care about the results after the
first failure, but the test should still run through to the end
without hanging or crashing the kernel. Hence I consider peppering
tests with _fail conditions to be a bad practice....
> + done
> +}
> +
> +_trim_prealloc()
> +{
> + file=$1
> + args=$2
> +
> + $XFS_SPACEMAN_PROG -c "prealloc $args" $file >> $seq.full 2>&1 \
> + || _fail "failed to trim file preallocation"
Same again - leave the output in $seq.out, let the golden image
match fail the test. Essentially this brings the function down to a
single line, and hence no need for a function at all...
> +}
> +
> +_stat_files()
> +{
> + for i in $SCRATCH_MNT/test.*
> + do
> + echo -n "$(basename $i) "
> + stat -c "%s bytes %b blocks" $i
> + done
That whole function can be replaced by:
stat -c "%n %s bytes %b blocks" $SCRATCH_MNT/test.* | _filter_scratch
> +}
> +
> +# Create a set of test.* files that fall under different filters for prealloc
> +# scanning.
> +_create_files()
> +{
> + rm -f $SCRATCH_MNT/test.*
> +
> + _write_prealloc_file $SCRATCH_MNT/test.${qa_user} 6m
> + chown ${qa_user}:${qa_user} $SCRATCH_MNT/test.${qa_user}
> +
> + _write_prealloc_file $SCRATCH_MNT/test.${qa_user}.root 6m
> + chown ${qa_user}:root $SCRATCH_MNT/test.${qa_user}.root
> +
> + _write_prealloc_file $SCRATCH_MNT/test.prid.42 6m
> + $XFS_QUOTA_PROG -x -c "project -s -p $SCRATCH_MNT/test.prid.42 42" \
> + $SCRATCH_MNT >> $seq.full 2>&1 || _fail "failed to set project id"
> +
> + _write_prealloc_file $SCRATCH_MNT/test.10m 10m
> +
> + _write_prealloc_file $SCRATCH_MNT/test.falloc 6m
> + $XFS_IO_PROG -c "falloc 0 1" $SCRATCH_MNT/test.falloc \
> + >> $seq.full 2>&1 || _fail "failed to fallocate file"
> +}
> +
> +_set_speculative_prealloc_lifetime()
> +{
> + seconds=$1
> + echo $seconds > /proc/sys/fs/xfs/speculative_prealloc_lifetime || \
> + _fail "failed to set speculative_prealloc_lifetime"
> +}
I wouldn't bother with a function for this, and once again just let
errors end up in the $seq.out file for golden image matching to
fail.
> +# real QA test starts here
> +_supported_fs xfs
> +_require_xfs_spaceman_prealloc
> +_require_scratch
> +_require_user
> +_require_xfs_quota
> +
> +_scratch_mkfs_xfs -d size=200m >> $seq.full 2>&1 || _fail "mkfs failed"
_scratch_mkfs_sized 200m >> $seq.full 2>&1
And once again, there's generally no need to check for mkfs failure.
> +_scratch_mount
> +
> +echo "==================="
> +echo -e "speculative preallocation trim"
> +
> +_create_files
> +echo -e "\nfiles"
I can't say I like the extra line output here. "pretty" .out
contents isn't really necessary, especially when it makes the test
code harder to read...
> +_stat_files
> +# trim all files
> +_trim_prealloc $SCRATCH_MNT "-s"
> +echo -e "\nno filter"
> +_stat_files
If you are going to put progress output in the $seq.out file (which
I still think is unnecessary), then put it in the function doing
that work.
> +echo -e "\nuid/gid filter"
> +_stat_files
> +
> +echo -e "\n==================="
> +echo -e "background speculative preallocation trim"
> +
> +# Clean up (unmount), set the lifetime to 5s and remount to ensure that the new
> +# lifetime kicks in immediately.
> +
> +_cleanup
> +_set_speculative_prealloc_lifetime 5
old_lifetime=`cat ....`
echo 5 > ....
> +
> +_scratch_mount
Did I miss an unmount somewhere?
> +_create_files
> +
> +# flush and wait a few scan intervals
> +sync
> +sleep 15
> +echo -e "\nbackground scan"
> +_stat_files
> +
> +_set_speculative_prealloc_lifetime 300
echo $old_lifetime > ....
> diff --git a/group b/group
> index a846b60..675d5b5 100644
> --- a/group
> +++ b/group
> @@ -408,3 +408,4 @@ deprecated
> 287 auto dump quota quick
> 288 auto quick ioctl trim
> 289 auto quick
> +290 auto quick
I'd suggest that the rw and ioctl groups are appropriate here as
well.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
