xfs/242: fix for archs with 8k page size
PATCH 1/1 xfs/242: fix for archs with 8k page size
This test was failing on sparc64 because there is a minimum granularity of PAGE_CACHE_SIZE in xfs_vnodeops.c:xfs_zero_file_space(). This change follows the approach taken in xfs/194 to filter the bmap output to be in terms of "blocksize" which is computed from pagesize.
_test_generic_punch is modified to optionally take multiple as an argument, so the file under test will be twice the size on an 8k machine as a 4k machine. Since the files will be different sizes, we can no longer use md5sum so od -x is used instead with the byte offsets converted to "blocksize" offsets.
The changes to _test_generic_punch only take effect in the case of the xfs/242 caller, other callers will have multiple=1 and still use md5sum.
xfs/242 was tested with these changes on both a 4k and 8k machine.
--- common/punch | 63 +++++++--------- tests/xfs/242 | 76 +++++++++++++++++++- tests/xfs/242.out | 211 ++++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 254 insertions(+), 96 deletions(-)
common/punch +27 -36
@@ -234,23 +234,6 @@ _filter_hole_fiemap() _coalesce_extents } -_filter_bmap() -{ - awk ' - $3 ~ /hole/ { - print $1, $2, $3; - next; - } - $7 ~ /10000/ { - print $1, $2, "unwritten"; - next; - } - $7 ~ /00000/ { - print $1, $2, "data" - }' | - _coalesce_extents -} - die_now() { status=1
@@ -317,10 +300,18 @@ _test_generic_punch() map_cmd=$4 filter_cmd=$5 testfile=$6 - multiple=1 + md5_cmd=$7 + multiple=$8 + + if [ -z "$md5_cmd" ]; then + md5_cmd=_md5_checksum + fi + if [ -z "$multiple" ]; then + multiple=1 + fi # - # If we are testing collapse range, we increare all the offsets of this + # If we are testing collapse range, we increase all the offsets of this # test by a factor of 4. We do this because unlike punch, collapse # range also decreases the size of file hence require bigger offsets. #
@@ -342,7 +333,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile echo " 2. into allocated space" if [ "$remove_testfile" ]; then
@@ -353,7 +344,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile if [ "$unwritten_tests" ]; then echo " 3. into unwritten space"
@@ -365,7 +356,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile fi echo " 4. hole -> data"
@@ -377,7 +368,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile if [ "$unwritten_tests" ]; then echo " 5. hole -> unwritten"
@@ -389,7 +380,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile fi echo " 6. data -> hole"
@@ -401,7 +392,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile if [ "$unwritten_tests" ]; then echo " 7. data -> unwritten"
@@ -414,7 +405,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile echo " 8. unwritten -> hole" if [ "$remove_testfile" ]; then
@@ -425,7 +416,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile echo " 9. unwritten -> data" if [ "$remove_testfile" ]; then
@@ -437,7 +428,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile fi echo " 10. hole -> data -> hole"
@@ -449,7 +440,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_12k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile echo " 11. data -> hole -> data" if [ "$remove_testfile" ]; then
@@ -463,7 +454,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_12k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile if [ "$unwritten_tests" ]; then echo " 12. unwritten -> data -> unwritten"
@@ -476,7 +467,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_12k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile echo " 13. data -> unwritten -> data" if [ "$remove_testfile" ]; then
@@ -489,7 +480,7 @@ _test_generic_punch() -c "$zero_cmd $_4k $_12k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile fi # Don't need to check EOF case for collapse range.
@@ -503,7 +494,7 @@ _test_generic_punch() -c "$zero_cmd $_12k $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile fi if [ "$zero_cmd" == "fcollapse" ]; then
@@ -520,7 +511,7 @@ _test_generic_punch() -c "$zero_cmd 0 $_8k" \ -c "$map_cmd -v" $testfile | $filter_cmd [ $? -ne 0 ] && die_now - _md5_checksum $testfile + $md5_cmd $testfile # If zero_cmd is fcollpase, don't check unaligned offsets if [ "$zero_cmd" == "fcollapse" ]; then
@@ -545,7 +536,7 @@ _test_generic_punch() diff $testfile $testfile.2 [ $? -ne 0 ] && die_now rm -f $testfile.2 - _md5_checksum $testfile + $md5_cmd $testfile # different file sizes mean we can't use md5sum to check the hole is # valid. Hence use hexdump to dump the contents and chop off the last
@@ -561,7 +552,7 @@ _test_generic_punch() $XFS_IO_PROG -f -c "truncate $block_size" \ -c "pwrite 0 $block_size" $sync_cmd \ -c "$zero_cmd 128 128" \ - -c "$map_cmd -v" $testfile | $filter_cmd | \ + -c "$map_cmd -v" $testfile | $filter_cmd $block_size | \ sed -e "s/\.\.[0-9]*\]/..7\]/" [ $? -ne 0 ] && die_now od -x $testfile | head -n -1
tests/xfs/242 +74 -2
97 lines not on this page · raw message
tests/xfs/242.out +153 -58
232 lines not on this page · raw message
Signature · 1 line
1.7.1
Quoted 4 lines · Dwight Engen
This test was failing on sparc64 because there is a minimum granularity of
PAGE_CACHE_SIZEin xfs_vnodeops.c:xfs_zero_file_space(). This change follows the approach taken in xfs/194 to filter the bmap output to be in terms of "blocksize" which is computed from pagesize.
xfs/194 existed long before xfs/242, so it's not necessarily the best example to follow. You've missed various things that make the special hackery xfs/194 does to make it work. e.g. clearing mkfs/mount options. Your change doesn't do this, so it will make it fail on CRC enable XFS filesystems because 4k / 8 = 512 bytes and that's smaller than the minimum block size support on CRC enabled XFS filesystems.
Quoted 5 lines
_test_generic_punchis modified to optionally take multiple as an argument, so the file under test will be twice the size on an 8k machine as a 4k machine. Since the files will be different sizes, we can no longer use md5sum so od -x is used instead with the byte offsets converted to "blocksize" offsets.
Brian posted patches yesterday on the XFS list to fix zero range problems, and they remove the page size rounding from xfs_zero_file_space(). Hence this strange corner case behaviour is likely to go away real soon, and so I don't think we should change the test to work around it now...
Would would be much more useful for you to do would with a platform like sparc64 is use it to test MKFS_OPTION="-b size=8k" and make all these extent-map-output dependent tests work properly with >4k block size filesystems. ;)
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 7 lines · On Wed, 8 Oct 2014 15:33:41 +1100 Dave Chinner <[email protected]> wrote:
Quoted 5 lines · Dwight Engen
This test was failing on sparc64 because there is a minimum granularity of
PAGE_CACHE_SIZEin xfs_vnodeops.c:xfs_zero_file_space(). This change follows the approach taken in xfs/194 to filter the bmap output to be in terms of "blocksize" which is computed from pagesize.
Quoted 7 lines
xfs/194 existed long before xfs/242, so it's not necessarily the best example to follow. You've missed various things that make the special hackery xfs/194 does to make it work. e.g. clearing mkfs/mount options. Your change doesn't do this, so it will make it fail on CRC enable XFS filesystems because 4k / 8 = 512 bytes and that's smaller than the minimum block size support on CRC enabled XFS filesystems.
Hi Dave, I sure did miss that 194 was doing unset MKFS_OPTIONS. Partly because running check '*/194' outputs:
MKFS_OPTIONS -- -f -bsize=4096 /dev/vdiskd
but this is only for the mkfs that check itself does, and as you note 194 will redo the mkfs with its computed block size. Thats a bit confusing :( 242 doesn't do a second mkfs, so there isn't really a need to unset MKFS_OPTIONS.
What I thought 194 was doing, and what I made 242 do was to be page size independent by computing the size of things and filtering the output to be in units of some factor of a page size, which is why I put blksize in quotes, its not the actual block size (ie we don't pass it to mkfs). Maybe I should have used a different name for less confusion, suggestions welcome. Doing this does make it so 242 passes with no MKFS_OPTIONS (which will be 4k blocksize) or with MKFS_OPTIONS="-b size=8k", and both of those with -m crc=1 also.
Quoted 11 lines
Quoted 5 lines
_test_generic_punchis modified to optionally take multiple as an argument, so the file under test will be twice the size on an 8k machine as a 4k machine. Since the files will be different sizes, we can no longer use md5sum so od -x is used instead with the byte offsets converted to "blocksize" offsets.Brian posted patches yesterday on the XFS list to fix zero range problems, and they remove the page size rounding from
xfs_zero_file_space(). Hence this strange corner case behaviour is likely to go away real soon, and so I don't think we should change the test to work around it now...
That sounds good, but I'd think we want 242 to be able to pass regardless of page size on 3.12-3.17 too?
Quoted 4 lines
Would would be much more useful for you to do would with a platform like sparc64 is use it to test MKFS_OPTION="-b size=8k" and make all these extent-map-output dependent tests work properly with >4k block size filesystems. ;)
This does do that for 242 :) I can look at the others also if the approach here is okay. Thanks.
Quoted 3 lines
Cheers,
Dave.
Hi Dave, any further comments on this?
Quoted 68 lines · On Thu, 16 Oct 2014 11:29:49 -0400 Dwight Engen <[email protected]> wrote:
Quoted 7 lines · On Wed, 8 Oct 2014 15:33:41 +1100 Dave Chinner <[email protected]> wrote:
Quoted 5 lines · Dwight Engen
This test was failing on sparc64 because there is a minimum granularity of
PAGE_CACHE_SIZEin xfs_vnodeops.c:xfs_zero_file_space(). This change follows the approach taken in xfs/194 to filter the bmap output to be in terms of "blocksize" which is computed from pagesize.Quoted 7 lines
xfs/194 existed long before xfs/242, so it's not necessarily the best example to follow. You've missed various things that make the special hackery xfs/194 does to make it work. e.g. clearing mkfs/mount options. Your change doesn't do this, so it will make it fail on CRC enable XFS filesystems because 4k / 8 = 512 bytes and that's smaller than the minimum block size support on CRC enabled XFS filesystems.
Hi Dave, I sure did miss that 194 was doing unset
MKFS_OPTIONS. Partly because running check '*/194' outputs:
MKFS_OPTIONS-- -f -bsize=4096/dev/vdiskdbut this is only for the mkfs that check itself does, and as you note 194 will redo the mkfs with its computed block size. Thats a bit confusing :( 242 doesn't do a second mkfs, so there isn't really a need to unset
MKFS_OPTIONS.What I thought 194 was doing, and what I made 242 do was to be page size independent by computing the size of things and filtering the output to be in units of some factor of a page size, which is why I put blksize in quotes, its not the actual block size (ie we don't pass it to mkfs). Maybe I should have used a different name for less confusion, suggestions welcome. Doing this does make it so 242 passes with no
MKFS_OPTIONS(which will be 4k blocksize) or with MKFS_OPTIONS="-b size=8k", and both of those with -m crc=1 also.Quoted 11 lines
Quoted 5 lines
_test_generic_punchis modified to optionally take multiple as an argument, so the file under test will be twice the size on an 8k machine as a 4k machine. Since the files will be different sizes, we can no longer use md5sum so od -x is used instead with the byte offsets converted to "blocksize" offsets.Brian posted patches yesterday on the XFS list to fix zero range problems, and they remove the page size rounding from
xfs_zero_file_space(). Hence this strange corner case behaviour is likely to go away real soon, and so I don't think we should change the test to work around it now...That sounds good, but I'd think we want 242 to be able to pass regardless of page size on 3.12-3.17 too?
Quoted 4 lines
Would would be much more useful for you to do would with a platform like sparc64 is use it to test MKFS_OPTION="-b size=8k" and make all these extent-map-output dependent tests work properly with >4k block size filesystems. ;)
This does do that for 242 :) I can look at the others also if the approach here is okay. Thanks.
Quoted 3 lines
Cheers,
Dave.
--
To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html