xfstests: generic/255: Execute only if blocksize <= 4096
diff mbox

Message ID 2909046.09LYLQvZ6q@localhost.localdomain
State Not Applicable
Headers show

Commit Message

Chandan Rajendra Aug. 25, 2013, 3:06 p.m. UTC
The patch adds the function _require_le_4k_blocksize to perform the
block size check on $TEST_DEV and $SCRATCH_DEV.

Signed-off-by: chandan <chandan@linux.vnet.ibm.com>
---
 common/rc         | 10 ++++++++++
 tests/generic/255 |  1 +
 2 files changed, 11 insertions(+)

Comments

Eric Sandeen Aug. 25, 2013, 4:54 p.m. UTC | #1
On 8/25/13 10:06 AM, chandan wrote:
> The patch adds the function _require_le_4k_blocksize to perform the
> block size check on $TEST_DEV and $SCRATCH_DEV.
> 
> Signed-off-by: chandan <chandan@linux.vnet.ibm.com>

Can you explain why this is necessary?

What failures do you see, on what filesystems?

That kind of info needs to be in the changelog so reviewers
can understand the need for the change (and to be sure
it's not papering over an actual bug for block sizes
> 4k...)

Thanks,
-Eric

> ---
>  common/rc         | 10 ++++++++++
>  tests/generic/255 |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index ae80b12..d8ee132 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2106,6 +2106,16 @@ _require_dumpe2fs()
>  	fi
>  }
>  
> +_require_le_4k_blocksize()
> +{
> +	test_dev_bs=`stat -f $TEST_DEV | grep "Block size" | cut -d " " -f3`
> +	scratch_dev_bs=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
> +
> +	if (( $test_dev_bs > 4096 || $scratch_dev_bs > 4096 )); then
> +		_notrun "This test requires a filesystem with a block size less than or equal to 4k."
> +	fi
> +}
> +
>  _create_loop_device()
>  {
>  	file=$1
> diff --git a/tests/generic/255 b/tests/generic/255
> index dd329b4..763e861 100755
> --- a/tests/generic/255
> +++ b/tests/generic/255
> @@ -49,6 +49,7 @@ _supported_os Linux
>  _require_xfs_io_falloc_punch
>  _require_xfs_io_falloc
>  _require_xfs_io_fiemap
> +_require_le_4k_blocksize
>  
>  testfile=$TEST_DIR/255.$$
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chandan Rajendra Aug. 26, 2013, 5:38 a.m. UTC | #2
On Sunday, August 25, 2013 11:54:30 AM Eric Sandeen wrote:
> Can you explain why this is necessary?
> 
> What failures do you see, on what filesystems?

generic/255 currently fails on Btrfs on a ppc64 machine with 64k page size and
hence 64k block size.

generic/255 has been written to test the corner cases for 4k block size. I did
try to make it work with variable sized block sizes, But I got stuck
working with md5sum (since we would need two sets of md5sums, due to
_test_generic_punch() being invoked with and without '-k' option per block
size). 

Since 4k block size support for Btrfs on ppc64 is already being worked on, I
think its better to prevent execution of generic/255 for block sizes greater
than 4k.

Apologies for not including the above description in the patch.

- chandan

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Aug. 26, 2013, 2:29 p.m. UTC | #3
On 8/26/13 12:38 AM, chandan wrote:
> On Sunday, August 25, 2013 11:54:30 AM Eric Sandeen wrote:
>> Can you explain why this is necessary?
>>
>> What failures do you see, on what filesystems?
> 
> generic/255 currently fails on Btrfs on a ppc64 machine with 64k page size and
> hence 64k block size.
> 
> generic/255 has been written to test the corner cases for 4k block size. I did
> try to make it work with variable sized block sizes, But I got stuck
> working with md5sum (since we would need two sets of md5sums, due to
> _test_generic_punch() being invoked with and without '-k' option per block
> size). 
> 
> Since 4k block size support for Btrfs on ppc64 is already being worked on, I
> think its better to prevent execution of generic/255 for block sizes greater
> than 4k.
> 
> Apologies for not including the above description in the patch.

It happens.  ;)

To be honest I haven't really looked at how _test_generic_punch & generic/255
works lately.

Just as a sanity check, does it also fail on xfs for 64k block sizes on ppc64?

Thanks,
-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chandan Rajendra Aug. 27, 2013, 12:16 p.m. UTC | #4
On Monday, August 26, 2013 09:29:31 AM Eric Sandeen wrote:
> 
> Just as a sanity check, does it also fail on xfs for 64k block sizes on ppc64?
> 

Yes, it does fail as shown below:

generic/255      - output mismatch (see /home/chandan/xfstests/results//generic/255.out.bad)
    --- tests/generic/255.out   2013-08-27 06:59:08.241344176 -0400
    +++ /home/chandan/xfstests/results//generic/255.out.bad     2013-08-27 07:01:45.311352161 -0400
    @@ -2,306 +2,202 @@
        1. into a hole
     daa100df6e6711906b61c9ab5aa16032
        2. into allocated space
    -0: [0..7]: extent
    -1: [8..23]: hole
    -2: [24..39]: extent
    +0: [0..127]: extent
     ...
     (Run 'diff -u tests/generic/255.out /home/chandan/xfstests/results//generic/255.out.bad' to see the entire diff)

In this instance, the test basically ends up punching a hole within a block
and hence fails.

- chandan

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/common/rc b/common/rc
index ae80b12..d8ee132 100644
--- a/common/rc
+++ b/common/rc
@@ -2106,6 +2106,16 @@  _require_dumpe2fs()
 	fi
 }
 
+_require_le_4k_blocksize()
+{
+	test_dev_bs=`stat -f $TEST_DEV | grep "Block size" | cut -d " " -f3`
+	scratch_dev_bs=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
+
+	if (( $test_dev_bs > 4096 || $scratch_dev_bs > 4096 )); then
+		_notrun "This test requires a filesystem with a block size less than or equal to 4k."
+	fi
+}
+
 _create_loop_device()
 {
 	file=$1
diff --git a/tests/generic/255 b/tests/generic/255
index dd329b4..763e861 100755
--- a/tests/generic/255
+++ b/tests/generic/255
@@ -49,6 +49,7 @@  _supported_os Linux
 _require_xfs_io_falloc_punch
 _require_xfs_io_falloc
 _require_xfs_io_fiemap
+_require_le_4k_blocksize
 
 testfile=$TEST_DIR/255.$$