[Top] [All Lists]

Re: [XFS Tests Punch Hole 3/3 v3] XFS TESTS: Add Fallocate Punch Hole Te

To: Allison Henderson <achender@xxxxxxxxxxxxxxxxxx>
Subject: Re: [XFS Tests Punch Hole 3/3 v3] XFS TESTS: Add Fallocate Punch Hole Test
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 19 May 2011 12:41:43 +1000
Cc: xfs-oss <xfs@xxxxxxxxxxx>, Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>, linux-fsdevel <linux-fsdevel@xxxxxxxxxxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxx>, Theodore Tso <tytso@xxxxxxx>
In-reply-to: <4DD43305.7090508@xxxxxxxxxxxxxxxxxx>
References: <4DD43305.7090508@xxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Wed, May 18, 2011 at 01:58:45PM -0700, Allison Henderson wrote:
> This patch adds punch hole tests that are based off the test scripts that 
> were 
> used to test and develop punch hole for ext4.  The tests use the new common 
> routines in common.punch to run through various corners cases for punching 
> holes.  

It's really just a slightly different set of corner cases to what
252 tests, and there's quite a bit of overlap. What your tests don't
do is test different combinations of extent type conversions and he
splits and merges which, historically, have been the major source of
bugs with hole punching...

Adding your various cases to 252 (generic_test_punch) would be a much
better idea, I think.

With that in mind:

> +# Small Hole Test

Covered by 252, case 2 "into allocated space".

> +# Big Hole and Hole Removal Test

Covered by 252, cases 11, 12 and 13. it doesn't need to write 600MB
files to do this, and in the case of XFS, a 600MB file is still only
a single extent, so the test is behaving according to your desire on

> +# also ensure the all of the extents are properly
> +# removed when the file is deleted.

That's covered by the removing of the test file between test
cases in 252.

> +#record how many blocks free blocks the filesystem has after the test
> +fs_used_after_test=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | 
> awk '{ print $3 }'`
> +fs_avail_after_test=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV 
> | awk '{ print $4 }'`
> +
> +if [ $fs_used_before_test != $fs_used_after_test ] || \
> +   [ $fs_avail_before_test != $fs_avail_after_test ]; then
> +
> +     echo Error: File system did not reclaim the correct number of blocks
> +     echo The number of used and available blocks should be the same before 
> and after the test
> +     echo Used blocks before test: $fs_used_before_test
> +     echo Available blocks before test: $fs_avail_before_test
> +     echo Used blocks after test: $fs_used_after_test
> +     echo Available blocks after test: $fs_avail_after_test
> +     status=1 ; exit
> +else
> +     echo Big Hole and Hole Removal Test OK
> +fi

Again, the filesystem free block count tests are simply not
reliable. I'm pretty sure that this simply won't work on btrfs....

> +# Last Block Hole Test

Not directly handled by 252. Easily added, though.

        echo "  14. data -> hole @ EOF"
        rm -f $testfile
        $XFS_IO_PROG $xfs_io_opt -f -c "truncate 20k" \
                -c "pwrite 0 20k" -c "fsync" \
                -c "$zero_cmd 12k 8k" \
                -c "$map_cmd -v" $testfile | $filter_cmd
        [ $? -ne 0 ] && die_now

> +# First Block Hole Test

Same here

        echo "  15. data -> hole @ 0"
        rm -f $testfile
        $XFS_IO_PROG $xfs_io_opt -f -c "truncate 20k" \
                -c "pwrite 0 20k" -c "fsync" \
                -c "$zero_cmd 0k 8k" \
                -c "$map_cmd -v" $testfile | $filter_cmd
        [ $? -ne 0 ] && die_now

> +# Cold Cache Hole Test
> +#
> +# This test is similar to the Small Hole Test, but the drive is 
> +# unmounted and remounted before the hole is punched out to make 
> +# sure that there are no pages in the cache. This test verifies
> +# that the hole contains zeros even when the pages are cold.
> +#----------------------------------------------------------------------
> +echo Running Cold Cache Hole Test
> +file_len=40960
> +hole_offset=8180
> +hole_len=8180
> +_hole_test -m -s -c $file_len $testfile $hole_offset $hole_len
> +echo Cold Cache Hole Test OK

Not directly tested in 252, but easily added via:

        echo "  15. data -> cache cold ->hole"
        rm -f $testfile
        rm -f $testfile.2
        $XFS_IO_PROG $xfs_io_opt -f -c "truncate 20k" \
                -c "pwrite 8k 12k" -c "fsync" $testfile.2
        $XFS_IO_PROG $xfs_io_opt -f -c "truncate 20k" \
                -c "pwrite 0 20k" -c "fsync" \
                -c "$zero_cmd 0k 8k" \
                -c "fadvise -d" \
                -c "$map_cmd -v" $testfile | $filter_cmd
        diff $testfile $testfile.2
        [ $? -ne 0 ] && die_now
        rm -f $testfile.2

> +# Extended Last Block Hole Test
> +#
> +# This test cases punches a hole in 10K file from bytes 32768 to 45056
> +# resulting in a hole that extends beyond the end of the file.  This 
> +# test verifies that the code appropriately handles punching holes 
> +# that extend beyond the end of the file
> +#----------------------------------------------------------------------
> +echo Running Last Block Hole Test
> +file_len=40960
> +hole_offset=32768
> +hole_len=12288
> +_hole_test -s -c $file_len $testfile $hole_offset $hole_len
> +echo Last Block Hole Test OK

That's effectively a repeat of the "data -> hole @ EOF" test.
Realistically, this test needs to be run with preallocation beyond
EOF to show that it can punch extents beyond EOF....

> +# Off Edge Hole Test

Just using a non-block aligned start hole offset in the above test
will cover this one as well. No need for multiple tests that
effectively do the same thing.

> +# One Block Hole Test

If anyone is doing this in an application, they deserve what they
get :/

        echo "  16. data -> hole in single block file"
        rm -f $testfile
        $XFS_IO_PROG $xfs_io_opt -f -c "truncate 512" \
                -c "pwrite 0 512" -c "fsync" \
                -c "$zero_cmd 128 128" \
                -c "$map_cmd -v" $testfile | $filter_cmd
        [ $? -ne 0 ] && die_now

> +# Delayed Allocation Hole Test
> +#
> +# This test is similar to the Small Hole Test, but the file is not sync'd 
> +# to the disk before the hole is punched out. This will test the codes 
> +# ability to punch holes in delayed extents and also ensure that dirty
> +# pages are zero'd properly
> +#----------------------------------------------------------------------
> +echo Running Delayed Allocation Hole Test
> +file_len=40960
> +hole_offset=8180
> +hole_len=8180
> +_hole_test -c $file_len $testfile $hole_offset $hole_len
> +echo Delayed Allocation Hole Test OK

If you want to test this properly, you need to run all the 252 tests
without the fsync after the data write so the extents are still
delalloc before the other operations are done. i.e. make the fsync
command a variable like map_cmd and zero_cmd.

> +
> +# Pre Allocation Hole Test

Already covered by case 3 "into unwritten space".

> +# Multi Hole Test

Far better to redo the whole punch series without removing the file
in-between the individual tests. That will cover far more edge
conditions that "same hole, 12 bytes lower, 12 bytes higher and 4k
higher." As it is, the last test is already covered by 252.

> +# Multi Hole Test Front

And in doing so would cover this case.

> +# Multi Hole Test Delayed

and this case without needing any extra work.

And the good part about extending 252 (from my persepctive) is that
test 242 (for the XFS_IOC_ZERO_RANGE ioctl) will also get better
coverage. And if I add the fallocate zero-range equivalent, it will
also be easy to test for all these cases, too, just by running
test_generic_punch with a different zero command....


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>