On Sun, Dec 06, 2020 at 09:50:43PM +0800, Eryu Guan wrote:
> On Fri, Dec 04, 2020 at 04:13:54PM +0530, Ritesh Harjani wrote:
> > 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
> >
> > w/o the kernel fix we get below errors and mount fails
> >
> > [ 1461.988701] run fstests generic/613 at 2020-10-27 19:57:34
> > [ 1530.406645] ------------[ cut here ]------------
> > [ 1530.407332] would truncate bmap result
> > [ 1530.408956] WARNING: CPU: 0 PID: 6401 at fs/iomap/fiemap.c:116
> > iomap_bmap_actor+0x43/0x50
> > [ 1530.410607] Modules linked in:
> > [ 1530.411024] CPU: 0 PID: 6401 Comm: mount Tainted: G W
> > <...>
> > 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@xxxxxxxxxxxxx>
> > ---
> > common/rc | 10 +++++++
> > tests/generic/618 | 70 +++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/618.out | 3 ++
> > tests/generic/group | 1 +
> > 4 files changed, 84 insertions(+)
> > create mode 100755 tests/generic/618
> > create mode 100644 tests/generic/618.out
> >
> > diff --git a/common/rc b/common/rc
> > index b5a504e0dcb4..128d75226958 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1608,6 +1608,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/618 b/tests/generic/618
> > new file mode 100755
> > index 000000000000..45c14da80c06
> > --- /dev/null
> > +++ b/tests/generic/618
> > @@ -0,0 +1,70 @@
> > +#! /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/618
> > +#
> > +# 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 due to size limitation and also it
> > +# becomes too slow when doing an mkfs on a huge sparse ext4 FS with 1k bs.
> > +# Hence on ext4 run only for 4K bs.
> > +if [ "$FSTYP" == "ext4" ]; then
> > + _scratch_mkfs > /dev/null 2>&1
> > + blksz=$(sudo debugfs -R stats $SCRATCH_DEV 2> /dev/null |grep "Block size" |cut -d ':' -f 2)
> > + test $blksz -lt 4096 && _notrun "This test requires ext4 with minimum 4k bs"
> > +fi
>
> As this is a generic test, the same check should be done with ext2 and
> ext3. And actually this test requires > 16T fs support. So I'd suggest
> add a new helper, maybe called _require_16T_support, to check if $FSTYP
> supports filesystem size > 16T. And for now, we could only check for
> extN.
>
> Also, there's no need to run command with sudo in fstests, as all tests
> are required to be run by root.
>
> And there's a helper to get fs block size called _get_block_size, no
> need to parse output of debugfs.
Maybe something like (not tested)
_require_scratch_16T_support()
{
case $FSTYP in
ext2|ext3)
_notrun "$FSTYP doesn't support >16T filesystem"
ext4)
_scratch_mkfs >> $seqres.full 2>&1
_scratch_mount
local blocksize=$(_get_block_size $SCRATCH_MNT)
if [ $blocksize -lt 4096 ]; then
_notrun "This test requires >16T fs support"
fi
;;
*)
;;
esac
}
Thanks,
Eryu
