Re: [PATCH V4 02/11] xfs: Check for extent overflow when trivally adding a new extent

[Chandan Babu R <chandanrlinux@gmail.com>]

On 03 Mar 2021 at 23:08, Darrick J. Wong wrote:
> On Mon, Jan 18, 2021 at 11:50:13AM +0530, Chandan Babu R wrote:
>> This test verifies that XFS does not cause inode fork's extent count to
>> overflow when adding a single extent while there's no possibility of
>> splitting an existing mapping.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> ---
>>  tests/xfs/522     | 173 ++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/522.out |  20 ++++++
>>  tests/xfs/group   |   1 +
>>  3 files changed, 194 insertions(+)
>>  create mode 100755 tests/xfs/522
>>  create mode 100644 tests/xfs/522.out
>>
>> diff --git a/tests/xfs/522 b/tests/xfs/522
>> new file mode 100755
>> index 00000000..33f0591e
>> --- /dev/null
>> +++ b/tests/xfs/522
>> @@ -0,0 +1,173 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2020 Chandan Babu R.  All Rights Reserved.
>> +#
>> +# FS QA Test 522
>> +#
>> +# Verify that XFS does not cause inode fork's extent count to overflow when
>> +# adding a single extent while there's no possibility of splitting an existing
>> +# mapping.
>> +
>> +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/quota
>> +. ./common/inject
>> +. ./common/populate
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +
>> +_supported_fs xfs
>> +_require_scratch
>> +_require_xfs_quota
>> +_require_xfs_debug
>> +_require_test_program "punch-alternating"
>> +_require_xfs_io_command "falloc"
>> +_require_xfs_io_error_injection "reduce_max_iextents"
>> +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>> +
>> +echo "Format and mount fs"
>> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
>> +_scratch_mount -o uquota >> $seqres.full
>> +
>> +bsize=$(_get_file_block_size $SCRATCH_MNT)
>> +
>> +echo "* Delalloc to written extent conversion"
>
> What happens if delalloc is disabled?  e.g. the mkfs parameters have
> extent size hints set on the root directory?
>
> (I think it'll be fine since you write every other block...)

Yes, XFS ends up doing Direct IO and the file blocks are still fragmented
causing extent count to increase beyond the pseudo max limit of 10.

>
>> +testfile=$SCRATCH_MNT/testfile
>> +
>> +echo "Inject reduce_max_iextents error tag"
>> +_scratch_inject_error reduce_max_iextents 1
>> +
>> +nr_blks=$((15 * 2))
>> +
>> +echo "Create fragmented file"
>> +for i in $(seq 0 2 $((nr_blks - 1))); do
>> +	$XFS_IO_PROG -f -s -c "pwrite $((i * bsize)) $bsize" $testfile \
>> +	       >> $seqres.full 2>&1
>> +	[[ $? != 0 ]] && break
>> +done
>> +
>> +echo "Verify \$testfile's extent count"
>> +
>> +nextents=$($XFS_IO_PROG -f -c 'stat' $testfile | grep -i nextents)
>> +nextents=${nextents##fsxattr.nextents = }
>
> /me wonders if it's time for a helper to extract these fields...

Thanks for the suggestion. I will include a helper function in the next
version of the patchset.

>
>> +if (( $nextents > 10 )); then
>> +	echo "Extent count overflow check failed: nextents = $nextents"
>> +	exit 1
>> +fi
>> +
>> +rm $testfile
>> +
>> +echo "* Fallocate unwritten extents"
>> +
>> +echo "Fallocate fragmented file"
>> +for i in $(seq 0 2 $((nr_blks - 1))); do
>> +	$XFS_IO_PROG -f -c "falloc $((i * bsize)) $bsize" $testfile \
>> +	       >> $seqres.full 2>&1
>> +	[[ $? != 0 ]] && break
>> +done
>> +
>> +echo "Verify \$testfile's extent count"
>> +
>> +nextents=$($XFS_IO_PROG -f -c 'stat' $testfile | grep -i nextents)
>> +nextents=${nextents##fsxattr.nextents = }
>> +if (( $nextents > 10 )); then
>> +	echo "Extent count overflow check failed: nextents = $nextents"
>> +	exit 1
>> +fi
>> +
>> +rm $testfile
>> +
>> +echo "* Directio write"
>> +
>> +echo "Create fragmented file via directio writes"
>> +for i in $(seq 0 2 $((nr_blks - 1))); do
>> +	$XFS_IO_PROG -d -s -f -c "pwrite $((i * bsize)) $bsize" $testfile \
>> +	       >> $seqres.full 2>&1
>> +	[[ $? != 0 ]] && break
>> +done
>> +
>> +echo "Verify \$testfile's extent count"
>> +
>> +nextents=$($XFS_IO_PROG -f -c 'stat' $testfile | grep -i nextents)
>> +nextents=${nextents##fsxattr.nextents = }
>> +if (( $nextents > 10 )); then
>> +	echo "Extent count overflow check failed: nextents = $nextents"
>> +	exit 1
>> +fi
>> +
>> +rm $testfile
>> +
>> +echo "* Extend quota inodes"
>> +
>> +echo "Disable reduce_max_iextents error tag"
>> +_scratch_inject_error reduce_max_iextents 0
>> +
>> +echo "Consume free space"
>> +fillerdir=$SCRATCH_MNT/fillerdir
>> +nr_free_blks=$(stat -f -c '%f' $SCRATCH_MNT)
>> +nr_free_blks=$((nr_free_blks * 90 / 100))
>> +
>> +_fill_fs $((bsize * nr_free_blks)) $fillerdir $bsize 0 >> $seqres.full 2>&1
>> +
>> +echo "Create fragmented filesystem"
>> +for dentry in $(ls -1 $fillerdir/); do
>> +	$here/src/punch-alternating $fillerdir/$dentry >> $seqres.full
>> +done
>> +
>> +echo "Inject reduce_max_iextents error tag"
>> +_scratch_inject_error reduce_max_iextents 1
>> +
>> +echo "Inject bmap_alloc_minlen_extent error tag"
>> +_scratch_inject_error bmap_alloc_minlen_extent 1
>
> Are we testing to see if the fs blows up when the quota inodes hit max
> iext count?  If so, please put that in the comments for this section.

Sure, I will add a description.

>
>> +
>> +nr_blks=20
>> +
>> +# This is a rough calculation; It doesn't take block headers into
>> +# consideration.
>> +# gdb -batch vmlinux -ex 'print sizeof(struct xfs_dqblk)'
>> +# $1 = 136
>> +nr_quotas_per_block=$((bsize / 136))
>> +nr_quotas=$((nr_quotas_per_block * nr_blks))
>> +
>> +echo "Extend uquota file"
>> +for i in $(seq 0 $nr_quotas_per_block $nr_quotas); do
>> +	chown $i $testfile >> $seqres.full 2>&1
>> +	[[ $? != 0 ]] && break
>> +done
>> +
>> +_scratch_unmount >> $seqres.full
>> +
>> +echo "Verify uquota inode's extent count"
>> +uquotino=$(_scratch_xfs_db -c sb -c "print uquotino")
>> +uquotino=${uquotino##uquotino = }
>
> _scratch_xfs_get_metadata_field

I will fix this.

--
chandan