Re: [PATCH 08/11] xfs: Check for extent overflow when moving extent from cow to data fork

Chandan Babu R
2020-11-18T05:20:47Z

On Saturday 14 November 2020 6:12:32 AM IST Darrick J. Wong wrote:
> On Fri, Nov 13, 2020 at 04:57:00PM +0530, Chandan Babu R wrote:
> > This test verifies that XFS does not cause inode fork's extent count to
> > overflow when writing to a shared extent.
> >
> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> > ---
> > tests/xfs/528 | 87 +++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/528.out | 8 +++++
> > tests/xfs/group | 1 +
> > 3 files changed, 96 insertions(+)
> > create mode 100755 tests/xfs/528
> > create mode 100644 tests/xfs/528.out
> >
> > diff --git a/tests/xfs/528 b/tests/xfs/528
> > new file mode 100755
> > index 00000000..0d39f05e
> > --- /dev/null
> > +++ b/tests/xfs/528
> > @@ -0,0 +1,87 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2020 Chandan Babu R. All Rights Reserved.
> > +#
> > +# FS QA Test 528
> > +#
> > +# Verify that XFS does not cause inode fork's extent count to overflow when
> > +# writing to a shared extent.
> > +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()
> > +{
> > + cd /
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/reflink
> > +. ./common/inject
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +
> > +_supported_fs xfs
> > +_require_scratch
> > +_require_scratch_reflink
> > +_require_xfs_debug
> > +_require_xfs_io_command "reflink"
> > +_require_xfs_io_error_injection "reduce_max_iextents"
> > +
> > +echo "* Write to shared extent"
> > +
> > +echo "Format and mount fs"
> > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> > +_scratch_mount >> $seqres.full
> > +
> > +bsize=$(_get_block_size $SCRATCH_MNT)
>
> Now that we're playing with regular files again -- should this be
> _get_file_block_size ? I think the same question applies to patches 2,
> 3, and 4, and perhaps the next one too.
>
> (Note that regular files can have cluster sizes that aren't the same as
> the fs block size if I set MKFS_OPTIONS="-d rtinherit=1 -r extsize=64k".)

Patch 2 computes the size of the rt volume as a function of the bitmap
inode. The data associated with bitmap inode is stored in the regular
filesystem space. Hence filesystem block size is the appropriate choice here.
The same applies to quota inode extent count overflow test.

The test included in the next patch requires reflink to be enabled. Hence
filesystem block size is the correct choice.

For the other tests that you have mentioned and also for the fstress test I
will use _get_file_block_size().

>
> --D
>
> > +
> > +srcfile=${SCRATCH_MNT}/srcfile
> > +dstfile=${SCRATCH_MNT}/dstfile
> > +
> > +nr_blks=15
> > +
> > +echo "Create a \$srcfile having an extent of length $nr_blks blocks"
> > +xfs_io -f -c "pwrite -b $((nr_blks * bsize)) 0 $((nr_blks * bsize))" \
> > + -c fsync $srcfile >> $seqres.full
> > +
> > +echo "Share the extent with \$dstfile"
> > +xfs_io -f -c "reflink $srcfile" $dstfile >> $seqres.full
> > +
> > +echo "Inject reduce_max_iextents error tag"
> > +xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > +echo "Buffered write to every other block of \$dstfile's shared extent"
> > +for i in $(seq 1 2 $((nr_blks - 1))); do
> > + xfs_io -f -c "pwrite $((i * bsize)) $bsize" -c fsync $dstfile \
> > + >> $seqres.full 2>&1
> > + [[ $? != 0 ]] && break
> > +done
> > +
> > +ino=$(stat -c "%i" $dstfile)
> > +
> > +_scratch_unmount >> $seqres.full
> > +
> > +echo "Verify \$dstfile's extent count"
> > +
> > +nextents=$(_scratch_get_iext_count $ino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > +if (( $nextents > 10 )); then
> > + echo "Extent count overflow check failed: nextents = $nextents"
> > +fi
> > +
> > +# success, all done
> > +status=0
> > +exit
> > +
> > diff --git a/tests/xfs/528.out b/tests/xfs/528.out
> > new file mode 100644
> > index 00000000..8666488b
> > --- /dev/null
> > +++ b/tests/xfs/528.out
> > @@ -0,0 +1,8 @@
> > +QA output created by 528
> > +* Write to shared extent
> > +Format and mount fs
> > +Create a $srcfile having an extent of length 15 blocks
> > +Share the extent with $dstfile
> > +Inject reduce_max_iextents error tag
> > +Buffered write to every other block of $dstfile's shared extent
> > +Verify $dstfile's extent count
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index 627813fe..c85aac6b 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -525,3 +525,4 @@
> > 525 auto quick attr
> > 526 auto quick dir hardlink symlink
> > 527 auto quick
> > +528 auto quick reflink
>

--
chandan