diff mbox series

[1/1] generic: Add test to check for mounting a huge sparse dm device

Message ID daec44e9f2e3ce483b7845065db3bf148ff5cd2c.1603864280.git.riteshh@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [1/1] generic: Add test to check for mounting a huge sparse dm device | expand

Commit Message

Ritesh Harjani Oct. 28, 2020, 6:44 a.m. UTC 
Add this test (which Christian Kujau reported) to check for regression
caused due to ext4 bmap aops implementation was moved to use iomap APIs.
jbd2 calls bmap() kernel function from fs/inode.c which was failing since
iomap_bmap() implementation earlier returned 0 for block addr > INT_MAX.
This regression was fixed with following kernel commit [1]
commit b75dfde1212991b24b220c3995101c60a7b8ae74
("fibmap: Warn and return an error in case of block > INT_MAX")
[1]: https://patchwork.ozlabs.org/patch/1279914

w/o the kernel fix we get below error and mount fails

[ 1461.988701] run fstests generic/613 at 2020-10-27 19:57:34
[ 1530.511978] jbd2_journal_init_inode: Cannot locate journal superblock
[ 1530.513310] EXT4-fs (dm-1): Could not load journal inode

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 common/rc             | 10 +++++++
 tests/generic/613     | 66 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/613.out |  3 ++
 tests/generic/group   |  1 +
 4 files changed, 80 insertions(+)
 create mode 100755 tests/generic/613
 create mode 100644 tests/generic/613.out

--
2.26.2

Comments

Eryu Guan Nov. 1, 2020, 4:24 p.m. UTC | #1 
On Wed, Oct 28, 2020 at 12:14:52PM +0530, Ritesh Harjani wrote:
> Add this test (which Christian Kujau reported) to check for regression
> caused due to ext4 bmap aops implementation was moved to use iomap APIs.
> jbd2 calls bmap() kernel function from fs/inode.c which was failing since
> iomap_bmap() implementation earlier returned 0 for block addr > INT_MAX.
> This regression was fixed with following kernel commit [1]
> commit b75dfde1212991b24b220c3995101c60a7b8ae74
> ("fibmap: Warn and return an error in case of block > INT_MAX")
> [1]: https://patchwork.ozlabs.org/patch/1279914
> 
> w/o the kernel fix we get below error and mount fails
> 
> [ 1461.988701] run fstests generic/613 at 2020-10-27 19:57:34
> [ 1530.511978] jbd2_journal_init_inode: Cannot locate journal superblock
> [ 1530.513310] EXT4-fs (dm-1): Could not load journal inode
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>  common/rc             | 10 +++++++
>  tests/generic/613     | 66 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/613.out |  3 ++
>  tests/generic/group   |  1 +
>  4 files changed, 80 insertions(+)
>  create mode 100755 tests/generic/613
>  create mode 100644 tests/generic/613.out
> 
> diff --git a/common/rc b/common/rc
> index 27a27ea36f75..b0c353c4c107 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1607,6 +1607,16 @@ _require_scratch_size()
>  	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
>  }
> 
> +# require a scratch dev of a minimum size (in kb) and should not be checked
> +# post test
> +_require_scratch_size_nocheck()
> +{
> +	[ $# -eq 1 ] || _fail "_require_scratch_size: expected size param"
> +
> +	_require_scratch_nocheck
> +	local devsize=`_get_device_size $SCRATCH_DEV`
> +	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
> +}

Seems there's no need to introduce this new helper, just open-coded

# comments on why we use _nocheck here
_require_scratch_nocheck
_require_scratch_size $size

> 
>  # this test needs a test partition - check we're ok & mount it
>  #
> diff --git a/tests/generic/613 b/tests/generic/613
> new file mode 100755
> index 000000000000..b426ef91cacf
> --- /dev/null
> +++ b/tests/generic/613
> @@ -0,0 +1,66 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Christian Kujau. All Rights Reserved.
> +# Copyright (c) 2020 Ritesh Harjani. All Rights Reserved.
> +#
> +# FS QA Test generic/613
> +#
> +# Since the test is not specific to ext4, hence adding it to generic.
> +# Add this test to check for regression which was reported when ext4 bmap
> +# aops was moved to use iomap APIs. jbd2 calls bmap() kernel function
> +# from fs/inode.c which was failing since iomap_bmap() implementation earlier
> +# returned 0 for block addr > INT_MAX.
> +# This regression was fixed with following kernel commit [1]
> +# commit b75dfde1212991b24b220c3995101c60a7b8ae74
> +# ("fibmap: Warn and return an error in case of block > INT_MAX")
> +# [1]: https://patchwork.ozlabs.org/patch/1279914
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	_dmhugedisk_cleanup
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmhugedisk
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_dmhugedisk
> +_require_scratch_size_nocheck $((4 * 1024 * 1024)) #kB
> +
> +# For 1k bs with ext4, mkfs was failing maybe due to size limitation.

I think that's because only 4k blocksize ext4 supports filesystems
greater than 16T.

> +if [ "$FSTYP" = "ext4" ]; then
> +	export MKFS_OPTIONS="-F -b 4096"
> +fi

I'd check for fs block size after mkfs and _notrun if it's ext4 and
block_size < 4k. So we don't have to overwrite MKFS_OPTIONS and run the
test multiple times forcely in, for example ext4-1k config & ext4-4k
config.

Thanks,
Eryu

> +
> +# 17TB dm huge-test-zer0 device
> +# (in terms of 512 sectors)
> +sectors=$((2*1024*1024*1024*17))
> +chunk_size=128
> +
> +_dmhugedisk_init $sectors $chunk_size
> +_mkfs_dev $DMHUGEDISK_DEV
> +_mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
> +testfile=$SCRATCH_MNT/testfile-$seq
> +
> +$XFS_IO_PROG -fc "pwrite -S 0xaa 0 1m" -c "fsync" $testfile | _filter_xfs_io
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/613.out b/tests/generic/613.out
> new file mode 100644
> index 000000000000..4747b7596499
> --- /dev/null
> +++ b/tests/generic/613.out
> @@ -0,0 +1,3 @@
> +QA output created by 613
> +wrote 1048576/1048576 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> diff --git a/tests/generic/group b/tests/generic/group
> index 8054d874f005..360d145d2036 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -615,3 +615,4 @@
>  610 auto quick prealloc zero
>  611 auto quick attr
>  612 auto quick clone
> +613 auto mount quick
> --
> 2.26.2
Ritesh Harjani Dec. 4, 2020, 3:57 a.m. UTC | #2 
On 11/1/20 9:54 PM, Eryu Guan wrote:
> On Wed, Oct 28, 2020 at 12:14:52PM +0530, Ritesh Harjani wrote:
>> Add this test (which Christian Kujau reported) to check for regression
>> caused due to ext4 bmap aops implementation was moved to use iomap APIs.
>> jbd2 calls bmap() kernel function from fs/inode.c which was failing since
>> iomap_bmap() implementation earlier returned 0 for block addr > INT_MAX.
>> This regression was fixed with following kernel commit [1]
>> commit b75dfde1212991b24b220c3995101c60a7b8ae74
>> ("fibmap: Warn and return an error in case of block > INT_MAX")
>> [1]: https://patchwork.ozlabs.org/patch/1279914
>>
>> w/o the kernel fix we get below error and mount fails
>>
>> [ 1461.988701] run fstests generic/613 at 2020-10-27 19:57:34
>> [ 1530.511978] jbd2_journal_init_inode: Cannot locate journal superblock
>> [ 1530.513310] EXT4-fs (dm-1): Could not load journal inode
>>
>> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
>> ---
>>   common/rc             | 10 +++++++
>>   tests/generic/613     | 66 +++++++++++++++++++++++++++++++++++++++++++
>>   tests/generic/613.out |  3 ++
>>   tests/generic/group   |  1 +
>>   4 files changed, 80 insertions(+)
>>   create mode 100755 tests/generic/613
>>   create mode 100644 tests/generic/613.out
>>
>> diff --git a/common/rc b/common/rc
>> index 27a27ea36f75..b0c353c4c107 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -1607,6 +1607,16 @@ _require_scratch_size()
>>   	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
>>   }
>>
>> +# require a scratch dev of a minimum size (in kb) and should not be checked
>> +# post test
>> +_require_scratch_size_nocheck()
>> +{
>> +	[ $# -eq 1 ] || _fail "_require_scratch_size: expected size param"
>> +
>> +	_require_scratch_nocheck
>> +	local devsize=`_get_device_size $SCRATCH_DEV`
>> +	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
>> +}
> 
> Seems there's no need to introduce this new helper, just open-coded
> 
> # comments on why we use _nocheck here
> _require_scratch_nocheck
> _require_scratch_size $size

This does not work. Bcoz _require_scratch_size will create a
scratch file which I guess it will check at the end of the test.
If not there, then it reports FS as inconsistent.

I get below errors with above open coding this, as you mentioned above.
generic/618 138s ... _check_generic_filesystem: filesystem on /dev/loop3 
is inconsistent
(see 
/home/machine/src/tools-work/xfstests-dev/results//ext4_4k/generic/618.full 
for details)


> 
>>
>>   # this test needs a test partition - check we're ok & mount it
>>   #
>> diff --git a/tests/generic/613 b/tests/generic/613
>> new file mode 100755
>> index 000000000000..b426ef91cacf
>> --- /dev/null
>> +++ b/tests/generic/613
>> @@ -0,0 +1,66 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2020 Christian Kujau. All Rights Reserved.
>> +# Copyright (c) 2020 Ritesh Harjani. All Rights Reserved.
>> +#
>> +# FS QA Test generic/613
>> +#
>> +# Since the test is not specific to ext4, hence adding it to generic.
>> +# Add this test to check for regression which was reported when ext4 bmap
>> +# aops was moved to use iomap APIs. jbd2 calls bmap() kernel function
>> +# from fs/inode.c which was failing since iomap_bmap() implementation earlier
>> +# returned 0 for block addr > INT_MAX.
>> +# This regression was fixed with following kernel commit [1]
>> +# commit b75dfde1212991b24b220c3995101c60a7b8ae74
>> +# ("fibmap: Warn and return an error in case of block > INT_MAX")
>> +# [1]: https://patchwork.ozlabs.org/patch/1279914
>> +#
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +	_dmhugedisk_cleanup
>> +	cd /
>> +	rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +. ./common/dmhugedisk
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# Modify as appropriate.
>> +_supported_fs generic
>> +_require_dmhugedisk
>> +_require_scratch_size_nocheck $((4 * 1024 * 1024)) #kB
>> +
>> +# For 1k bs with ext4, mkfs was failing maybe due to size limitation.
> 
> I think that's because only 4k blocksize ext4 supports filesystems
> greater than 16T.
> 
>> +if [ "$FSTYP" = "ext4" ]; then
>> +	export MKFS_OPTIONS="-F -b 4096"
>> +fi
> 
> I'd check for fs block size after mkfs and _notrun if it's ext4 and
> block_size < 4k. So we don't have to overwrite MKFS_OPTIONS and run the
> test multiple times forcely in, for example ext4-1k config & ext4-4k
> config.

Yup, I agree with this. Let me send a v2 addressing above comment.


> 
> 
>> +
>> +# 17TB dm huge-test-zer0 device
>> +# (in terms of 512 sectors)
>> +sectors=$((2*1024*1024*1024*17))
>> +chunk_size=128
>> +
>> +_dmhugedisk_init $sectors $chunk_size
>> +_mkfs_dev $DMHUGEDISK_DEV
>> +_mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
>> +testfile=$SCRATCH_MNT/testfile-$seq
>> +
>> +$XFS_IO_PROG -fc "pwrite -S 0xaa 0 1m" -c "fsync" $testfile | _filter_xfs_io
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/generic/613.out b/tests/generic/613.out
>> new file mode 100644
>> index 000000000000..4747b7596499
>> --- /dev/null
>> +++ b/tests/generic/613.out
>> @@ -0,0 +1,3 @@
>> +QA output created by 613
>> +wrote 1048576/1048576 bytes at offset 0
>> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> diff --git a/tests/generic/group b/tests/generic/group
>> index 8054d874f005..360d145d2036 100644
>> --- a/tests/generic/group
>> +++ b/tests/generic/group
>> @@ -615,3 +615,4 @@
>>   610 auto quick prealloc zero
>>   611 auto quick attr
>>   612 auto quick clone
>> +613 auto mount quick
>> --
>> 2.26.2
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 27a27ea36f75..b0c353c4c107 100644
--- a/common/rc
+++ b/common/rc
@@ -1607,6 +1607,16 @@  _require_scratch_size()
 	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
 }

+# require a scratch dev of a minimum size (in kb) and should not be checked
+# post test
+_require_scratch_size_nocheck()
+{
+	[ $# -eq 1 ] || _fail "_require_scratch_size: expected size param"
+
+	_require_scratch_nocheck
+	local devsize=`_get_device_size $SCRATCH_DEV`
+	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
+}

 # this test needs a test partition - check we're ok & mount it
 #
diff --git a/tests/generic/613 b/tests/generic/613
new file mode 100755
index 000000000000..b426ef91cacf
--- /dev/null
+++ b/tests/generic/613
@@ -0,0 +1,66 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Christian Kujau. All Rights Reserved.
+# Copyright (c) 2020 Ritesh Harjani. All Rights Reserved.
+#
+# FS QA Test generic/613
+#
+# Since the test is not specific to ext4, hence adding it to generic.
+# Add this test to check for regression which was reported when ext4 bmap
+# aops was moved to use iomap APIs. jbd2 calls bmap() kernel function
+# from fs/inode.c which was failing since iomap_bmap() implementation earlier
+# returned 0 for block addr > INT_MAX.
+# This regression was fixed with following kernel commit [1]
+# commit b75dfde1212991b24b220c3995101c60a7b8ae74
+# ("fibmap: Warn and return an error in case of block > INT_MAX")
+# [1]: https://patchwork.ozlabs.org/patch/1279914
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_dmhugedisk_cleanup
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmhugedisk
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# Modify as appropriate.
+_supported_fs generic
+_require_dmhugedisk
+_require_scratch_size_nocheck $((4 * 1024 * 1024)) #kB
+
+# For 1k bs with ext4, mkfs was failing maybe due to size limitation.
+if [ "$FSTYP" = "ext4" ]; then
+	export MKFS_OPTIONS="-F -b 4096"
+fi
+
+# 17TB dm huge-test-zer0 device
+# (in terms of 512 sectors)
+sectors=$((2*1024*1024*1024*17))
+chunk_size=128
+
+_dmhugedisk_init $sectors $chunk_size
+_mkfs_dev $DMHUGEDISK_DEV
+_mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
+testfile=$SCRATCH_MNT/testfile-$seq
+
+$XFS_IO_PROG -fc "pwrite -S 0xaa 0 1m" -c "fsync" $testfile | _filter_xfs_io
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/613.out b/tests/generic/613.out
new file mode 100644
index 000000000000..4747b7596499
--- /dev/null
+++ b/tests/generic/613.out
@@ -0,0 +1,3 @@ 
+QA output created by 613
+wrote 1048576/1048576 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/generic/group b/tests/generic/group
index 8054d874f005..360d145d2036 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -615,3 +615,4 @@ 
 610 auto quick prealloc zero
 611 auto quick attr
 612 auto quick clone
+613 auto mount quick