On Saturday 14 November 2020 6:07:54 AM IST Darrick J. Wong wrote:
> On Fri, Nov 13, 2020 at 04:56:58PM +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@xxxxxxxxx>
> > ---
> > tests/xfs/526 | 360 ++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/526.out | 47 ++++++
> > tests/xfs/group | 1 +
> > 3 files changed, 408 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..39cfbcf8
> > --- /dev/null
> > +++ b/tests/xfs/526
> > @@ -0,0 +1,360 @@
> > +#! /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
> > +
> > +# 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
> > +
> > +dir_entry_create()
> > +{
> > + echo "* Create directory entries"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
>
> Same questions as I've had for all the other tests -- why is it critical to
> reformat between each test? Why is it necessary to encode uuids in the
> attr/dir names?
I will replace uuids with output from printf.
However w.r.t not reformatting the filesystem between each test, we need to
look into one issue that occurs when executing either of dir_entry_rename_src
and dir_entry_remove tests. Please see dir_entry_remove below for an
explanation.
>
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Inject bmap_alloc_minlen_extent error tag"
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + echo "Create directory entries"
> > + dent_len=$(uuidgen | wc -c)
>
> Also, it'll explode the directories faster if you use maximal length
> names (255) instead of uuids (37).
>
> The same applies to the xattr tests in the previous patch.
Thanks for the suggestion. I will implement it in the next version of the
patchset.
>
> > + nr_dents=$((dbsize * 20 / dent_len))
> > + for i in $(seq 1 $nr_dents); do
> > + touch $SCRATCH_MNT/$(uuidgen) >> $seqres.full 2>&1 || break
> > + done
> > +
> > + dirino=$(stat -c "%i" $SCRATCH_MNT)
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + echo "Verify directory's extent count"
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > + if (( $nextents > 10 )); then
> > + echo "Extent count overflow check failed: nextents = $nextents"
> > + fi
> > +}
> > +
> > +dir_entry_rename_dst()
> > +{
> > + echo "* Rename: Populate destination directory"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Inject bmap_alloc_minlen_extent error tag"
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + dstdir=$SCRATCH_MNT/dstdir
> > + mkdir $dstdir
> > +
> > + dent_len=$(uuidgen | wc -c)
> > + nr_dents=$((dirbsize * 20 / dent_len))
> > +
> > + echo "Populate \$dstdir by mv-ing new directory entries"
> > + for i in $(seq 1 $nr_dents); do
> > + file=${SCRATCH_MNT}/$(uuidgen)
> > + touch $file || break
> > + mv $file $dstdir >> $seqres.full 2>&1 || break
> > + done
> > +
> > + dirino=$(stat -c "%i" $dstdir)
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + echo "Verify \$dstdir's extent count"
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > + if (( $nextents > 10 )); then
> > + echo "Extent count overflow check failed: nextents = $nextents"
> > + fi
> > +}
> > +
> > +dir_entry_rename_src()
> > +{
> > + echo "* Rename: Populate source directory and mv one entry to destination directory"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + srcdir=${SCRATCH_MNT}/srcdir
> > + dstdir=${SCRATCH_MNT}/dstdir
> > +
> > + mkdir $srcdir $dstdir
> > +
> > + dirino=$(stat -c "%i" $srcdir)
> > +
> > + dent_len=$(uuidgen | wc -c)
> > + nr_dents=$((dirbsize / dent_len))
> > + nextents=0
> > + last=""
> > +
> > + echo "Populate \$srcdir with atleast 4 extents"
> > + while (( $nextents < 4 )); do
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + for i in $(seq 1 $nr_dents); do
> > + last=${srcdir}/$(uuidgen)
> > + touch $last || break
> > + done
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > +
> > + _scratch_mount >> $seqres.full
> > + done
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Move an entry from \$srcdir to trigger -EFBIG"
> > + mv $last $dstdir >> $seqres.full 2>&1
> > + if [[ $? == 0 ]]; then
> > + echo "Moving from \$srcdir to \$dstdir succeeded; Should have failed"
> > + fi
> > +
> > + _scratch_unmount >> $seqres.full
> > +}
> > +
> > +dir_entry_create_hard_links()
> > +{
> > + echo "* Create multiple hard links to a single file"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Inject bmap_alloc_minlen_extent error tag"
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + dent_len=$(uuidgen | wc -c)
> > + nr_dents=$((dirbsize * 20 / dent_len))
> > +
> > + echo "Create multiple hardlinks"
> > + for i in $(seq 1 $nr_dents); do
> > + ln $testfile ${SCRATCH_MNT}/$(uuidgen) >> $seqres.full 2>&1 || break
> > + done
> > +
> > + dirino=$(stat -c "%i" $SCRATCH_MNT)
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + echo "Verify directory's extent count"
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > + if (( $nextents > 10 )); then
> > + echo "Extent count overflow check failed: nextents = $nextents"
> > + fi
> > +}
> > +
> > +dir_entry_create_symlinks()
> > +{
> > + echo "* Create multiple symbolic links to a single file"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Inject bmap_alloc_minlen_extent error tag"
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + dent_len=$(uuidgen | wc -c)
> > + nr_dents=$((dirbsize * 20 / dent_len))
> > +
> > + echo "Create multiple symbolic links"
> > + for i in $(seq 1 $nr_dents); do
> > + ln -s $testfile ${SCRATCH_MNT}/$(uuidgen) >> $seqres.full 2>&1 || break;
> > + done
> > +
> > + dirino=$(stat -c "%i" $SCRATCH_MNT)
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + echo "Verify directory's extent count"
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > + if (( $nextents > 10 )); then
> > + echo "Extent count overflow check failed: nextents = $nextents"
> > + fi
> > +}
> > +
> > +dir_entry_remove()
> > +{
> > + echo "* Populate a directory and remove one entry"
> > +
> > + echo "Format and mount fs"
> > + _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> > + _scratch_mount >> $seqres.full
> > +
> > + testfile=$SCRATCH_MNT/testfile
> > +
> > + echo "Consume free space"
> > + dd if=/dev/zero of=${testfile} bs=${dbsize} >> $seqres.full 2>&1
> > + sync
> > +
> > + echo "Create fragmented filesystem"
> > + $here/src/punch-alternating $testfile >> $seqres.full
> > + sync
> > +
> > + dirino=$(stat -c "%i" $SCRATCH_MNT)
> > +
> > + dent_len=$(uuidgen | wc -c)
> > + nr_dents=$((dirbsize / dent_len))
> > + nextents=0
> > + last=""
> > +
> > + echo "Populate directory with atleast 4 extents"
> > + while (( $nextents < 4 )); do
> > + xfs_io -x -c 'inject bmap_alloc_minlen_extent' $SCRATCH_MNT
> > +
> > + for i in $(seq 1 $nr_dents); do
> > + last=${SCRATCH_MNT}/$(uuidgen)
> > + touch $last || break
> > + done
> > +
> > + _scratch_unmount >> $seqres.full
> > +
> > + nextents=$(_scratch_get_iext_count $dirino data || \
> > + _fail "Unable to obtain inode fork's extent count")
> > +
> > + _scratch_mount >> $seqres.full
> > + done
> > +
> > + echo "Inject reduce_max_iextents error tag"
> > + xfs_io -x -c 'inject reduce_max_iextents' $SCRATCH_MNT
> > +
> > + echo "Remove an entry from directory to trigger -EFBIG"
> > + rm $last >> $seqres.full 2>&1
> > + if [[ $? == 0 ]]; then
> > + echo "Removing file succeeded; Should have failed"
> > + fi
> > +
> > + _scratch_unmount >> $seqres.full
> > +}
In the above function we create a directory with atleast 4 extents and this is
executed without the pseudo max extent limits enabled. Later the function
injects reduce_max_iextents error tag and since 4 + (XFS_DA_NODE_MAXDEPTH + 1
+ 1) = 4 + (5 + 1 + 1) = 11 > 10 (pseudo max limit) the directory entry remove
operation execute later would fail. This also would mean that the parent
directory cannot be deleted with the error tag enabled. This scenario would
not occur in production work load since the extent count limit is always
enabled as against enabling it at a later stage as is being done in this test.
This is different from the attr_remove() test from patch 5 where it is
possible to remove the file even though the corresponding xattr cannot be
removed.
Hence, I could either unmount and mount the filesystem so that the parent
directory can be removed or I could create the filesystem anew.
> > +
> > +# 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"
>
> Heh, I had wondered about that. Good that you caught this here!
>
> --D
>
> > +fi
> > +
> > +dir_entry_create
> > +dir_entry_rename_dst
> > +dir_entry_rename_src
> > +dir_entry_create_hard_links
> > +dir_entry_create_symlinks
> > +dir_entry_remove
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/526.out b/tests/xfs/526.out
> > new file mode 100644
> > index 00000000..21f77cd8
> > --- /dev/null
> > +++ b/tests/xfs/526.out
> > @@ -0,0 +1,47 @@
> > +QA output created by 526
> > +* Create directory entries
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Inject reduce_max_iextents error tag
> > +Inject bmap_alloc_minlen_extent error tag
> > +Create directory entries
> > +Verify directory's extent count
> > +* Rename: Populate destination directory
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Inject reduce_max_iextents error tag
> > +Inject bmap_alloc_minlen_extent error tag
> > +Populate $dstdir by mv-ing new directory entries
> > +Verify $dstdir's extent count
> > +* Rename: Populate source directory and mv one entry to destination directory
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Populate $srcdir with atleast 4 extents
> > +Inject reduce_max_iextents error tag
> > +Move an entry from $srcdir to trigger -EFBIG
> > +* Create multiple hard links to a single file
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Inject reduce_max_iextents error tag
> > +Inject bmap_alloc_minlen_extent error tag
> > +Create multiple hardlinks
> > +Verify directory's extent count
> > +* Create multiple symbolic links to a single file
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Inject reduce_max_iextents error tag
> > +Inject bmap_alloc_minlen_extent error tag
> > +Create multiple symbolic links
> > +Verify directory's extent count
> > +* Populate a directory and remove one entry
> > +Format and mount fs
> > +Consume free space
> > +Create fragmented filesystem
> > +Populate directory with atleast 4 extents
> > +Inject reduce_max_iextents error tag
> > +Remove an entry from directory to trigger -EFBIG
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index bd38aff0..d089797b 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -523,3 +523,4 @@
> > 523 auto quick realtime growfs
> > 524 auto quick punch zero insert collapse
> > 525 auto quick attr
> > +526 auto quick dir hardlink symlink
>
--
chandan
