| Message ID | 2909046.09LYLQvZ6q@localhost.localdomain (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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
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
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
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
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.$$
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(+)