Re: [PATCH V4 06/11] xfs: Check for extent overflow when adding/removing dir entries

Chandan Babu R
2021-03-04T12:24:00Z

On 03 Mar 2021 at 23:31, Darrick J. Wong wrote:
> On Mon, Jan 18, 2021 at 11:50:17AM +0530, Chandan Babu R wrote:
>> This test verifies that XFS does not cause inode fork's extent count to
>> overflow when adding/removing directory entries.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> ---
>> tests/xfs/526 | 186 ++++++++++++++++++++++++++++++++++++++++++++++
>> tests/xfs/526.out | 17 +++++
>> tests/xfs/group | 1 +
>> 3 files changed, 204 insertions(+)
>> create mode 100755 tests/xfs/526
>> create mode 100644 tests/xfs/526.out
>>
>> diff --git a/tests/xfs/526 b/tests/xfs/526
>> new file mode 100755
>> index 00000000..5a789d61
>> --- /dev/null
>> +++ b/tests/xfs/526
>> @@ -0,0 +1,186 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2020 Chandan Babu R. All Rights Reserved.
>> +#
>> +# FS QA Test 526
>> +#
>> +# Verify that XFS does not cause inode fork's extent count to overflow when
>> +# adding/removing directory entries.
>> +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/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_debug
>> +_require_test_program "punch-alternating"
>> +_require_xfs_io_error_injection "reduce_max_iextents"
>> +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>> +
>> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) | _filter_mkfs >> $seqres.full 2> $tmp.mkfs
>> +. $tmp.mkfs
>> +
>> +# Filesystems with directory block size greater than one FSB will not be tested,
>> +# since "7 (i.e. XFS_DA_NODE_MAXDEPTH + 1 data block + 1 free block) * 2 (fsb
>> +# count) = 14" is greater than the pseudo max extent count limit of 10.
>> +# Extending the pseudo max limit won't help either. Consider the case where 1
>> +# FSB is 1k in size and 1 dir block is 64k in size (i.e. fsb count = 64). In
>> +# this case, the pseudo max limit has to be greater than 7 * 64 = 448 extents.
>> +if (( $dbsize != $dirbsize )); then
>> + _notrun "FSB size ($dbsize) and directory block size ($dirbsize) do not match"
>> +fi
>
> But what about the case where fsb is 1k and dirblocks are 4k? :)
>
> I admit I'm reacting to my expectation that we would _notrun here based
> on the output of a more computation, not just fsb != dirblocksize. But
> I dunno, maybe you've already worked that out?

Ok. I will replace the above with the following,

if (( $dirbsize > $dbsize )); then
_notrun "Directory block size ($dirbsize) is larger than FSB size ($dbsize)"
fi

>
> (The rest of the test looks good to me.)
>
> --D

--
chandan