FSTests Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection
@ 2021-03-09  5:01 Chandan Babu R
  2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
                   ` (12 more replies)
  0 siblings, 13 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

XFS in Linux-v5.12-rc1 gained support for preventing inode extent
count overflow when performing various filesystem operations. Also,
new error injection tags were added for,
1. Reducing maximum extent count to 10.
2. Allocating only single block sized extents.

The corresponding code for xfsprogs can be obtained from
https://lore.kernel.org/linux-xfs/20201104114900.172147-1-chandanrlinux@gmail.com/.

The patches posted along with this cover letter add tests to verify
the correctness of in-kernel inode extent count overflow detection
mechanism.

These patches can also be obtained from
https://github.com/chandanr/xfstests.git at branch
extent-overflow-tests.

Changelog:
V5 -> V6:
  1. Rename xfs_get_fsxattr() helper to _xfs_get_fsxattr().
  2. Add a comment in _check_xfs_filesystem() describing as to why we
     need to invoke syncfs before executing scrub.
  3. Streamline code inside _scratch_get_iext_count() and _xfs_get_fsxattr().


V4 -> V5:
  1. Introduce and use new helper to extract an inode's fsxattr fields.
  2. Issue syncfs() call before executing xfs_scrub.
  3. Invoke syncfs before executing xfs_scrub.
  4. Use _scratch_xfs_get_metadata_field() helper to obtain on-disk data
     structure values.
  5. Declare local variables in _scratch_get_iext_count().
  6. Use _reflink and _reflink_range helpers instead of using xfs_io directly.
  7. Set realtime extent size to 4k with FS block size is less than 4k.
  8. Mark test as "not run" when synthetically constructed realtime fs fails to
     mount.
  9. Update copyright year.

V3 -> V4:
  1. Rebase on fstests' latest master branch.

V2 -> V3:
  1. Kernel's max pseudo extent count is once again set to 10. Hence
     change the tests back to using 10 as the maximum extent count.
  2. The following tests associated with directories have been removed
     since directory remove and rename operations are not expected to
     fail (with -EFBIG error code) in low inode extent count
     scenarios:
     - Populate source directory and mv one entry to destination
       directory.
     - Populate a directory and remove one entry.

V1 -> V2:
  1. Obtain extent count for inodes accessible through filesystem path
     names via "xfs_io -c 'stat' ..." command line. This gets rid of
     unmount/mount cycles from most of the tests.
  2. Use _fill_fs() to consume free space of a filesystem.
  3. Use _scratch_inject_error() helper to enable/disable error tags.
  4. Use sizeof(struct xfs_dqblk) to calculate number of quotas inside
     one filesystem block.
  5. Write once to every block of the quota inode instead of
     sequentially filling up each block.
  6. Use _get_file_block_size() for tests involving regular files.
  7. Modify tests to suit the new pseudo max extent count of 35.
  8. Replace xfs_io with $XFS_IO_PROG.
  9. Remove code that extended a realtime file since this takes the
     same path as direct I/O to a regular file.
  10. For xattr, do not execute test script when block size is < 4k.
  11. Add code to test if removing an xattr from a full attribute fork
      succeeds.
  12. Add code to test if removing a file whose attribute fork is full
      succeeds.
  13. Add code to test if removing a entry from a full directory
      succeeds.
  14. Add code to test if removing a full directory succeeds.
  15. Writing to unwritten extents: Integrate buffered and direct I/O
      tests into a for loop.
  16. Writing to a shared extent test: Add test to check funshare
      operation.
  17. Use _scale_fsstress_args() to scale values for "-p" and "-n"
      arguments for fsstress.

Chandan Babu R (13):
  _check_xfs_filesystem: sync fs before running scrub
  common/xfs: Add a helper to get an inode fork's extent count
  common/xfs: Add helper to obtain fsxattr field value
  xfs: Check for extent overflow when trivally adding a new extent
  xfs: Check for extent overflow when growing realtime bitmap/summary
    inodes
  xfs: Check for extent overflow when punching a hole
  xfs: Check for extent overflow when adding/removing xattrs
  xfs: Check for extent overflow when adding/removing dir entries
  xfs: Check for extent overflow when writing to unwritten extent
  xfs: Check for extent overflow when moving extent from cow to data
    fork
  xfs: Check for extent overflow when remapping an extent
  xfs: Check for extent overflow when swapping extents
  xfs: Stress test with bmap_alloc_minlen_extent error tag enabled

 common/xfs        |  40 ++++++++++
 tests/xfs/528     | 171 +++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/528.out |  20 +++++
 tests/xfs/529     | 124 +++++++++++++++++++++++++++++++
 tests/xfs/529.out |  11 +++
 tests/xfs/530     |  83 +++++++++++++++++++++
 tests/xfs/530.out |  19 +++++
 tests/xfs/531     | 139 +++++++++++++++++++++++++++++++++++
 tests/xfs/531.out |  18 +++++
 tests/xfs/532     | 182 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/532.out |  17 +++++
 tests/xfs/533     |  84 +++++++++++++++++++++
 tests/xfs/533.out |  11 +++
 tests/xfs/534     | 104 ++++++++++++++++++++++++++
 tests/xfs/534.out |  12 +++
 tests/xfs/535     |  81 +++++++++++++++++++++
 tests/xfs/535.out |   8 ++
 tests/xfs/536     | 105 ++++++++++++++++++++++++++
 tests/xfs/536.out |  13 ++++
 tests/xfs/537     |  84 +++++++++++++++++++++
 tests/xfs/537.out |   7 ++
 tests/xfs/group   |  10 +++
 22 files changed, 1343 insertions(+)
 create mode 100755 tests/xfs/528
 create mode 100644 tests/xfs/528.out
 create mode 100755 tests/xfs/529
 create mode 100644 tests/xfs/529.out
 create mode 100755 tests/xfs/530
 create mode 100644 tests/xfs/530.out
 create mode 100755 tests/xfs/531
 create mode 100644 tests/xfs/531.out
 create mode 100755 tests/xfs/532
 create mode 100644 tests/xfs/532.out
 create mode 100755 tests/xfs/533
 create mode 100644 tests/xfs/533.out
 create mode 100755 tests/xfs/534
 create mode 100644 tests/xfs/534.out
 create mode 100755 tests/xfs/535
 create mode 100644 tests/xfs/535.out
 create mode 100755 tests/xfs/536
 create mode 100644 tests/xfs/536.out
 create mode 100755 tests/xfs/537
 create mode 100644 tests/xfs/537.out

-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-09  5:03   ` Darrick J. Wong
  2021-03-10  6:12   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

Tests can create a scenario in which a call to syncfs() issued at the end of
the execution of the test script would return an error code. xfs_scrub
internally calls syncfs() before starting the actual online consistency check
operation. Since this call to syncfs() fails, xfs_scrub ends up returning
without performing consistency checks on the test filesystem. This can mask a
possible on-disk data structure corruption.

To fix the above stated problem, this commit invokes syncfs() prior to
executing xfs_scrub.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 common/xfs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/common/xfs b/common/xfs
index 2156749d..41dd8676 100644
--- a/common/xfs
+++ b/common/xfs
@@ -467,6 +467,17 @@ _check_xfs_filesystem()
 	# Run online scrub if we can.
 	mntpt="$(_is_dev_mounted $device)"
 	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
+		# Tests can create a scenario in which a call to syncfs() issued
+		# at the end of the execution of the test script would return an
+		# error code. xfs_scrub internally calls syncfs() before
+		# starting the actual online consistency check operation. Since
+		# such a call to syncfs() fails, xfs_scrub ends up returning
+		# without performing consistency checks on the test
+		# filesystem. This can mask a possible on-disk data structure
+		# corruption. Hence consume such a possible syncfs() failure
+		# before executing a scrub operation.
+		$XFS_IO_PROG -c syncfs $mntpt >> $seqres.full 2>&1
+
 		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $mntpt > $tmp.scrub 2>&1
 		if [ $? -ne 0 ]; then
 			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
  2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-09  5:04   ` Darrick J. Wong
  2021-03-10  6:12   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
                   ` (10 subsequent siblings)
  12 siblings, 2 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This commit adds the helper _scratch_get_iext_count() which returns an
inode fork's extent count.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 common/xfs | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/common/xfs b/common/xfs
index 41dd8676..26ae21b9 100644
--- a/common/xfs
+++ b/common/xfs
@@ -924,6 +924,26 @@ _scratch_get_bmx_prefix() {
 	return 1
 }
 
+_scratch_get_iext_count()
+{
+	local ino=$1
+	local whichfork=$2
+	local field=""
+
+	case $whichfork in
+		"attr")
+			field=core.naextents
+			;;
+		"data")
+			field=core.nextents
+			;;
+		*)
+			return 1
+	esac
+
+	_scratch_xfs_get_metadata_field $field "inode $ino"
+}
+
 #
 # Ensures that we don't pass any mount options incompatible with XFS v4
 #
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
  2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
  2021-03-09  5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-09  5:04   ` Darrick J. Wong
                     ` (2 more replies)
  2021-03-09  5:01 ` [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
                   ` (9 subsequent siblings)
  12 siblings, 3 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This commit adds a helper function to obtain the value of a particular field
of an inode's fsxattr fields.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 common/xfs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/xfs b/common/xfs
index 26ae21b9..130b3232 100644
--- a/common/xfs
+++ b/common/xfs
@@ -194,6 +194,15 @@ _xfs_get_file_block_size()
 	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
 }
 
+_xfs_get_fsxattr()
+{
+	local field="$1"
+	local path="$2"
+
+	local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field")
+	echo ${value##fsxattr.${field} = }
+}
+
 # xfs_check script is planned to be deprecated. But, we want to
 # be able to invoke "xfs_check" behavior in xfstests in order to
 # maintain the current verification levels.
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (2 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 19:54   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

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.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/528     | 171 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/528.out |  20 ++++++
 tests/xfs/group   |   1 +
 3 files changed, 192 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..557e6818
--- /dev/null
+++ b/tests/xfs/528
@@ -0,0 +1,171 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 528
+#
+# 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"
+
+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_get_fsxattr nextents $testfile)
+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_get_fsxattr nextents $testfile)
+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_get_fsxattr nextents $testfile)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm $testfile
+
+# Check if XFS gracefully returns with an error code when we try to increase
+# extent count of user quota inode beyond the pseudo max extent count limit.
+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
+
+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_get_metadata_field 'uquotino' 'sb 0')
+
+nextents=$(_scratch_get_iext_count $uquotino data || \
+		   _fail "Unable to obtain inode fork's extent count")
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+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..3973cc15
--- /dev/null
+++ b/tests/xfs/528.out
@@ -0,0 +1,20 @@
+QA output created by 528
+Format and mount fs
+* Delalloc to written extent conversion
+Inject reduce_max_iextents error tag
+Create fragmented file
+Verify $testfile's extent count
+* Fallocate unwritten extents
+Fallocate fragmented file
+Verify $testfile's extent count
+* Directio write
+Create fragmented file via directio writes
+Verify $testfile's extent count
+* Extend quota inodes
+Disable reduce_max_iextents error tag
+Consume free space
+Create fragmented filesystem
+Inject reduce_max_iextents error tag
+Inject bmap_alloc_minlen_extent error tag
+Extend uquota file
+Verify uquota inode's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index e861cec9..2356c4a9 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -525,3 +525,4 @@
 525 auto quick mkfs
 526 auto quick mkfs
 527 auto quick quota
+528 auto quick quota
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (3 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 19:55   ` Allison Henderson
  2021-03-22 17:56   ` Darrick J. Wong
  2021-03-09  5:01 ` [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole Chandan Babu R
                   ` (7 subsequent siblings)
  12 siblings, 2 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

Verify that XFS does not cause realtime bitmap/summary inode fork's
extent count to overflow when growing the realtime volume associated
with a filesystem.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/529     | 124 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/529.out |  11 ++++
 tests/xfs/group   |   1 +
 3 files changed, 136 insertions(+)
 create mode 100755 tests/xfs/529
 create mode 100644 tests/xfs/529.out

diff --git a/tests/xfs/529 b/tests/xfs/529
new file mode 100755
index 00000000..dd7019f5
--- /dev/null
+++ b/tests/xfs/529
@@ -0,0 +1,124 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 529
+#
+# Verify that XFS does not cause bitmap/summary inode fork's extent count to
+# overflow when growing an the realtime volume of the filesystem.
+#
+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 /
+	_scratch_unmount >> $seqres.full 2>&1
+	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
+	rm -f $tmp.* $TEST_DIR/$seq.rtvol
+}
+
+# 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
+# Note that we don't _require_realtime because we synthesize a rt volume
+# below.
+_require_test
+_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"
+_require_scratch_nocheck
+
+echo "* Test extending rt inodes"
+
+_scratch_mkfs | _filter_mkfs >> $seqres.full 2> $tmp.mkfs
+. $tmp.mkfs
+
+echo "Create fake rt volume"
+nr_bitmap_blks=25
+nr_bits=$((nr_bitmap_blks * dbsize * 8))
+
+# Realtime extent size has to be atleast 4k in size.
+if (( $dbsize < 4096 )); then
+	rtextsz=4096
+else
+	rtextsz=$dbsize
+fi
+
+rtdevsz=$((nr_bits * rtextsz))
+truncate -s $rtdevsz $TEST_DIR/$seq.rtvol
+rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
+
+echo "Format and mount rt volume"
+
+export USE_EXTERNAL=yes
+export SCRATCH_RTDEV=$rtdev
+_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
+	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
+_try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
+
+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 $((dbsize * nr_free_blks)) $fillerdir $dbsize 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
+
+echo "Grow realtime volume"
+$XFS_GROWFS_PROG -r $SCRATCH_MNT >> $seqres.full 2>&1
+if [[ $? == 0 ]]; then
+	echo "Growfs succeeded; should have failed."
+	exit 1
+fi
+
+_scratch_unmount >> $seqres.full
+
+echo "Verify rbmino's and rsumino's extent count"
+for rtino in rbmino rsumino; do
+	ino=$(_scratch_xfs_get_metadata_field $rtino "sb 0")
+	echo "$rtino = $ino" >> $seqres.full
+
+	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"
+		exit 1
+	fi
+done
+
+echo "Check filesystem"
+_check_xfs_filesystem $SCRATCH_DEV none $rtdev
+
+losetup -d $rtdev
+rm -f $TEST_DIR/$seq.rtvol
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/529.out b/tests/xfs/529.out
new file mode 100644
index 00000000..4ee113a4
--- /dev/null
+++ b/tests/xfs/529.out
@@ -0,0 +1,11 @@
+QA output created by 529
+* Test extending rt inodes
+Create fake rt volume
+Format and mount rt volume
+Consume free space
+Create fragmented filesystem
+Inject reduce_max_iextents error tag
+Inject bmap_alloc_minlen_extent error tag
+Grow realtime volume
+Verify rbmino's and rsumino's extent count
+Check filesystem
diff --git a/tests/xfs/group b/tests/xfs/group
index 2356c4a9..5dff7acb 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -526,3 +526,4 @@
 526 auto quick mkfs
 527 auto quick quota
 528 auto quick quota
+529 auto quick realtime growfs
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (4 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 19:55   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when punching out an extent.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/530     | 83 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/530.out | 19 +++++++++++
 tests/xfs/group   |  1 +
 3 files changed, 103 insertions(+)
 create mode 100755 tests/xfs/530
 create mode 100644 tests/xfs/530.out

diff --git a/tests/xfs/530 b/tests/xfs/530
new file mode 100755
index 00000000..f73f199c
--- /dev/null
+++ b/tests/xfs/530
@@ -0,0 +1,83 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 530
+#
+# Verify that XFS does not cause inode fork's extent count to overflow when
+# punching out an 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/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_xfs_io_command "fpunch"
+_require_xfs_io_command "finsert"
+_require_xfs_io_command "fcollapse"
+_require_xfs_io_command "fzero"
+_require_xfs_io_error_injection "reduce_max_iextents"
+
+echo "Format and mount fs"
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_file_block_size $SCRATCH_MNT)
+nr_blks=30
+
+testfile=$SCRATCH_MNT/testfile
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+for op in fpunch finsert fcollapse fzero; do
+	echo "* $op regular file"
+
+	echo "Create \$testfile"
+	$XFS_IO_PROG -f -s \
+		     -c "pwrite -b $((nr_blks * bsize)) 0 $((nr_blks * bsize))" \
+		     $testfile  >> $seqres.full
+
+	echo "$op alternating blocks"
+	for i in $(seq 1 2 $((nr_blks - 1))); do
+		$XFS_IO_PROG -f -c "$op $((i * bsize)) $bsize" $testfile \
+		       >> $seqres.full 2>&1
+		[[ $? != 0 ]] && break
+	done
+
+	echo "Verify \$testfile's extent count"
+
+	nextents=$(_xfs_get_fsxattr nextents $testfile)
+	if (( $nextents > 10 )); then
+		echo "Extent count overflow check failed: nextents = $nextents"
+		exit 1
+	fi
+
+	rm $testfile
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/530.out b/tests/xfs/530.out
new file mode 100644
index 00000000..4df2d9d0
--- /dev/null
+++ b/tests/xfs/530.out
@@ -0,0 +1,19 @@
+QA output created by 530
+Format and mount fs
+Inject reduce_max_iextents error tag
+* fpunch regular file
+Create $testfile
+fpunch alternating blocks
+Verify $testfile's extent count
+* finsert regular file
+Create $testfile
+finsert alternating blocks
+Verify $testfile's extent count
+* fcollapse regular file
+Create $testfile
+fcollapse alternating blocks
+Verify $testfile's extent count
+* fzero regular file
+Create $testfile
+fzero alternating blocks
+Verify $testfile's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index 5dff7acb..463d810d 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -527,3 +527,4 @@
 527 auto quick quota
 528 auto quick quota
 529 auto quick realtime growfs
+530 auto quick punch zero insert collapse
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (5 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 19:55   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when adding/removing xattrs.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/531     | 139 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/531.out |  18 ++++++
 tests/xfs/group   |   1 +
 3 files changed, 158 insertions(+)
 create mode 100755 tests/xfs/531
 create mode 100644 tests/xfs/531.out

diff --git a/tests/xfs/531 b/tests/xfs/531
new file mode 100755
index 00000000..432c02cb
--- /dev/null
+++ b/tests/xfs/531
@@ -0,0 +1,139 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 531
+#
+# Verify that XFS does not cause inode fork's extent count to overflow when
+# adding/removing xattrs.
+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/attr
+. ./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_attrs
+_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"
+
+echo "Format and mount fs"
+_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_block_size $SCRATCH_MNT)
+
+attr_len=255
+
+testfile=$SCRATCH_MNT/testfile
+
+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
+
+echo "* Set xattrs"
+
+echo "Create \$testfile"
+touch $testfile
+
+echo "Create xattrs"
+nr_attrs=$((bsize * 20 / attr_len))
+for i in $(seq 1 $nr_attrs); do
+	attr="$(printf "trusted.%0247d" $i)"
+	$SETFATTR_PROG -n "$attr" $testfile >> $seqres.full 2>&1
+	[[ $? != 0 ]] && break
+done
+
+echo "Verify \$testfile's naextent count"
+
+naextents=$(_xfs_get_fsxattr naextents $testfile)
+if (( $naextents > 10 )); then
+	echo "Extent count overflow check failed: naextents = $naextents"
+	exit 1
+fi
+
+echo "Remove \$testfile"
+rm $testfile
+
+echo "* Remove xattrs"
+
+echo "Create \$testfile"
+touch $testfile
+
+echo "Disable reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 0
+
+echo "Create initial xattr extents"
+
+naextents=0
+last=""
+start=1
+nr_attrs=$((bsize / attr_len))
+
+while (( $naextents < 4 )); do
+	end=$((start + nr_attrs - 1))
+
+	for i in $(seq $start $end); do
+		attr="$(printf "trusted.%0247d" $i)"
+		$SETFATTR_PROG -n $attr $testfile
+	done
+
+	start=$((end + 1))
+
+	naextents=$(_xfs_get_fsxattr naextents $testfile)
+done
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+echo "Remove xattr to trigger -EFBIG"
+attr="$(printf "trusted.%0247d" 1)"
+$SETFATTR_PROG -x "$attr" $testfile >> $seqres.full 2>&1
+if [[ $? == 0 ]]; then
+	echo "Xattr removal succeeded; Should have failed "
+	exit 1
+fi
+
+rm $testfile && echo "Successfully removed \$testfile"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/531.out b/tests/xfs/531.out
new file mode 100644
index 00000000..7b699b7a
--- /dev/null
+++ b/tests/xfs/531.out
@@ -0,0 +1,18 @@
+QA output created by 531
+Format and mount fs
+Consume free space
+Create fragmented filesystem
+Inject reduce_max_iextents error tag
+Inject bmap_alloc_minlen_extent error tag
+* Set xattrs
+Create $testfile
+Create xattrs
+Verify $testfile's naextent count
+Remove $testfile
+* Remove xattrs
+Create $testfile
+Disable reduce_max_iextents error tag
+Create initial xattr extents
+Inject reduce_max_iextents error tag
+Remove xattr to trigger -EFBIG
+Successfully removed $testfile
diff --git a/tests/xfs/group b/tests/xfs/group
index 463d810d..7e284841 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -528,3 +528,4 @@
 528 auto quick quota
 529 auto quick realtime growfs
 530 auto quick punch zero insert collapse
+531 auto quick attr
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (6 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 22:41   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when adding/removing directory entries.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/532     | 182 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/532.out |  17 +++++
 tests/xfs/group   |   1 +
 3 files changed, 200 insertions(+)
 create mode 100755 tests/xfs/532
 create mode 100644 tests/xfs/532.out

diff --git a/tests/xfs/532 b/tests/xfs/532
new file mode 100755
index 00000000..b241ddeb
--- /dev/null
+++ b/tests/xfs/532
@@ -0,0 +1,182 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 532
+#
+# 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 (( $dirbsize > $dbsize )); then
+	_notrun "Directory block size ($dirbsize) is larger than FSB size ($dbsize)"
+fi
+
+echo "Format and mount fs"
+_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
+_scratch_mount >> $seqres.full
+
+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 $((dbsize * nr_free_blks)) $fillerdir $dbsize 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
+
+dent_len=255
+
+echo "* Create directory entries"
+
+testdir=$SCRATCH_MNT/testdir
+mkdir $testdir
+
+nr_dents=$((dbsize * 20 / dent_len))
+for i in $(seq 1 $nr_dents); do
+	dentry="$(printf "%0255d" $i)"
+	touch ${testdir}/$dentry >> $seqres.full 2>&1 || break
+done
+
+echo "Verify directory's extent count"
+nextents=$(_xfs_get_fsxattr nextents $testdir)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm -rf $testdir
+
+echo "* Rename: Populate destination directory"
+
+dstdir=$SCRATCH_MNT/dstdir
+mkdir $dstdir
+
+nr_dents=$((dirbsize * 20 / dent_len))
+
+echo "Populate \$dstdir by moving new directory entries"
+for i in $(seq 1 $nr_dents); do
+	dentry="$(printf "%0255d" $i)"
+	dentry=${SCRATCH_MNT}/${dentry}
+	touch $dentry || break
+	mv $dentry $dstdir >> $seqres.full 2>&1 || break
+done
+
+rm $dentry
+
+echo "Verify \$dstdir's extent count"
+
+nextents=$(_xfs_get_fsxattr nextents $dstdir)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm -rf $dstdir
+
+echo "* Create multiple hard links to a single file"
+
+testdir=$SCRATCH_MNT/testdir
+mkdir $testdir
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+nr_dents=$((dirbsize * 20 / dent_len))
+
+echo "Create multiple hardlinks"
+for i in $(seq 1 $nr_dents); do
+	dentry="$(printf "%0255d" $i)"
+	ln $testfile ${testdir}/${dentry} >> $seqres.full 2>&1 || break
+done
+
+rm $testfile
+
+echo "Verify directory's extent count"
+nextents=$(_xfs_get_fsxattr nextents $testdir)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm -rf $testdir
+
+echo "* Create multiple symbolic links to a single file"
+
+testdir=$SCRATCH_MNT/testdir
+mkdir $testdir
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+nr_dents=$((dirbsize * 20 / dent_len))
+
+echo "Create multiple symbolic links"
+for i in $(seq 1 $nr_dents); do
+	dentry="$(printf "%0255d" $i)"
+	ln -s $testfile ${testdir}/${dentry} >> $seqres.full 2>&1 || break;
+done
+
+rm $testfile
+
+echo "Verify directory's extent count"
+nextents=$(_xfs_get_fsxattr nextents $testdir)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm -rf $testdir
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/532.out b/tests/xfs/532.out
new file mode 100644
index 00000000..2766c211
--- /dev/null
+++ b/tests/xfs/532.out
@@ -0,0 +1,17 @@
+QA output created by 532
+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
+Populate $dstdir by moving new directory entries
+Verify $dstdir's extent count
+* Create multiple hard links to a single file
+Create multiple hardlinks
+Verify directory's extent count
+* Create multiple symbolic links to a single file
+Create multiple symbolic links
+Verify directory's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index 7e284841..77abeefa 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -529,3 +529,4 @@
 529 auto quick realtime growfs
 530 auto quick punch zero insert collapse
 531 auto quick attr
+532 auto quick dir hardlink symlink
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (7 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 22:41   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when writing to an unwritten extent.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/533     | 84 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/533.out | 11 +++++++
 tests/xfs/group   |  1 +
 3 files changed, 96 insertions(+)
 create mode 100755 tests/xfs/533
 create mode 100644 tests/xfs/533.out

diff --git a/tests/xfs/533 b/tests/xfs/533
new file mode 100755
index 00000000..bb6f075e
--- /dev/null
+++ b/tests/xfs/533
@@ -0,0 +1,84 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 533
+#
+# Verify that XFS does not cause inode fork's extent count to overflow when
+# writing to an unwritten 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/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_xfs_io_command "falloc"
+_require_xfs_io_error_injection "reduce_max_iextents"
+
+echo "Format and mount fs"
+_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_file_block_size $SCRATCH_MNT)
+
+testfile=${SCRATCH_MNT}/testfile
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+nr_blks=15
+
+for io in Buffered Direct; do
+	echo "* $io write to unwritten extent"
+
+	echo "Fallocate $nr_blks blocks"
+	$XFS_IO_PROG -f -c "falloc 0 $((nr_blks * bsize))" $testfile >> $seqres.full
+
+	if [[ $io == "Buffered" ]]; then
+		xfs_io_flag=""
+	else
+		xfs_io_flag="-d"
+	fi
+
+	echo "$io write to every other block of fallocated space"
+	for i in $(seq 1 2 $((nr_blks - 1))); do
+		$XFS_IO_PROG -f -s $xfs_io_flag -c "pwrite $((i * bsize)) $bsize" \
+		       $testfile >> $seqres.full 2>&1
+		[[ $? != 0 ]] && break
+	done
+
+	echo "Verify \$testfile's extent count"
+	nextents=$(_xfs_get_fsxattr nextents $testfile)
+	if (( $nextents > 10 )); then
+		echo "Extent count overflow check failed: nextents = $nextents"
+		exit 1
+	fi
+
+	rm $testfile
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/533.out b/tests/xfs/533.out
new file mode 100644
index 00000000..5b93964a
--- /dev/null
+++ b/tests/xfs/533.out
@@ -0,0 +1,11 @@
+QA output created by 533
+Format and mount fs
+Inject reduce_max_iextents error tag
+* Buffered write to unwritten extent
+Fallocate 15 blocks
+Buffered write to every other block of fallocated space
+Verify $testfile's extent count
+* Direct write to unwritten extent
+Fallocate 15 blocks
+Direct write to every other block of fallocated space
+Verify $testfile's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index 77abeefa..3ad47d07 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -530,3 +530,4 @@
 530 auto quick punch zero insert collapse
 531 auto quick attr
 532 auto quick dir hardlink symlink
+533 auto quick
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (8 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 22:41   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent Chandan Babu R
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when writing to/funshare-ing a shared extent.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/534     | 104 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/534.out |  12 ++++++
 tests/xfs/group   |   1 +
 3 files changed, 117 insertions(+)
 create mode 100755 tests/xfs/534
 create mode 100644 tests/xfs/534.out

diff --git a/tests/xfs/534 b/tests/xfs/534
new file mode 100755
index 00000000..06b21f40
--- /dev/null
+++ b/tests/xfs/534
@@ -0,0 +1,104 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 534
+#
+# 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_command "funshare"
+_require_xfs_io_error_injection "reduce_max_iextents"
+
+echo "Format and mount fs"
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_block_size $SCRATCH_MNT)
+
+nr_blks=15
+
+srcfile=${SCRATCH_MNT}/srcfile
+dstfile=${SCRATCH_MNT}/dstfile
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+echo "Create a \$srcfile having an extent of length $nr_blks blocks"
+$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
+       -c fsync $srcfile  >> $seqres.full
+
+echo "* Write to shared extent"
+
+echo "Share the extent with \$dstfile"
+_reflink $srcfile $dstfile >> $seqres.full
+
+echo "Buffered write to every other block of \$dstfile's shared extent"
+for i in $(seq 1 2 $((nr_blks - 1))); do
+	$XFS_IO_PROG -f -s -c "pwrite $((i * bsize)) $bsize" $dstfile \
+	       >> $seqres.full 2>&1
+	[[ $? != 0 ]] && break
+done
+
+echo "Verify \$dstfile's extent count"
+nextents=$(_xfs_get_fsxattr nextents $dstfile)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+rm $dstfile
+
+echo "* Funshare shared extent"
+
+echo "Share the extent with \$dstfile"
+_reflink $srcfile $dstfile >> $seqres.full
+
+echo "Funshare every other block of \$dstfile's shared extent"
+for i in $(seq 1 2 $((nr_blks - 1))); do
+	$XFS_IO_PROG -f -s -c "funshare $((i * bsize)) $bsize" $dstfile \
+	       >> $seqres.full 2>&1
+	[[ $? != 0 ]] && break
+done
+
+echo "Verify \$dstfile's extent count"
+nextents=$(_xfs_get_fsxattr nextents $dstfile)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+# success, all done
+status=0
+exit
+ 
diff --git a/tests/xfs/534.out b/tests/xfs/534.out
new file mode 100644
index 00000000..53288d12
--- /dev/null
+++ b/tests/xfs/534.out
@@ -0,0 +1,12 @@
+QA output created by 534
+Format and mount fs
+Inject reduce_max_iextents error tag
+Create a $srcfile having an extent of length 15 blocks
+* Write to shared extent
+Share the extent with $dstfile
+Buffered write to every other block of $dstfile's shared extent
+Verify $dstfile's extent count
+* Funshare shared extent
+Share the extent with $dstfile
+Funshare every other block of $dstfile's shared extent
+Verify $dstfile's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index 3ad47d07..b4f0c777 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -531,3 +531,4 @@
 531 auto quick attr
 532 auto quick dir hardlink symlink
 533 auto quick
+534 auto quick reflink
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (9 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 23:49   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents Chandan Babu R
  2021-03-09  5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when remapping extents from one file's inode fork to another.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/535     | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/535.out |  8 +++++
 tests/xfs/group   |  1 +
 3 files changed, 90 insertions(+)
 create mode 100755 tests/xfs/535
 create mode 100644 tests/xfs/535.out

diff --git a/tests/xfs/535 b/tests/xfs/535
new file mode 100755
index 00000000..98209e06
--- /dev/null
+++ b/tests/xfs/535
@@ -0,0 +1,81 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 535
+#
+# Verify that XFS does not cause inode fork's extent count to overflow when
+# remapping extents from one file's inode fork to another.
+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 "* Reflink remap extents"
+
+echo "Format and mount fs"
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_block_size $SCRATCH_MNT)
+
+srcfile=${SCRATCH_MNT}/srcfile
+dstfile=${SCRATCH_MNT}/dstfile
+
+nr_blks=15
+
+echo "Create \$srcfile having an extent of length $nr_blks blocks"
+$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
+       -c fsync $srcfile >> $seqres.full
+
+echo "Create \$dstfile having an extent of length $nr_blks blocks"
+$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
+       -c fsync $dstfile >> $seqres.full
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+echo "Reflink every other block from \$srcfile into \$dstfile"
+for i in $(seq 1 2 $((nr_blks - 1))); do
+	_reflink_range $srcfile $((i * bsize)) $dstfile $((i * bsize)) $bsize \
+		       >> $seqres.full 2>&1
+done
+
+echo "Verify \$dstfile's extent count"
+nextents=$(_xfs_get_fsxattr nextents $dstfile)
+if (( $nextents > 10 )); then
+	echo "Extent count overflow check failed: nextents = $nextents"
+	exit 1
+fi
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/535.out b/tests/xfs/535.out
new file mode 100644
index 00000000..cfe50f45
--- /dev/null
+++ b/tests/xfs/535.out
@@ -0,0 +1,8 @@
+QA output created by 535
+* Reflink remap extents
+Format and mount fs
+Create $srcfile having an extent of length 15 blocks
+Create $dstfile having an extent of length 15 blocks
+Inject reduce_max_iextents error tag
+Reflink every other block from $srcfile into $dstfile
+Verify $dstfile's extent count
diff --git a/tests/xfs/group b/tests/xfs/group
index b4f0c777..aed06494 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -532,3 +532,4 @@
 532 auto quick dir hardlink symlink
 533 auto quick
 534 auto quick reflink
+535 auto quick reflink
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (10 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 23:49   ` Allison Henderson
  2021-03-09  5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
  12 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This test verifies that XFS does not cause inode fork's extent count to
overflow when swapping forks across two files.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/536     | 105 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/536.out |  13 ++++++
 tests/xfs/group   |   1 +
 3 files changed, 119 insertions(+)
 create mode 100755 tests/xfs/536
 create mode 100644 tests/xfs/536.out

diff --git a/tests/xfs/536 b/tests/xfs/536
new file mode 100755
index 00000000..9bb4094a
--- /dev/null
+++ b/tests/xfs/536
@@ -0,0 +1,105 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 536
+#
+# Verify that XFS does not cause inode fork's extent count to overflow when
+# swapping forks between files
+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_xfs_scratch_rmapbt
+_require_xfs_io_command "fcollapse"
+_require_xfs_io_command "swapext"
+_require_xfs_io_error_injection "reduce_max_iextents"
+
+echo "* Swap extent forks"
+
+echo "Format and mount fs"
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_block_size $SCRATCH_MNT)
+
+srcfile=${SCRATCH_MNT}/srcfile
+donorfile=${SCRATCH_MNT}/donorfile
+
+echo "Create \$donorfile having an extent of length 67 blocks"
+$XFS_IO_PROG -f -s -c "pwrite -b $((17 * bsize)) 0 $((17 * bsize))" $donorfile \
+       >> $seqres.full
+
+# After the for loop the donor file will have the following extent layout
+# | 0-4 | 5 | 6 | 7 | 8 | 9 | 10 |
+echo "Fragment \$donorfile"
+for i in $(seq 5 10); do
+	start_offset=$((i * bsize))
+	$XFS_IO_PROG -f -c "fcollapse $start_offset $bsize" $donorfile >> $seqres.full
+done
+
+echo "Create \$srcfile having an extent of length 18 blocks"
+$XFS_IO_PROG -f -s -c "pwrite -b $((18 * bsize)) 0 $((18 * bsize))" $srcfile \
+       >> $seqres.full
+
+echo "Fragment \$srcfile"
+# After the for loop the src file will have the following extent layout
+# | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7-10 |
+for i in $(seq 1 7); do
+	start_offset=$((i * bsize))
+	$XFS_IO_PROG -f -c "fcollapse $start_offset $bsize" $srcfile >> $seqres.full
+done
+
+echo "Collect \$donorfile's extent count"
+donor_nr_exts=$(_xfs_get_fsxattr nextents $donorfile)
+
+echo "Collect \$srcfile's extent count"
+src_nr_exts=$(_xfs_get_fsxattr nextents $srcfile)
+
+echo "Inject reduce_max_iextents error tag"
+_scratch_inject_error reduce_max_iextents 1
+
+echo "Swap \$srcfile's and \$donorfile's extent forks"
+$XFS_IO_PROG -f -c "swapext $donorfile" $srcfile >> $seqres.full 2>&1
+
+echo "Check for \$donorfile's extent count overflow"
+nextents=$(_xfs_get_fsxattr nextents $donorfile)
+
+if (( $nextents == $src_nr_exts )); then
+	echo "\$donorfile: Extent count overflow check failed"
+fi
+
+echo "Check for \$srcfile's extent count overflow"
+nextents=$(_xfs_get_fsxattr nextents $srcfile)
+
+if (( $nextents == $donor_nr_exts )); then
+	echo "\$srcfile: Extent count overflow check failed"
+fi
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/536.out b/tests/xfs/536.out
new file mode 100644
index 00000000..f550aa1e
--- /dev/null
+++ b/tests/xfs/536.out
@@ -0,0 +1,13 @@
+QA output created by 536
+* Swap extent forks
+Format and mount fs
+Create $donorfile having an extent of length 67 blocks
+Fragment $donorfile
+Create $srcfile having an extent of length 18 blocks
+Fragment $srcfile
+Collect $donorfile's extent count
+Collect $srcfile's extent count
+Inject reduce_max_iextents error tag
+Swap $srcfile's and $donorfile's extent forks
+Check for $donorfile's extent count overflow
+Check for $srcfile's extent count overflow
diff --git a/tests/xfs/group b/tests/xfs/group
index aed06494..ba674a58 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -533,3 +533,4 @@
 533 auto quick
 534 auto quick reflink
 535 auto quick reflink
+536 auto quick
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
                   ` (11 preceding siblings ...)
  2021-03-09  5:01 ` [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents Chandan Babu R
@ 2021-03-09  5:01 ` Chandan Babu R
  2021-03-10 23:49   ` Allison Henderson
  2021-03-22 18:54   ` Darrick J. Wong
  12 siblings, 2 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-09  5:01 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong

This commit adds a stress test that executes fsstress with
bmap_alloc_minlen_extent error tag enabled.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/537     | 84 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/537.out |  7 ++++
 tests/xfs/group   |  1 +
 3 files changed, 92 insertions(+)
 create mode 100755 tests/xfs/537
 create mode 100644 tests/xfs/537.out

diff --git a/tests/xfs/537 b/tests/xfs/537
new file mode 100755
index 00000000..77fa60d9
--- /dev/null
+++ b/tests/xfs/537
@@ -0,0 +1,84 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
+#
+# FS QA Test 537
+#
+# Execute fsstress with bmap_alloc_minlen_extent error tag enabled.
+#
+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 "bmap_alloc_minlen_extent"
+
+echo "Format and mount fs"
+_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
+_scratch_mount >> $seqres.full
+
+bsize=$(_get_file_block_size $SCRATCH_MNT)
+
+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 bmap_alloc_minlen_extent error tag"
+_scratch_inject_error bmap_alloc_minlen_extent 1
+
+echo "Scale fsstress args"
+args=$(_scale_fsstress_args -p $((LOAD_FACTOR * 75)) -n $((TIME_FACTOR * 1000)))
+
+echo "Execute fsstress in background"
+$FSSTRESS_PROG -d $SCRATCH_MNT $args \
+		 -f bulkstat=0 \
+		 -f bulkstat1=0 \
+		 -f fiemap=0 \
+		 -f getattr=0 \
+		 -f getdents=0 \
+		 -f getfattr=0 \
+		 -f listfattr=0 \
+		 -f mread=0 \
+		 -f read=0 \
+		 -f readlink=0 \
+		 -f readv=0 \
+		 -f stat=0 \
+		 -f aread=0 \
+		 -f dread=0 > /dev/null 2>&1
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/537.out b/tests/xfs/537.out
new file mode 100644
index 00000000..19633a07
--- /dev/null
+++ b/tests/xfs/537.out
@@ -0,0 +1,7 @@
+QA output created by 537
+Format and mount fs
+Consume free space
+Create fragmented filesystem
+Inject bmap_alloc_minlen_extent error tag
+Scale fsstress args
+Execute fsstress in background
diff --git a/tests/xfs/group b/tests/xfs/group
index ba674a58..5c827727 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -534,3 +534,4 @@
 534 auto quick reflink
 535 auto quick reflink
 536 auto quick
+537 auto stress
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub
  2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
@ 2021-03-09  5:03   ` Darrick J. Wong
  2021-03-10  6:12   ` Allison Henderson
  1 sibling, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-09  5:03 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 09, 2021 at 10:31:12AM +0530, Chandan Babu R wrote:
> Tests can create a scenario in which a call to syncfs() issued at the end of
> the execution of the test script would return an error code. xfs_scrub
> internally calls syncfs() before starting the actual online consistency check
> operation. Since this call to syncfs() fails, xfs_scrub ends up returning
> without performing consistency checks on the test filesystem. This can mask a
> possible on-disk data structure corruption.
> 
> To fix the above stated problem, this commit invokes syncfs() prior to
> executing xfs_scrub.
> 
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>

Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/xfs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 2156749d..41dd8676 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -467,6 +467,17 @@ _check_xfs_filesystem()
>  	# Run online scrub if we can.
>  	mntpt="$(_is_dev_mounted $device)"
>  	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
> +		# Tests can create a scenario in which a call to syncfs() issued
> +		# at the end of the execution of the test script would return an
> +		# error code. xfs_scrub internally calls syncfs() before
> +		# starting the actual online consistency check operation. Since
> +		# such a call to syncfs() fails, xfs_scrub ends up returning
> +		# without performing consistency checks on the test
> +		# filesystem. This can mask a possible on-disk data structure
> +		# corruption. Hence consume such a possible syncfs() failure
> +		# before executing a scrub operation.
> +		$XFS_IO_PROG -c syncfs $mntpt >> $seqres.full 2>&1
> +
>  		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $mntpt > $tmp.scrub 2>&1
>  		if [ $? -ne 0 ]; then
>  			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count
  2021-03-09  5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
@ 2021-03-09  5:04   ` Darrick J. Wong
  2021-03-10  6:12   ` Allison Henderson
  1 sibling, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-09  5:04 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 09, 2021 at 10:31:13AM +0530, Chandan Babu R wrote:
> This commit adds the helper _scratch_get_iext_count() which returns an
> inode fork's extent count.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>

Looks good now,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/xfs | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 41dd8676..26ae21b9 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -924,6 +924,26 @@ _scratch_get_bmx_prefix() {
>  	return 1
>  }
>  
> +_scratch_get_iext_count()
> +{
> +	local ino=$1
> +	local whichfork=$2
> +	local field=""
> +
> +	case $whichfork in
> +		"attr")
> +			field=core.naextents
> +			;;
> +		"data")
> +			field=core.nextents
> +			;;
> +		*)
> +			return 1
> +	esac
> +
> +	_scratch_xfs_get_metadata_field $field "inode $ino"
> +}
> +
>  #
>  # Ensures that we don't pass any mount options incompatible with XFS v4
>  #
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value
  2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
@ 2021-03-09  5:04   ` Darrick J. Wong
  2021-03-10  6:13   ` Allison Henderson
  2021-03-11  8:52   ` [PATCH V6.1] " Chandan Babu R
  2 siblings, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-09  5:04 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 09, 2021 at 10:31:14AM +0530, Chandan Babu R wrote:
> This commit adds a helper function to obtain the value of a particular field
> of an inode's fsxattr fields.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>

Seems reasonable,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/xfs | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 26ae21b9..130b3232 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
>  	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
>  }
>  
> +_xfs_get_fsxattr()
> +{
> +	local field="$1"
> +	local path="$2"
> +
> +	local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field")
> +	echo ${value##fsxattr.${field} = }
> +}
> +
>  # xfs_check script is planned to be deprecated. But, we want to
>  # be able to invoke "xfs_check" behavior in xfstests in order to
>  # maintain the current verification levels.
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count
  2021-03-09  5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
  2021-03-09  5:04   ` Darrick J. Wong
@ 2021-03-10  6:12   ` Allison Henderson
  1 sibling, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10  6:12 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This commit adds the helper _scratch_get_iext_count() which returns an
> inode fork's extent count.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/xfs | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 41dd8676..26ae21b9 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -924,6 +924,26 @@ _scratch_get_bmx_prefix() {
>   	return 1
>   }
>   
> +_scratch_get_iext_count()
> +{
> +	local ino=$1
> +	local whichfork=$2
> +	local field=""
> +
> +	case $whichfork in
> +		"attr")
> +			field=core.naextents
> +			;;
> +		"data")
> +			field=core.nextents
> +			;;
> +		*)
> +			return 1
> +	esac
> +
> +	_scratch_xfs_get_metadata_field $field "inode $ino"
> +}
> +
>   #
>   # Ensures that we don't pass any mount options incompatible with XFS v4
>   #
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub
  2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
  2021-03-09  5:03   ` Darrick J. Wong
@ 2021-03-10  6:12   ` Allison Henderson
  1 sibling, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10  6:12 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> Tests can create a scenario in which a call to syncfs() issued at the end of
> the execution of the test script would return an error code. xfs_scrub
> internally calls syncfs() before starting the actual online consistency check
> operation. Since this call to syncfs() fails, xfs_scrub ends up returning
> without performing consistency checks on the test filesystem. This can mask a
> possible on-disk data structure corruption.
> 
> To fix the above stated problem, this commit invokes syncfs() prior to
> executing xfs_scrub.
> 
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/xfs | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 2156749d..41dd8676 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -467,6 +467,17 @@ _check_xfs_filesystem()
>   	# Run online scrub if we can.
>   	mntpt="$(_is_dev_mounted $device)"
>   	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
> +		# Tests can create a scenario in which a call to syncfs() issued
> +		# at the end of the execution of the test script would return an
> +		# error code. xfs_scrub internally calls syncfs() before
> +		# starting the actual online consistency check operation. Since
> +		# such a call to syncfs() fails, xfs_scrub ends up returning
> +		# without performing consistency checks on the test
> +		# filesystem. This can mask a possible on-disk data structure
> +		# corruption. Hence consume such a possible syncfs() failure
> +		# before executing a scrub operation.
> +		$XFS_IO_PROG -c syncfs $mntpt >> $seqres.full 2>&1
> +
>   		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $mntpt > $tmp.scrub 2>&1
>   		if [ $? -ne 0 ]; then
>   			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value
  2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
  2021-03-09  5:04   ` Darrick J. Wong
@ 2021-03-10  6:13   ` Allison Henderson
  2021-03-10 19:54     ` Darrick J. Wong
  2021-03-11  2:54     ` Chandan Babu R
  2021-03-11  8:52   ` [PATCH V6.1] " Chandan Babu R
  2 siblings, 2 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10  6:13 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This commit adds a helper function to obtain the value of a particular field
> of an inode's fsxattr fields.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
>   common/xfs | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 26ae21b9..130b3232 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
>   	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
>   }
>   
> +_xfs_get_fsxattr()
> +{
> +	local field="$1"
> +	local path="$2"
> +
> +	local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field")
> +	echo ${value##fsxattr.${field} = }
> +}
> +
In fiddling with the commands here, I think I may have noticed a bug.  I 
think you want to grep whole words only, or you may mistakenly match sub 
words. Example:

root@garnet:/home/achender/work_area# field="extsize "
root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test 
| grep "$field"
fsxattr.extsize = 0
fsxattr.cowextsize = 0

I think if you add the -w to the grep that fixes it:
root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test 
| grep -w "$field"
fsxattr.extsize = 0

I think that's what you meant to do right?

Allison

>   # xfs_check script is planned to be deprecated. But, we want to
>   # be able to invoke "xfs_check" behavior in xfstests in order to
>   # maintain the current verification levels.
> 


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value
  2021-03-10  6:13   ` Allison Henderson
@ 2021-03-10 19:54     ` Darrick J. Wong
  2021-03-11  2:54     ` Chandan Babu R
  1 sibling, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-10 19:54 UTC (permalink / raw)
  To: Allison Henderson; +Cc: Chandan Babu R, fstests, linux-xfs

On Tue, Mar 09, 2021 at 11:13:17PM -0700, Allison Henderson wrote:
> 
> 
> On 3/8/21 10:01 PM, Chandan Babu R wrote:
> > This commit adds a helper function to obtain the value of a particular field
> > of an inode's fsxattr fields.
> > 
> > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> > ---
> >   common/xfs | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/common/xfs b/common/xfs
> > index 26ae21b9..130b3232 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
> >   	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
> >   }
> > +_xfs_get_fsxattr()
> > +{
> > +	local field="$1"
> > +	local path="$2"
> > +
> > +	local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field")
> > +	echo ${value##fsxattr.${field} = }
> > +}
> > +
> In fiddling with the commands here, I think I may have noticed a bug.  I
> think you want to grep whole words only, or you may mistakenly match sub
> words. Example:
> 
> root@garnet:/home/achender/work_area# field="extsize "
> root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test |
> grep "$field"
> fsxattr.extsize = 0
> fsxattr.cowextsize = 0
> 
> I think if you add the -w to the grep that fixes it:
> root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test |
> grep -w "$field"

Hey, that's a neat trick I didn't know about!

Oooh and it even exists in both busybox /and/ gnu grep! :)

/me madly goes digging through his shell scripts

<and that was the last we heard from djwong>

--D

> fsxattr.extsize = 0
> 
> I think that's what you meant to do right?
> 
> Allison
> 
> >   # xfs_check script is planned to be deprecated. But, we want to
> >   # be able to invoke "xfs_check" behavior in xfstests in order to
> >   # maintain the current verification levels.
> > 
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent
  2021-03-09  5:01 ` [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
@ 2021-03-10 19:54   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 19:54 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, 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.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
ok, looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/528     | 171 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/528.out |  20 ++++++
>   tests/xfs/group   |   1 +
>   3 files changed, 192 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..557e6818
> --- /dev/null
> +++ b/tests/xfs/528
> @@ -0,0 +1,171 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 528
> +#
> +# 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"
> +
> +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_get_fsxattr nextents $testfile)
> +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_get_fsxattr nextents $testfile)
> +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_get_fsxattr nextents $testfile)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm $testfile
> +
> +# Check if XFS gracefully returns with an error code when we try to increase
> +# extent count of user quota inode beyond the pseudo max extent count limit.
> +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
> +
> +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_get_metadata_field 'uquotino' 'sb 0')
> +
> +nextents=$(_scratch_get_iext_count $uquotino data || \
> +		   _fail "Unable to obtain inode fork's extent count")
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +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..3973cc15
> --- /dev/null
> +++ b/tests/xfs/528.out
> @@ -0,0 +1,20 @@
> +QA output created by 528
> +Format and mount fs
> +* Delalloc to written extent conversion
> +Inject reduce_max_iextents error tag
> +Create fragmented file
> +Verify $testfile's extent count
> +* Fallocate unwritten extents
> +Fallocate fragmented file
> +Verify $testfile's extent count
> +* Directio write
> +Create fragmented file via directio writes
> +Verify $testfile's extent count
> +* Extend quota inodes
> +Disable reduce_max_iextents error tag
> +Consume free space
> +Create fragmented filesystem
> +Inject reduce_max_iextents error tag
> +Inject bmap_alloc_minlen_extent error tag
> +Extend uquota file
> +Verify uquota inode's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index e861cec9..2356c4a9 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -525,3 +525,4 @@
>   525 auto quick mkfs
>   526 auto quick mkfs
>   527 auto quick quota
> +528 auto quick quota
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-09  5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
@ 2021-03-10 19:55   ` Allison Henderson
  2021-03-22 17:56   ` Darrick J. Wong
  1 sibling, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 19:55 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> Verify that XFS does not cause realtime bitmap/summary inode fork's
> extent count to overflow when growing the realtime volume associated
> with a filesystem.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/529     | 124 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/529.out |  11 ++++
>   tests/xfs/group   |   1 +
>   3 files changed, 136 insertions(+)
>   create mode 100755 tests/xfs/529
>   create mode 100644 tests/xfs/529.out
> 
> diff --git a/tests/xfs/529 b/tests/xfs/529
> new file mode 100755
> index 00000000..dd7019f5
> --- /dev/null
> +++ b/tests/xfs/529
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 529
> +#
> +# Verify that XFS does not cause bitmap/summary inode fork's extent count to
> +# overflow when growing an the realtime volume of the filesystem.
> +#
> +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 /
> +	_scratch_unmount >> $seqres.full 2>&1
> +	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
> +	rm -f $tmp.* $TEST_DIR/$seq.rtvol
> +}
> +
> +# 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
> +# Note that we don't _require_realtime because we synthesize a rt volume
> +# below.
> +_require_test
> +_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"
> +_require_scratch_nocheck
> +
> +echo "* Test extending rt inodes"
> +
> +_scratch_mkfs | _filter_mkfs >> $seqres.full 2> $tmp.mkfs
> +. $tmp.mkfs
> +
> +echo "Create fake rt volume"
> +nr_bitmap_blks=25
> +nr_bits=$((nr_bitmap_blks * dbsize * 8))
> +
> +# Realtime extent size has to be atleast 4k in size.
> +if (( $dbsize < 4096 )); then
> +	rtextsz=4096
> +else
> +	rtextsz=$dbsize
> +fi
> +
> +rtdevsz=$((nr_bits * rtextsz))
> +truncate -s $rtdevsz $TEST_DIR/$seq.rtvol
> +rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
> +
> +echo "Format and mount rt volume"
> +
> +export USE_EXTERNAL=yes
> +export SCRATCH_RTDEV=$rtdev
> +_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
> +	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
> +_try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
> +
> +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 $((dbsize * nr_free_blks)) $fillerdir $dbsize 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
> +
> +echo "Grow realtime volume"
> +$XFS_GROWFS_PROG -r $SCRATCH_MNT >> $seqres.full 2>&1
> +if [[ $? == 0 ]]; then
> +	echo "Growfs succeeded; should have failed."
> +	exit 1
> +fi
> +
> +_scratch_unmount >> $seqres.full
> +
> +echo "Verify rbmino's and rsumino's extent count"
> +for rtino in rbmino rsumino; do
> +	ino=$(_scratch_xfs_get_metadata_field $rtino "sb 0")
> +	echo "$rtino = $ino" >> $seqres.full
> +
> +	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"
> +		exit 1
> +	fi
> +done
> +
> +echo "Check filesystem"
> +_check_xfs_filesystem $SCRATCH_DEV none $rtdev
> +
> +losetup -d $rtdev
> +rm -f $TEST_DIR/$seq.rtvol
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/529.out b/tests/xfs/529.out
> new file mode 100644
> index 00000000..4ee113a4
> --- /dev/null
> +++ b/tests/xfs/529.out
> @@ -0,0 +1,11 @@
> +QA output created by 529
> +* Test extending rt inodes
> +Create fake rt volume
> +Format and mount rt volume
> +Consume free space
> +Create fragmented filesystem
> +Inject reduce_max_iextents error tag
> +Inject bmap_alloc_minlen_extent error tag
> +Grow realtime volume
> +Verify rbmino's and rsumino's extent count
> +Check filesystem
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 2356c4a9..5dff7acb 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -526,3 +526,4 @@
>   526 auto quick mkfs
>   527 auto quick quota
>   528 auto quick quota
> +529 auto quick realtime growfs
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole
  2021-03-09  5:01 ` [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole Chandan Babu R
@ 2021-03-10 19:55   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 19:55 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when punching out an extent.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, looks good
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/530     | 83 +++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/530.out | 19 +++++++++++
>   tests/xfs/group   |  1 +
>   3 files changed, 103 insertions(+)
>   create mode 100755 tests/xfs/530
>   create mode 100644 tests/xfs/530.out
> 
> diff --git a/tests/xfs/530 b/tests/xfs/530
> new file mode 100755
> index 00000000..f73f199c
> --- /dev/null
> +++ b/tests/xfs/530
> @@ -0,0 +1,83 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 530
> +#
> +# Verify that XFS does not cause inode fork's extent count to overflow when
> +# punching out an 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/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_xfs_io_command "fpunch"
> +_require_xfs_io_command "finsert"
> +_require_xfs_io_command "fcollapse"
> +_require_xfs_io_command "fzero"
> +_require_xfs_io_error_injection "reduce_max_iextents"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_file_block_size $SCRATCH_MNT)
> +nr_blks=30
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +for op in fpunch finsert fcollapse fzero; do
> +	echo "* $op regular file"
> +
> +	echo "Create \$testfile"
> +	$XFS_IO_PROG -f -s \
> +		     -c "pwrite -b $((nr_blks * bsize)) 0 $((nr_blks * bsize))" \
> +		     $testfile  >> $seqres.full
> +
> +	echo "$op alternating blocks"
> +	for i in $(seq 1 2 $((nr_blks - 1))); do
> +		$XFS_IO_PROG -f -c "$op $((i * bsize)) $bsize" $testfile \
> +		       >> $seqres.full 2>&1
> +		[[ $? != 0 ]] && break
> +	done
> +
> +	echo "Verify \$testfile's extent count"
> +
> +	nextents=$(_xfs_get_fsxattr nextents $testfile)
> +	if (( $nextents > 10 )); then
> +		echo "Extent count overflow check failed: nextents = $nextents"
> +		exit 1
> +	fi
> +
> +	rm $testfile
> +done
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/530.out b/tests/xfs/530.out
> new file mode 100644
> index 00000000..4df2d9d0
> --- /dev/null
> +++ b/tests/xfs/530.out
> @@ -0,0 +1,19 @@
> +QA output created by 530
> +Format and mount fs
> +Inject reduce_max_iextents error tag
> +* fpunch regular file
> +Create $testfile
> +fpunch alternating blocks
> +Verify $testfile's extent count
> +* finsert regular file
> +Create $testfile
> +finsert alternating blocks
> +Verify $testfile's extent count
> +* fcollapse regular file
> +Create $testfile
> +fcollapse alternating blocks
> +Verify $testfile's extent count
> +* fzero regular file
> +Create $testfile
> +fzero alternating blocks
> +Verify $testfile's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 5dff7acb..463d810d 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -527,3 +527,4 @@
>   527 auto quick quota
>   528 auto quick quota
>   529 auto quick realtime growfs
> +530 auto quick punch zero insert collapse
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs
  2021-03-09  5:01 ` [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
@ 2021-03-10 19:55   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 19:55 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when adding/removing xattrs.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>   tests/xfs/531     | 139 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/531.out |  18 ++++++
>   tests/xfs/group   |   1 +
>   3 files changed, 158 insertions(+)
>   create mode 100755 tests/xfs/531
>   create mode 100644 tests/xfs/531.out
> 
> diff --git a/tests/xfs/531 b/tests/xfs/531
> new file mode 100755
> index 00000000..432c02cb
> --- /dev/null
> +++ b/tests/xfs/531
> @@ -0,0 +1,139 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 531
> +#
> +# Verify that XFS does not cause inode fork's extent count to overflow when
> +# adding/removing xattrs.
> +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/attr
> +. ./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_attrs
> +_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"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_block_size $SCRATCH_MNT)
> +
> +attr_len=255
> +
> +testfile=$SCRATCH_MNT/testfile
> +
> +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
> +
> +echo "* Set xattrs"
> +
> +echo "Create \$testfile"
> +touch $testfile
> +
> +echo "Create xattrs"
> +nr_attrs=$((bsize * 20 / attr_len))
> +for i in $(seq 1 $nr_attrs); do
> +	attr="$(printf "trusted.%0247d" $i)"
> +	$SETFATTR_PROG -n "$attr" $testfile >> $seqres.full 2>&1
> +	[[ $? != 0 ]] && break
> +done
> +
> +echo "Verify \$testfile's naextent count"
> +
> +naextents=$(_xfs_get_fsxattr naextents $testfile)
> +if (( $naextents > 10 )); then
> +	echo "Extent count overflow check failed: naextents = $naextents"
> +	exit 1
> +fi
> +
> +echo "Remove \$testfile"
> +rm $testfile
> +
> +echo "* Remove xattrs"
> +
> +echo "Create \$testfile"
> +touch $testfile
> +
> +echo "Disable reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 0
> +
> +echo "Create initial xattr extents"
> +
> +naextents=0
> +last=""
> +start=1
> +nr_attrs=$((bsize / attr_len))
> +
> +while (( $naextents < 4 )); do
> +	end=$((start + nr_attrs - 1))
> +
> +	for i in $(seq $start $end); do
> +		attr="$(printf "trusted.%0247d" $i)"
> +		$SETFATTR_PROG -n $attr $testfile
> +	done
> +
> +	start=$((end + 1))
> +
> +	naextents=$(_xfs_get_fsxattr naextents $testfile)
> +done
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +echo "Remove xattr to trigger -EFBIG"
> +attr="$(printf "trusted.%0247d" 1)"
> +$SETFATTR_PROG -x "$attr" $testfile >> $seqres.full 2>&1
> +if [[ $? == 0 ]]; then
> +	echo "Xattr removal succeeded; Should have failed "
> +	exit 1
> +fi
> +
> +rm $testfile && echo "Successfully removed \$testfile"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/531.out b/tests/xfs/531.out
> new file mode 100644
> index 00000000..7b699b7a
> --- /dev/null
> +++ b/tests/xfs/531.out
> @@ -0,0 +1,18 @@
> +QA output created by 531
> +Format and mount fs
> +Consume free space
> +Create fragmented filesystem
> +Inject reduce_max_iextents error tag
> +Inject bmap_alloc_minlen_extent error tag
> +* Set xattrs
> +Create $testfile
> +Create xattrs
> +Verify $testfile's naextent count
> +Remove $testfile
> +* Remove xattrs
> +Create $testfile
> +Disable reduce_max_iextents error tag
> +Create initial xattr extents
> +Inject reduce_max_iextents error tag
> +Remove xattr to trigger -EFBIG
> +Successfully removed $testfile
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 463d810d..7e284841 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -528,3 +528,4 @@
>   528 auto quick quota
>   529 auto quick realtime growfs
>   530 auto quick punch zero insert collapse
> +531 auto quick attr
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries
  2021-03-09  5:01 ` [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
@ 2021-03-10 22:41   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 22:41 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when adding/removing directory entries.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/532     | 182 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/532.out |  17 +++++
>   tests/xfs/group   |   1 +
>   3 files changed, 200 insertions(+)
>   create mode 100755 tests/xfs/532
>   create mode 100644 tests/xfs/532.out
> 
> diff --git a/tests/xfs/532 b/tests/xfs/532
> new file mode 100755
> index 00000000..b241ddeb
> --- /dev/null
> +++ b/tests/xfs/532
> @@ -0,0 +1,182 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 532
> +#
> +# 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 (( $dirbsize > $dbsize )); then
> +	_notrun "Directory block size ($dirbsize) is larger than FSB size ($dbsize)"
> +fi
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +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 $((dbsize * nr_free_blks)) $fillerdir $dbsize 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
> +
> +dent_len=255
> +
> +echo "* Create directory entries"
> +
> +testdir=$SCRATCH_MNT/testdir
> +mkdir $testdir
> +
> +nr_dents=$((dbsize * 20 / dent_len))
> +for i in $(seq 1 $nr_dents); do
> +	dentry="$(printf "%0255d" $i)"
> +	touch ${testdir}/$dentry >> $seqres.full 2>&1 || break
> +done
> +
> +echo "Verify directory's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $testdir)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm -rf $testdir
> +
> +echo "* Rename: Populate destination directory"
> +
> +dstdir=$SCRATCH_MNT/dstdir
> +mkdir $dstdir
> +
> +nr_dents=$((dirbsize * 20 / dent_len))
> +
> +echo "Populate \$dstdir by moving new directory entries"
> +for i in $(seq 1 $nr_dents); do
> +	dentry="$(printf "%0255d" $i)"
> +	dentry=${SCRATCH_MNT}/${dentry}
> +	touch $dentry || break
> +	mv $dentry $dstdir >> $seqres.full 2>&1 || break
> +done
> +
> +rm $dentry
> +
> +echo "Verify \$dstdir's extent count"
> +
> +nextents=$(_xfs_get_fsxattr nextents $dstdir)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm -rf $dstdir
> +
> +echo "* Create multiple hard links to a single file"
> +
> +testdir=$SCRATCH_MNT/testdir
> +mkdir $testdir
> +
> +testfile=$SCRATCH_MNT/testfile
> +touch $testfile
> +
> +nr_dents=$((dirbsize * 20 / dent_len))
> +
> +echo "Create multiple hardlinks"
> +for i in $(seq 1 $nr_dents); do
> +	dentry="$(printf "%0255d" $i)"
> +	ln $testfile ${testdir}/${dentry} >> $seqres.full 2>&1 || break
> +done
> +
> +rm $testfile
> +
> +echo "Verify directory's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $testdir)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm -rf $testdir
> +
> +echo "* Create multiple symbolic links to a single file"
> +
> +testdir=$SCRATCH_MNT/testdir
> +mkdir $testdir
> +
> +testfile=$SCRATCH_MNT/testfile
> +touch $testfile
> +
> +nr_dents=$((dirbsize * 20 / dent_len))
> +
> +echo "Create multiple symbolic links"
> +for i in $(seq 1 $nr_dents); do
> +	dentry="$(printf "%0255d" $i)"
> +	ln -s $testfile ${testdir}/${dentry} >> $seqres.full 2>&1 || break;
> +done
> +
> +rm $testfile
> +
> +echo "Verify directory's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $testdir)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm -rf $testdir
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/532.out b/tests/xfs/532.out
> new file mode 100644
> index 00000000..2766c211
> --- /dev/null
> +++ b/tests/xfs/532.out
> @@ -0,0 +1,17 @@
> +QA output created by 532
> +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
> +Populate $dstdir by moving new directory entries
> +Verify $dstdir's extent count
> +* Create multiple hard links to a single file
> +Create multiple hardlinks
> +Verify directory's extent count
> +* Create multiple symbolic links to a single file
> +Create multiple symbolic links
> +Verify directory's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 7e284841..77abeefa 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -529,3 +529,4 @@
>   529 auto quick realtime growfs
>   530 auto quick punch zero insert collapse
>   531 auto quick attr
> +532 auto quick dir hardlink symlink
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent
  2021-03-09  5:01 ` [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
@ 2021-03-10 22:41   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 22:41 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when writing to an unwritten extent.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/533     | 84 +++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/533.out | 11 +++++++
>   tests/xfs/group   |  1 +
>   3 files changed, 96 insertions(+)
>   create mode 100755 tests/xfs/533
>   create mode 100644 tests/xfs/533.out
> 
> diff --git a/tests/xfs/533 b/tests/xfs/533
> new file mode 100755
> index 00000000..bb6f075e
> --- /dev/null
> +++ b/tests/xfs/533
> @@ -0,0 +1,84 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 533
> +#
> +# Verify that XFS does not cause inode fork's extent count to overflow when
> +# writing to an unwritten 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/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_xfs_io_command "falloc"
> +_require_xfs_io_error_injection "reduce_max_iextents"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_file_block_size $SCRATCH_MNT)
> +
> +testfile=${SCRATCH_MNT}/testfile
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +nr_blks=15
> +
> +for io in Buffered Direct; do
> +	echo "* $io write to unwritten extent"
> +
> +	echo "Fallocate $nr_blks blocks"
> +	$XFS_IO_PROG -f -c "falloc 0 $((nr_blks * bsize))" $testfile >> $seqres.full
> +
> +	if [[ $io == "Buffered" ]]; then
> +		xfs_io_flag=""
> +	else
> +		xfs_io_flag="-d"
> +	fi
> +
> +	echo "$io write to every other block of fallocated space"
> +	for i in $(seq 1 2 $((nr_blks - 1))); do
> +		$XFS_IO_PROG -f -s $xfs_io_flag -c "pwrite $((i * bsize)) $bsize" \
> +		       $testfile >> $seqres.full 2>&1
> +		[[ $? != 0 ]] && break
> +	done
> +
> +	echo "Verify \$testfile's extent count"
> +	nextents=$(_xfs_get_fsxattr nextents $testfile)
> +	if (( $nextents > 10 )); then
> +		echo "Extent count overflow check failed: nextents = $nextents"
> +		exit 1
> +	fi
> +
> +	rm $testfile
> +done
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/533.out b/tests/xfs/533.out
> new file mode 100644
> index 00000000..5b93964a
> --- /dev/null
> +++ b/tests/xfs/533.out
> @@ -0,0 +1,11 @@
> +QA output created by 533
> +Format and mount fs
> +Inject reduce_max_iextents error tag
> +* Buffered write to unwritten extent
> +Fallocate 15 blocks
> +Buffered write to every other block of fallocated space
> +Verify $testfile's extent count
> +* Direct write to unwritten extent
> +Fallocate 15 blocks
> +Direct write to every other block of fallocated space
> +Verify $testfile's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 77abeefa..3ad47d07 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -530,3 +530,4 @@
>   530 auto quick punch zero insert collapse
>   531 auto quick attr
>   532 auto quick dir hardlink symlink
> +533 auto quick
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork
  2021-03-09  5:01 ` [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
@ 2021-03-10 22:41   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 22:41 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when writing to/funshare-ing a shared extent.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>   tests/xfs/534     | 104 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/534.out |  12 ++++++
>   tests/xfs/group   |   1 +
>   3 files changed, 117 insertions(+)
>   create mode 100755 tests/xfs/534
>   create mode 100644 tests/xfs/534.out
> 
> diff --git a/tests/xfs/534 b/tests/xfs/534
> new file mode 100755
> index 00000000..06b21f40
> --- /dev/null
> +++ b/tests/xfs/534
> @@ -0,0 +1,104 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 534
> +#
> +# 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_command "funshare"
> +_require_xfs_io_error_injection "reduce_max_iextents"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_block_size $SCRATCH_MNT)
> +
> +nr_blks=15
> +
> +srcfile=${SCRATCH_MNT}/srcfile
> +dstfile=${SCRATCH_MNT}/dstfile
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +echo "Create a \$srcfile having an extent of length $nr_blks blocks"
> +$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
> +       -c fsync $srcfile  >> $seqres.full
> +
> +echo "* Write to shared extent"
> +
> +echo "Share the extent with \$dstfile"
> +_reflink $srcfile $dstfile >> $seqres.full
> +
> +echo "Buffered write to every other block of \$dstfile's shared extent"
> +for i in $(seq 1 2 $((nr_blks - 1))); do
> +	$XFS_IO_PROG -f -s -c "pwrite $((i * bsize)) $bsize" $dstfile \
> +	       >> $seqres.full 2>&1
> +	[[ $? != 0 ]] && break
> +done
> +
> +echo "Verify \$dstfile's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $dstfile)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +rm $dstfile
> +
> +echo "* Funshare shared extent"
> +
> +echo "Share the extent with \$dstfile"
> +_reflink $srcfile $dstfile >> $seqres.full
> +
> +echo "Funshare every other block of \$dstfile's shared extent"
> +for i in $(seq 1 2 $((nr_blks - 1))); do
> +	$XFS_IO_PROG -f -s -c "funshare $((i * bsize)) $bsize" $dstfile \
> +	       >> $seqres.full 2>&1
> +	[[ $? != 0 ]] && break
> +done
> +
> +echo "Verify \$dstfile's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $dstfile)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +# success, all done
> +status=0
> +exit
> +
> diff --git a/tests/xfs/534.out b/tests/xfs/534.out
> new file mode 100644
> index 00000000..53288d12
> --- /dev/null
> +++ b/tests/xfs/534.out
> @@ -0,0 +1,12 @@
> +QA output created by 534
> +Format and mount fs
> +Inject reduce_max_iextents error tag
> +Create a $srcfile having an extent of length 15 blocks
> +* Write to shared extent
> +Share the extent with $dstfile
> +Buffered write to every other block of $dstfile's shared extent
> +Verify $dstfile's extent count
> +* Funshare shared extent
> +Share the extent with $dstfile
> +Funshare every other block of $dstfile's shared extent
> +Verify $dstfile's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 3ad47d07..b4f0c777 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -531,3 +531,4 @@
>   531 auto quick attr
>   532 auto quick dir hardlink symlink
>   533 auto quick
> +534 auto quick reflink
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent
  2021-03-09  5:01 ` [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent Chandan Babu R
@ 2021-03-10 23:49   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 23:49 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when remapping extents from one file's inode fork to another.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/535     | 81 +++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/535.out |  8 +++++
>   tests/xfs/group   |  1 +
>   3 files changed, 90 insertions(+)
>   create mode 100755 tests/xfs/535
>   create mode 100644 tests/xfs/535.out
> 
> diff --git a/tests/xfs/535 b/tests/xfs/535
> new file mode 100755
> index 00000000..98209e06
> --- /dev/null
> +++ b/tests/xfs/535
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 535
> +#
> +# Verify that XFS does not cause inode fork's extent count to overflow when
> +# remapping extents from one file's inode fork to another.
> +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 "* Reflink remap extents"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_block_size $SCRATCH_MNT)
> +
> +srcfile=${SCRATCH_MNT}/srcfile
> +dstfile=${SCRATCH_MNT}/dstfile
> +
> +nr_blks=15
> +
> +echo "Create \$srcfile having an extent of length $nr_blks blocks"
> +$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
> +       -c fsync $srcfile >> $seqres.full
> +
> +echo "Create \$dstfile having an extent of length $nr_blks blocks"
> +$XFS_IO_PROG -f -c "pwrite -b $((nr_blks * bsize))  0 $((nr_blks * bsize))" \
> +       -c fsync $dstfile >> $seqres.full
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +echo "Reflink every other block from \$srcfile into \$dstfile"
> +for i in $(seq 1 2 $((nr_blks - 1))); do
> +	_reflink_range $srcfile $((i * bsize)) $dstfile $((i * bsize)) $bsize \
> +		       >> $seqres.full 2>&1
> +done
> +
> +echo "Verify \$dstfile's extent count"
> +nextents=$(_xfs_get_fsxattr nextents $dstfile)
> +if (( $nextents > 10 )); then
> +	echo "Extent count overflow check failed: nextents = $nextents"
> +	exit 1
> +fi
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/535.out b/tests/xfs/535.out
> new file mode 100644
> index 00000000..cfe50f45
> --- /dev/null
> +++ b/tests/xfs/535.out
> @@ -0,0 +1,8 @@
> +QA output created by 535
> +* Reflink remap extents
> +Format and mount fs
> +Create $srcfile having an extent of length 15 blocks
> +Create $dstfile having an extent of length 15 blocks
> +Inject reduce_max_iextents error tag
> +Reflink every other block from $srcfile into $dstfile
> +Verify $dstfile's extent count
> diff --git a/tests/xfs/group b/tests/xfs/group
> index b4f0c777..aed06494 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -532,3 +532,4 @@
>   532 auto quick dir hardlink symlink
>   533 auto quick
>   534 auto quick reflink
> +535 auto quick reflink
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents
  2021-03-09  5:01 ` [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents Chandan Babu R
@ 2021-03-10 23:49   ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 23:49 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This test verifies that XFS does not cause inode fork's extent count to
> overflow when swapping forks across two files.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Makes sense.  The extent illustrations help
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/536     | 105 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/536.out |  13 ++++++
>   tests/xfs/group   |   1 +
>   3 files changed, 119 insertions(+)
>   create mode 100755 tests/xfs/536
>   create mode 100644 tests/xfs/536.out
> 
> diff --git a/tests/xfs/536 b/tests/xfs/536
> new file mode 100755
> index 00000000..9bb4094a
> --- /dev/null
> +++ b/tests/xfs/536
> @@ -0,0 +1,105 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 536
> +#
> +# Verify that XFS does not cause inode fork's extent count to overflow when
> +# swapping forks between files
> +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_xfs_scratch_rmapbt
> +_require_xfs_io_command "fcollapse"
> +_require_xfs_io_command "swapext"
> +_require_xfs_io_error_injection "reduce_max_iextents"
> +
> +echo "* Swap extent forks"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_block_size $SCRATCH_MNT)
> +
> +srcfile=${SCRATCH_MNT}/srcfile
> +donorfile=${SCRATCH_MNT}/donorfile
> +
> +echo "Create \$donorfile having an extent of length 67 blocks"
> +$XFS_IO_PROG -f -s -c "pwrite -b $((17 * bsize)) 0 $((17 * bsize))" $donorfile \
> +       >> $seqres.full
> +
> +# After the for loop the donor file will have the following extent layout
> +# | 0-4 | 5 | 6 | 7 | 8 | 9 | 10 |
> +echo "Fragment \$donorfile"
> +for i in $(seq 5 10); do
> +	start_offset=$((i * bsize))
> +	$XFS_IO_PROG -f -c "fcollapse $start_offset $bsize" $donorfile >> $seqres.full
> +done
> +
> +echo "Create \$srcfile having an extent of length 18 blocks"
> +$XFS_IO_PROG -f -s -c "pwrite -b $((18 * bsize)) 0 $((18 * bsize))" $srcfile \
> +       >> $seqres.full
> +
> +echo "Fragment \$srcfile"
> +# After the for loop the src file will have the following extent layout
> +# | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7-10 |
> +for i in $(seq 1 7); do
> +	start_offset=$((i * bsize))
> +	$XFS_IO_PROG -f -c "fcollapse $start_offset $bsize" $srcfile >> $seqres.full
> +done
> +
> +echo "Collect \$donorfile's extent count"
> +donor_nr_exts=$(_xfs_get_fsxattr nextents $donorfile)
> +
> +echo "Collect \$srcfile's extent count"
> +src_nr_exts=$(_xfs_get_fsxattr nextents $srcfile)
> +
> +echo "Inject reduce_max_iextents error tag"
> +_scratch_inject_error reduce_max_iextents 1
> +
> +echo "Swap \$srcfile's and \$donorfile's extent forks"
> +$XFS_IO_PROG -f -c "swapext $donorfile" $srcfile >> $seqres.full 2>&1
> +
> +echo "Check for \$donorfile's extent count overflow"
> +nextents=$(_xfs_get_fsxattr nextents $donorfile)
> +
> +if (( $nextents == $src_nr_exts )); then
> +	echo "\$donorfile: Extent count overflow check failed"
> +fi
> +
> +echo "Check for \$srcfile's extent count overflow"
> +nextents=$(_xfs_get_fsxattr nextents $srcfile)
> +
> +if (( $nextents == $donor_nr_exts )); then
> +	echo "\$srcfile: Extent count overflow check failed"
> +fi
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/536.out b/tests/xfs/536.out
> new file mode 100644
> index 00000000..f550aa1e
> --- /dev/null
> +++ b/tests/xfs/536.out
> @@ -0,0 +1,13 @@
> +QA output created by 536
> +* Swap extent forks
> +Format and mount fs
> +Create $donorfile having an extent of length 67 blocks
> +Fragment $donorfile
> +Create $srcfile having an extent of length 18 blocks
> +Fragment $srcfile
> +Collect $donorfile's extent count
> +Collect $srcfile's extent count
> +Inject reduce_max_iextents error tag
> +Swap $srcfile's and $donorfile's extent forks
> +Check for $donorfile's extent count overflow
> +Check for $srcfile's extent count overflow
> diff --git a/tests/xfs/group b/tests/xfs/group
> index aed06494..ba674a58 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -533,3 +533,4 @@
>   533 auto quick
>   534 auto quick reflink
>   535 auto quick reflink
> +536 auto quick
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-09  5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
@ 2021-03-10 23:49   ` Allison Henderson
  2021-03-22 18:54   ` Darrick J. Wong
  1 sibling, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-10 23:49 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/8/21 10:01 PM, Chandan Babu R wrote:
> This commit adds a stress test that executes fsstress with
> bmap_alloc_minlen_extent error tag enabled.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok makes sense
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   tests/xfs/537     | 84 +++++++++++++++++++++++++++++++++++++++++++++++
>   tests/xfs/537.out |  7 ++++
>   tests/xfs/group   |  1 +
>   3 files changed, 92 insertions(+)
>   create mode 100755 tests/xfs/537
>   create mode 100644 tests/xfs/537.out
> 
> diff --git a/tests/xfs/537 b/tests/xfs/537
> new file mode 100755
> index 00000000..77fa60d9
> --- /dev/null
> +++ b/tests/xfs/537
> @@ -0,0 +1,84 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 537
> +#
> +# Execute fsstress with bmap_alloc_minlen_extent error tag enabled.
> +#
> +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 "bmap_alloc_minlen_extent"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_file_block_size $SCRATCH_MNT)
> +
> +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 bmap_alloc_minlen_extent error tag"
> +_scratch_inject_error bmap_alloc_minlen_extent 1
> +
> +echo "Scale fsstress args"
> +args=$(_scale_fsstress_args -p $((LOAD_FACTOR * 75)) -n $((TIME_FACTOR * 1000)))
> +
> +echo "Execute fsstress in background"
> +$FSSTRESS_PROG -d $SCRATCH_MNT $args \
> +		 -f bulkstat=0 \
> +		 -f bulkstat1=0 \
> +		 -f fiemap=0 \
> +		 -f getattr=0 \
> +		 -f getdents=0 \
> +		 -f getfattr=0 \
> +		 -f listfattr=0 \
> +		 -f mread=0 \
> +		 -f read=0 \
> +		 -f readlink=0 \
> +		 -f readv=0 \
> +		 -f stat=0 \
> +		 -f aread=0 \
> +		 -f dread=0 > /dev/null 2>&1
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/537.out b/tests/xfs/537.out
> new file mode 100644
> index 00000000..19633a07
> --- /dev/null
> +++ b/tests/xfs/537.out
> @@ -0,0 +1,7 @@
> +QA output created by 537
> +Format and mount fs
> +Consume free space
> +Create fragmented filesystem
> +Inject bmap_alloc_minlen_extent error tag
> +Scale fsstress args
> +Execute fsstress in background
> diff --git a/tests/xfs/group b/tests/xfs/group
> index ba674a58..5c827727 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -534,3 +534,4 @@
>   534 auto quick reflink
>   535 auto quick reflink
>   536 auto quick
> +537 auto stress
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value
  2021-03-10  6:13   ` Allison Henderson
  2021-03-10 19:54     ` Darrick J. Wong
@ 2021-03-11  2:54     ` Chandan Babu R
  1 sibling, 0 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-11  2:54 UTC (permalink / raw)
  To: Allison Henderson; +Cc: fstests, linux-xfs, djwong


On 10 Mar 2021 at 11:43, Allison Henderson wrote:
> On 3/8/21 10:01 PM, Chandan Babu R wrote:
>> This commit adds a helper function to obtain the value of a particular field
>> of an inode's fsxattr fields.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> ---
>>   common/xfs | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/common/xfs b/common/xfs
>> index 26ae21b9..130b3232 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
>>   	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
>>   }
>>   +_xfs_get_fsxattr()
>> +{
>> +	local field="$1"
>> +	local path="$2"
>> +
>> +	local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field")
>> +	echo ${value##fsxattr.${field} = }
>> +}
>> +
> In fiddling with the commands here, I think I may have noticed a bug.
> I think you want to grep whole words only, or you may mistakenly match
> sub words. Example:
>
> root@garnet:/home/achender/work_area# field="extsize "
> root@garnet:/home/achender/work_area# xfs_io -c "stat"
> /mnt/scratch/test | grep "$field"
> fsxattr.extsize = 0
> fsxattr.cowextsize = 0
>
> I think if you add the -w to the grep that fixes it:
> root@garnet:/home/achender/work_area# xfs_io -c "stat"
> /mnt/scratch/test | grep -w "$field"
> fsxattr.extsize = 0
>
> I think that's what you meant to do right?
>

Yes, that was the intended behaviour. Thanks for catching the bug and
suggesting the appropriate solution.

I will fix this.

Also, Thanks for reviewing the entire patchset.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH V6.1] common/xfs: Add helper to obtain fsxattr field value
  2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
  2021-03-09  5:04   ` Darrick J. Wong
  2021-03-10  6:13   ` Allison Henderson
@ 2021-03-11  8:52   ` Chandan Babu R
  2021-03-11 18:36     ` Allison Henderson
  2 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-11  8:52 UTC (permalink / raw)
  To: fstests; +Cc: Chandan Babu R, linux-xfs, djwong, allison.henderson

This commit adds a helper function to obtain the value of a particular field
of an inode's fsxattr fields.

Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
V6 -> V6.1
1. Pass '-w' flag to grep to limit searches that match whole words.

 common/xfs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/xfs b/common/xfs
index 26ae21b9..aec2cea6 100644
--- a/common/xfs
+++ b/common/xfs
@@ -194,6 +194,15 @@ _xfs_get_file_block_size()
 	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
 }
 
+_xfs_get_fsxattr()
+{
+	local field="$1"
+	local path="$2"
+
+	local value=$($XFS_IO_PROG -c "stat" "$path" | grep -w "$field")
+	echo ${value##fsxattr.${field} = }
+}
+
 # xfs_check script is planned to be deprecated. But, we want to
 # be able to invoke "xfs_check" behavior in xfstests in order to
 # maintain the current verification levels.
-- 
2.29.2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6.1] common/xfs: Add helper to obtain fsxattr field value
  2021-03-11  8:52   ` [PATCH V6.1] " Chandan Babu R
@ 2021-03-11 18:36     ` Allison Henderson
  0 siblings, 0 replies; 46+ messages in thread
From: Allison Henderson @ 2021-03-11 18:36 UTC (permalink / raw)
  To: Chandan Babu R, fstests; +Cc: linux-xfs, djwong



On 3/11/21 1:52 AM, Chandan Babu R wrote:
> This commit adds a helper function to obtain the value of a particular field
> of an inode's fsxattr fields.
> 
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Ok, looks good!
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
> V6 -> V6.1
> 1. Pass '-w' flag to grep to limit searches that match whole words.
> 
>   common/xfs | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 26ae21b9..aec2cea6 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -194,6 +194,15 @@ _xfs_get_file_block_size()
>   	$XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g'
>   }
>   
> +_xfs_get_fsxattr()
> +{
> +	local field="$1"
> +	local path="$2"
> +
> +	local value=$($XFS_IO_PROG -c "stat" "$path" | grep -w "$field")
> +	echo ${value##fsxattr.${field} = }
> +}
> +
>   # xfs_check script is planned to be deprecated. But, we want to
>   # be able to invoke "xfs_check" behavior in xfstests in order to
>   # maintain the current verification levels.
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-09  5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
  2021-03-10 19:55   ` Allison Henderson
@ 2021-03-22 17:56   ` Darrick J. Wong
  2021-03-23 15:51     ` Chandan Babu R
  1 sibling, 1 reply; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-22 17:56 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 09, 2021 at 10:31:16AM +0530, Chandan Babu R wrote:
> Verify that XFS does not cause realtime bitmap/summary inode fork's
> extent count to overflow when growing the realtime volume associated
> with a filesystem.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>

Soo... I discovered that this test doesn't pass with multiblock
directories:

FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 alder-mtr00 5.12.0-rc4-xfsx #rc4 SMP PREEMPT Mon Mar 22 10:03:45 PDT 2021
MKFS_OPTIONS  -- -f -b size=1024, /dev/sdf
MOUNT_OPTIONS -- -o usrquota,grpquota,prjquota, /dev/sdf /opt

xfs/529 - output mismatch (see /var/tmp/fstests/xfs/529.out.bad)
    --- tests/xfs/529.out       2021-03-21 11:44:09.383407733 -0700
    +++ /var/tmp/fstests/xfs/529.out.bad        2021-03-22 10:36:34.000348426 -0700
    @@ -4,12 +4,21 @@
     Inject reduce_max_iextents error tag
     Create fragmented file
     Verify $testfile's extent count
    +/opt/testfile: No such file or directory
    +/tmp/fstests/tests/xfs/529: line 72: ((: > 10 : syntax error: operand expected (error token is "> 10 ")
    +rm: cannot remove '/opt/testfile': No such file or directory
     * Fallocate unwritten extents
    ...
    (Run 'diff -u /tmp/fstests/tests/xfs/529.out /var/tmp/fstests/xfs/529.out.bad'  to see the entire diff)
Ran: xfs/529
Failures: xfs/529
Failed 1 of 1 tests

Test xfs/529 FAILED with code 1 and bad golden output:
--- /tmp/fstests/tests/xfs/529.out      2021-03-21 11:44:09.383407733 -0700
+++ /var/tmp/fstests/xfs/529.out.bad    2021-03-22 10:36:34.000348426 -0700
@@ -4,12 +4,21 @@
 Inject reduce_max_iextents error tag
 Create fragmented file
 Verify $testfile's extent count
+/opt/testfile: No such file or directory
+/tmp/fstests/tests/xfs/529: line 72: ((: > 10 : syntax error: operand expected (error token is "> 10 ")
+rm: cannot remove '/opt/testfile': No such file or directory
 * Fallocate unwritten extents
 Fallocate fragmented file
 Verify $testfile's extent count
+/opt/testfile: No such file or directory
+/tmp/fstests/tests/xfs/529: line 91: ((: > 10 : syntax error: operand expected (error token is "> 10 ")
+rm: cannot remove '/opt/testfile': No such file or directory
 * Directio write
 Create fragmented file via directio writes
 Verify $testfile's extent count
+/opt/testfile: No such file or directory
+/tmp/fstests/tests/xfs/529: line 110: ((: > 10 : syntax error: operand expected (error token is "> 10 ")
+rm: cannot remove '/opt/testfile': No such file or directory
 * Extend quota inodes
 Disable reduce_max_iextents error tag
 Consume free space

The test appears to fail because we cannot create even a single file in
the root directory.  Looking at xfs_create, I see:

	error = xfs_iext_count_may_overflow(dp, XFS_DATA_FORK,
			XFS_IEXT_DIR_MANIP_CNT(mp));
	if (error)
		goto out_trans_cancel;

XFS_IEXT_DIR_MANIP_CNT is defined as:

	#define XFS_IEXT_DIR_MANIP_CNT(mp) \
		((XFS_DA_NODE_MAXDEPTH + 1 + 1) * (mp)->m_dir_geo->fsbcount)

If one formats a filesystem with 1k blocks, the result will be a
filesystem with 4k directory blocks:

# mkfs.xfs -b size=1024 /dev/sdf -Nf
meta-data=/dev/sdf               isize=512    agcount=4, agsize=5192704 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=1    bigtime=1 inobtcount=1
         =                       metadir=0   
data     =                       bsize=1024   blocks=20770816, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1
log      =internal log           bsize=1024   blocks=10240, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0

Note "data bsize" is 1024, and "naming bsize" is 4096.

In the kernel, we set m_dir_geo->fsbcount = "naming bsize" /
"data bsize", or 4 in this case.  Since XFS_DA_NODE_MAXDEPTH is always
5, this macro expands to:

	(5 + 1 + 1) * (4) = 28

The reason for the test failure I think is because of this code in
xfs_iext_count_may_overflow, which is called from xfs_create on the
parent directory:

	if (XFS_TEST_ERROR(false, ip->i_mount, XFS_ERRTAG_REDUCE_MAX_IEXTENTS))
		max_exts = 10;

	nr_exts = ifp->if_nextents + nr_to_add;
	if (nr_exts < ifp->if_nextents || nr_exts > max_ext)
		return -EFBIG

The second part of the if statement becomes (28 > 10) which is trivially
true, so we return -EFBIG for all attempts to create a file in a
directory.  xfs/529, in turn, cannot create $testfile because nothing
can create a file in $SCRATCH_MNT, and the test goes off the rails.

I think this can be trivially solved by changing this (and the other
tests) to ensure that the error injection is only set when we're running
a command to check if we get EFBIG.  In other words, this code in
xfs/529:

	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

Should become:

	rm -f $testfile
	touch $testfile

	echo "* Fallocate unwritten extents"

	echo "Fallocate fragmented file"
	_scratch_inject_error reduce_max_iextents 1
	for i in $(seq 0 2 $((nr_blks - 1))); do
		$XFS_IO_PROG -c "falloc $((i * bsize)) $bsize" $testfile \
		       >> $seqres.full 2>&1
		[[ $? != 0 ]] && break
	done
	_scratch_inject_error reduce_max_iextents 0

With that patched up, xfs/529 passes on 1k block filesystems.  I suspect
the other tests in this series (xfs/531, 532, 534, and 535) are going to
need similar patching.

--D

> ---
>  tests/xfs/529     | 124 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/529.out |  11 ++++
>  tests/xfs/group   |   1 +
>  3 files changed, 136 insertions(+)
>  create mode 100755 tests/xfs/529
>  create mode 100644 tests/xfs/529.out
> 
> diff --git a/tests/xfs/529 b/tests/xfs/529
> new file mode 100755
> index 00000000..dd7019f5
> --- /dev/null
> +++ b/tests/xfs/529
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 529
> +#
> +# Verify that XFS does not cause bitmap/summary inode fork's extent count to
> +# overflow when growing an the realtime volume of the filesystem.
> +#
> +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 /
> +	_scratch_unmount >> $seqres.full 2>&1
> +	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
> +	rm -f $tmp.* $TEST_DIR/$seq.rtvol
> +}
> +
> +# 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
> +# Note that we don't _require_realtime because we synthesize a rt volume
> +# below.
> +_require_test
> +_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"
> +_require_scratch_nocheck
> +
> +echo "* Test extending rt inodes"
> +
> +_scratch_mkfs | _filter_mkfs >> $seqres.full 2> $tmp.mkfs
> +. $tmp.mkfs
> +
> +echo "Create fake rt volume"
> +nr_bitmap_blks=25
> +nr_bits=$((nr_bitmap_blks * dbsize * 8))
> +
> +# Realtime extent size has to be atleast 4k in size.
> +if (( $dbsize < 4096 )); then
> +	rtextsz=4096
> +else
> +	rtextsz=$dbsize
> +fi
> +
> +rtdevsz=$((nr_bits * rtextsz))
> +truncate -s $rtdevsz $TEST_DIR/$seq.rtvol
> +rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
> +
> +echo "Format and mount rt volume"
> +
> +export USE_EXTERNAL=yes
> +export SCRATCH_RTDEV=$rtdev
> +_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
> +	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
> +_try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
> +
> +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 $((dbsize * nr_free_blks)) $fillerdir $dbsize 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
> +
> +echo "Grow realtime volume"
> +$XFS_GROWFS_PROG -r $SCRATCH_MNT >> $seqres.full 2>&1
> +if [[ $? == 0 ]]; then
> +	echo "Growfs succeeded; should have failed."
> +	exit 1
> +fi
> +
> +_scratch_unmount >> $seqres.full
> +
> +echo "Verify rbmino's and rsumino's extent count"
> +for rtino in rbmino rsumino; do
> +	ino=$(_scratch_xfs_get_metadata_field $rtino "sb 0")
> +	echo "$rtino = $ino" >> $seqres.full
> +
> +	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"
> +		exit 1
> +	fi
> +done
> +
> +echo "Check filesystem"
> +_check_xfs_filesystem $SCRATCH_DEV none $rtdev
> +
> +losetup -d $rtdev
> +rm -f $TEST_DIR/$seq.rtvol
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/529.out b/tests/xfs/529.out
> new file mode 100644
> index 00000000..4ee113a4
> --- /dev/null
> +++ b/tests/xfs/529.out
> @@ -0,0 +1,11 @@
> +QA output created by 529
> +* Test extending rt inodes
> +Create fake rt volume
> +Format and mount rt volume
> +Consume free space
> +Create fragmented filesystem
> +Inject reduce_max_iextents error tag
> +Inject bmap_alloc_minlen_extent error tag
> +Grow realtime volume
> +Verify rbmino's and rsumino's extent count
> +Check filesystem
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 2356c4a9..5dff7acb 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -526,3 +526,4 @@
>  526 auto quick mkfs
>  527 auto quick quota
>  528 auto quick quota
> +529 auto quick realtime growfs
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-09  5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
  2021-03-10 23:49   ` Allison Henderson
@ 2021-03-22 18:54   ` Darrick J. Wong
  2021-03-23  5:28     ` Chandan Babu R
  2021-04-15  9:33     ` Chandan Babu R
  1 sibling, 2 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-22 18:54 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
> This commit adds a stress test that executes fsstress with
> bmap_alloc_minlen_extent error tag enabled.

Continuing along the theme of watching the magic smoke come out when dir
block size > fs block size, I also observed the following assertion when
running this test:

 XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
 ------------[ cut here ]------------
 WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
 Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
 CPU: 0 PID: 3892 Comm: fsstress Tainted: G           O      5.12.0-rc4-xfsx #rc4
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
 RIP: 0010:assfail+0x3c/0x40 [xfs]
 Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
 RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
 RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
 RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
 R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
 R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
 FS:  00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
 Call Trace:
  xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
  xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
  xfs_dir2_block_removename+0x221/0x290 [xfs]
  xfs_dir_removename+0x1a0/0x220 [xfs]
  xfs_dir_rename+0x343/0x3b0 [xfs]
  xfs_rename+0x79e/0xae0 [xfs]
  xfs_vn_rename+0xdb/0x150 [xfs]
  vfs_rename+0x4e2/0x8e0
  do_renameat2+0x393/0x550
  __x64_sys_rename+0x40/0x50
  do_syscall_64+0x2d/0x40
  entry_SYSCALL_64_after_hwframe+0x44/0xae
 RIP: 0033:0x7fe7b5e9800b
 Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
 RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
 RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
 RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
 RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
 R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
 R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
 ---[ end trace 98f99784621d65fe ]---

It looks to me as though we return from xfs_bunmapi having not completed
all the unmapping work, though I can't tell if that's because bunmapi
returned early because it thought it would overflow the extent count; or
some other reason.

OH CRAP, I just realized that xfs_dir2_shrink_inode only calls
xfs_bunmapi once, which means that if the directory block it's removing
is a multi-fsb block, it will remove the last extent map.  It then trips
the assertion, having left the rest of the directory block still mapped.

This is also what's going on when xfs_inactive_symlink_rmt trips the
same ASSERT(done), because the symlink remote block can span multiple
(two?) fs blocks but we only ever call xfs_bunmapi once.

So, no, there's nothing wrong with this test, but it _did_ shake loose
a couple of XFS bugs.  Congratulations!

So... who wants to tackle this one?  This isn't trivial to clean up
because you'll have to clean up all callers of xfs_dir2_shrink_inode to
handle rolling of the transaction, and I bet the only way to fix this is
to use deferred bunmap items to make sure the unmap always completes.

--D

> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
>  tests/xfs/537     | 84 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/537.out |  7 ++++
>  tests/xfs/group   |  1 +
>  3 files changed, 92 insertions(+)
>  create mode 100755 tests/xfs/537
>  create mode 100644 tests/xfs/537.out
> 
> diff --git a/tests/xfs/537 b/tests/xfs/537
> new file mode 100755
> index 00000000..77fa60d9
> --- /dev/null
> +++ b/tests/xfs/537
> @@ -0,0 +1,84 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Chandan Babu R.  All Rights Reserved.
> +#
> +# FS QA Test 537
> +#
> +# Execute fsstress with bmap_alloc_minlen_extent error tag enabled.
> +#
> +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 "bmap_alloc_minlen_extent"
> +
> +echo "Format and mount fs"
> +_scratch_mkfs_sized $((1024 * 1024 * 1024)) >> $seqres.full
> +_scratch_mount >> $seqres.full
> +
> +bsize=$(_get_file_block_size $SCRATCH_MNT)
> +
> +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 bmap_alloc_minlen_extent error tag"
> +_scratch_inject_error bmap_alloc_minlen_extent 1
> +
> +echo "Scale fsstress args"
> +args=$(_scale_fsstress_args -p $((LOAD_FACTOR * 75)) -n $((TIME_FACTOR * 1000)))
> +
> +echo "Execute fsstress in background"
> +$FSSTRESS_PROG -d $SCRATCH_MNT $args \
> +		 -f bulkstat=0 \
> +		 -f bulkstat1=0 \
> +		 -f fiemap=0 \
> +		 -f getattr=0 \
> +		 -f getdents=0 \
> +		 -f getfattr=0 \
> +		 -f listfattr=0 \
> +		 -f mread=0 \
> +		 -f read=0 \
> +		 -f readlink=0 \
> +		 -f readv=0 \
> +		 -f stat=0 \
> +		 -f aread=0 \
> +		 -f dread=0 > /dev/null 2>&1
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/537.out b/tests/xfs/537.out
> new file mode 100644
> index 00000000..19633a07
> --- /dev/null
> +++ b/tests/xfs/537.out
> @@ -0,0 +1,7 @@
> +QA output created by 537
> +Format and mount fs
> +Consume free space
> +Create fragmented filesystem
> +Inject bmap_alloc_minlen_extent error tag
> +Scale fsstress args
> +Execute fsstress in background
> diff --git a/tests/xfs/group b/tests/xfs/group
> index ba674a58..5c827727 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -534,3 +534,4 @@
>  534 auto quick reflink
>  535 auto quick reflink
>  536 auto quick
> +537 auto stress
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-22 18:54   ` Darrick J. Wong
@ 2021-03-23  5:28     ` Chandan Babu R
  2021-03-23 11:27       ` Chandan Babu R
  2021-03-26  4:05       ` Chandan Babu R
  2021-04-15  9:33     ` Chandan Babu R
  1 sibling, 2 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-23  5:28 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
>> This commit adds a stress test that executes fsstress with
>> bmap_alloc_minlen_extent error tag enabled.
>
> Continuing along the theme of watching the magic smoke come out when dir
> block size > fs block size, I also observed the following assertion when
> running this test:
>
>  XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
>  ------------[ cut here ]------------
>  WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
>  Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
>  CPU: 0 PID: 3892 Comm: fsstress Tainted: G           O      5.12.0-rc4-xfsx #rc4
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>  RIP: 0010:assfail+0x3c/0x40 [xfs]
>  Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
>  RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
>  RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
>  RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
>  R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
>  R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
>  FS:  00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
>  Call Trace:
>   xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
>   xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
>   xfs_dir2_block_removename+0x221/0x290 [xfs]
>   xfs_dir_removename+0x1a0/0x220 [xfs]
>   xfs_dir_rename+0x343/0x3b0 [xfs]
>   xfs_rename+0x79e/0xae0 [xfs]
>   xfs_vn_rename+0xdb/0x150 [xfs]
>   vfs_rename+0x4e2/0x8e0
>   do_renameat2+0x393/0x550
>   __x64_sys_rename+0x40/0x50
>   do_syscall_64+0x2d/0x40
>   entry_SYSCALL_64_after_hwframe+0x44/0xae
>  RIP: 0033:0x7fe7b5e9800b
>  Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
>  RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
>  RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
>  RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
>  RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
>  R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
>  R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
>  ---[ end trace 98f99784621d65fe ]---
>
> It looks to me as though we return from xfs_bunmapi having not completed
> all the unmapping work, though I can't tell if that's because bunmapi
> returned early because it thought it would overflow the extent count; or
> some other reason.

It could also be because the following conditions may have evaluated to true,

if (!wasdel && !isrt) {
    agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
    if (prev_agno != NULLAGNUMBER && prev_agno > agno)
          break;
    prev_agno = agno;
}

i.e. the fs block being unmapped belongs to an AG whose agno is less than that
of the previous fs block that was successfully unmapped.

I can't seem to recreate this bug. I tried it with 64k directory block size
with both 4k and 1k fs block sizes. I will keep trying.

However I hit another call trace with directory block size > fs block size,

------------[ cut here ]------------
WARNING: CPU: 2 PID: 6136 at fs/xfs/libxfs/xfs_bmap.c:717 xfs_bmap_extents_to_btree+0x520/0x5d0
Modules linked in:
CPU: 2 PID: 6136 Comm: fsstress Tainted: G        W         5.12.0-rc2-chandan #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:xfs_bmap_extents_to_btree+0x520/0x5d0
Code: 5f fb ff ff 89 0c 24 48 c7 c2 1d 14 ac b2 b9 9d 02 00 00 31 ff 48 c7 c6 bf 14 ac b2 e8 1e 70 ac 00 44 8b 14 24 e9 55 ff ff ff <0f> 0b c7 44 24 0c e4 ff ff ff e9 0f ff ff ff b9 0e 03 00 00 48 c7
RSP: 0018:ffffb43d4939f5c0 EFLAGS: 00010246
RAX: ffffffffffffffff RBX: ffffa011c8887048 RCX: 0000000000000de5
RDX: 00000000ffffffff RSI: 0000000000000000 RDI: ffffa010e29c9000
RBP: ffffa010e2e00000 R08: ffffffffb180e81a R09: 0000000000000116
R10: 0000000000000000 R11: 0000000000000000 R12: ffffa011c8887000
R13: 0000000000000000 R14: ffffa010e11ed7e0 R15: ffffa011c8468100
FS:  00007efd9458fb80(0000) GS:ffffa011f7c80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007efd9458d000 CR3: 00000001fff6c000 CR4: 00000000000006e0
Call Trace:
 ? __cond_resched+0x16/0x40
 ? __kmalloc_track_caller+0x6d/0x260
 xfs_bmap_add_extent_hole_real+0x747/0x960
 xfs_bmapi_allocate+0x380/0x410
 xfs_bmapi_write+0x507/0x9e0
 xfs_da_grow_inode_int+0x1cd/0x330
 ? _xfs_trans_bjoin+0x72/0x110
 xfs_dir2_grow_inode+0x62/0x110
 ? xfs_trans_log_inode+0xce/0x2d0
 xfs_dir2_sf_to_block+0x103/0x940
 ? xfs_dir2_sf_check+0x8c/0x210
 ? xfs_da_compname+0x19/0x30
 ? xfs_dir2_sf_lookup+0xd0/0x3d0
 xfs_dir2_sf_addname+0x10d/0x910
 xfs_dir_createname+0x1ad/0x210
 ? xfs_trans_log_inode+0xce/0x2d0
 xfs_rename+0x803/0xbb0
 ? avc_has_perm_noaudit+0x83/0x100
 xfs_vn_rename+0xdb/0x150
 vfs_rename+0x691/0xa90
 do_renameat2+0x393/0x540
 __x64_sys_renameat2+0x4b/0x60
 do_syscall_64+0x33/0x40
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7efd945e8e9f
Code: 44 00 00 48 8b 15 f1 5f 16 00 f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 49 89 ca 45 85 c0 74 40 b8 3c 01 00 00 0f 05 <48> 3d 00 f0 ff ff 77 39 41 89 c0 83 f8 ff 74 09 44 89 c0 c3 0f 1f
RSP: 002b:00007fff55bbfee8 EFLAGS: 00000202 ORIG_RAX: 000000000000013c
RAX: ffffffffffffffda RBX: 0000563e5f50654f RCX: 00007efd945e8e9f
RDX: 00000000ffffff9c RSI: 0000563e5ff284e0 RDI: 00000000ffffff9c
RBP: 00007fff55bc0140 R08: 0000000000000004 R09: feff7efdff2f3562
R10: 0000563e5ff286c0 R11: 0000000000000202 R12: 0000563e5f4fb630
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
---[ end trace 6d859f8beaa17680 ]---

>
> OH CRAP, I just realized that xfs_dir2_shrink_inode only calls
> xfs_bunmapi once, which means that if the directory block it's removing
> is a multi-fsb block, it will remove the last extent map.  It then trips
> the assertion, having left the rest of the directory block still mapped.
>
> This is also what's going on when xfs_inactive_symlink_rmt trips the
> same ASSERT(done), because the symlink remote block can span multiple
> (two?) fs blocks but we only ever call xfs_bunmapi once.
>
> So, no, there's nothing wrong with this test, but it _did_ shake loose
> a couple of XFS bugs.  Congratulations!
>
> So... who wants to tackle this one?  This isn't trivial to clean up
> because you'll have to clean up all callers of xfs_dir2_shrink_inode to
> handle rolling of the transaction, and I bet the only way to fix this is
> to use deferred bunmap items to make sure the unmap always completes.

I will work on both the above listed call traces. Thanks for reporting the bug
and also the follow up analysis.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-23  5:28     ` Chandan Babu R
@ 2021-03-23 11:27       ` Chandan Babu R
  2021-03-26  4:05       ` Chandan Babu R
  1 sibling, 0 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-23 11:27 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 23 Mar 2021 at 10:58, Chandan Babu R wrote:
> On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
>> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
>>> This commit adds a stress test that executes fsstress with
>>> bmap_alloc_minlen_extent error tag enabled.
>>
>> Continuing along the theme of watching the magic smoke come out when dir
>> block size > fs block size, I also observed the following assertion when
>> running this test:
>>
>>  XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
>>  ------------[ cut here ]------------
>>  WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
>>  Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
>>  CPU: 0 PID: 3892 Comm: fsstress Tainted: G           O      5.12.0-rc4-xfsx #rc4
>>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>>  RIP: 0010:assfail+0x3c/0x40 [xfs]
>>  Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
>>  RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
>>  RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
>>  RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
>>  RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
>>  R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
>>  R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
>>  FS:  00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
>>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>  CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
>>  Call Trace:
>>   xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
>>   xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
>>   xfs_dir2_block_removename+0x221/0x290 [xfs]
>>   xfs_dir_removename+0x1a0/0x220 [xfs]
>>   xfs_dir_rename+0x343/0x3b0 [xfs]
>>   xfs_rename+0x79e/0xae0 [xfs]
>>   xfs_vn_rename+0xdb/0x150 [xfs]
>>   vfs_rename+0x4e2/0x8e0
>>   do_renameat2+0x393/0x550
>>   __x64_sys_rename+0x40/0x50
>>   do_syscall_64+0x2d/0x40
>>   entry_SYSCALL_64_after_hwframe+0x44/0xae
>>  RIP: 0033:0x7fe7b5e9800b
>>  Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
>>  RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
>>  RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
>>  RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
>>  RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
>>  R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
>>  R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
>>  ---[ end trace 98f99784621d65fe ]---
>>
>> It looks to me as though we return from xfs_bunmapi having not completed
>> all the unmapping work, though I can't tell if that's because bunmapi
>> returned early because it thought it would overflow the extent count; or
>> some other reason.
>
> It could also be because the following conditions may have evaluated to true,
>
> if (!wasdel && !isrt) {
>     agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
>     if (prev_agno != NULLAGNUMBER && prev_agno > agno)
>           break;
>     prev_agno = agno;
> }
>
> i.e. the fs block being unmapped belongs to an AG whose agno is less than that
> of the previous fs block that was successfully unmapped.
>
> I can't seem to recreate this bug. I tried it with 64k directory block size
> with both 4k and 1k fs block sizes. I will keep trying.
>
> However I hit another call trace with directory block size > fs block size,
>
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 6136 at fs/xfs/libxfs/xfs_bmap.c:717 xfs_bmap_extents_to_btree+0x520/0x5d0
> Modules linked in:
> CPU: 2 PID: 6136 Comm: fsstress Tainted: G        W         5.12.0-rc2-chandan #7
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
> RIP: 0010:xfs_bmap_extents_to_btree+0x520/0x5d0
> Code: 5f fb ff ff 89 0c 24 48 c7 c2 1d 14 ac b2 b9 9d 02 00 00 31 ff 48 c7 c6 bf 14 ac b2 e8 1e 70 ac 00 44 8b 14 24 e9 55 ff ff ff <0f> 0b c7 44 24 0c e4 ff ff ff e9 0f ff ff ff b9 0e 03 00 00 48 c7
> RSP: 0018:ffffb43d4939f5c0 EFLAGS: 00010246
> RAX: ffffffffffffffff RBX: ffffa011c8887048 RCX: 0000000000000de5
> RDX: 00000000ffffffff RSI: 0000000000000000 RDI: ffffa010e29c9000
> RBP: ffffa010e2e00000 R08: ffffffffb180e81a R09: 0000000000000116
> R10: 0000000000000000 R11: 0000000000000000 R12: ffffa011c8887000
> R13: 0000000000000000 R14: ffffa010e11ed7e0 R15: ffffa011c8468100
> FS:  00007efd9458fb80(0000) GS:ffffa011f7c80000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007efd9458d000 CR3: 00000001fff6c000 CR4: 00000000000006e0
> Call Trace:
>  ? __cond_resched+0x16/0x40
>  ? __kmalloc_track_caller+0x6d/0x260
>  xfs_bmap_add_extent_hole_real+0x747/0x960
>  xfs_bmapi_allocate+0x380/0x410
>  xfs_bmapi_write+0x507/0x9e0
>  xfs_da_grow_inode_int+0x1cd/0x330
>  ? _xfs_trans_bjoin+0x72/0x110
>  xfs_dir2_grow_inode+0x62/0x110
>  ? xfs_trans_log_inode+0xce/0x2d0
>  xfs_dir2_sf_to_block+0x103/0x940
>  ? xfs_dir2_sf_check+0x8c/0x210
>  ? xfs_da_compname+0x19/0x30
>  ? xfs_dir2_sf_lookup+0xd0/0x3d0
>  xfs_dir2_sf_addname+0x10d/0x910
>  xfs_dir_createname+0x1ad/0x210
>  ? xfs_trans_log_inode+0xce/0x2d0
>  xfs_rename+0x803/0xbb0
>  ? avc_has_perm_noaudit+0x83/0x100
>  xfs_vn_rename+0xdb/0x150
>  vfs_rename+0x691/0xa90
>  do_renameat2+0x393/0x540
>  __x64_sys_renameat2+0x4b/0x60
>  do_syscall_64+0x33/0x40
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7efd945e8e9f
> Code: 44 00 00 48 8b 15 f1 5f 16 00 f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 49 89 ca 45 85 c0 74 40 b8 3c 01 00 00 0f 05 <48> 3d 00 f0 ff ff 77 39 41 89 c0 83 f8 ff 74 09 44 89 c0 c3 0f 1f
> RSP: 002b:00007fff55bbfee8 EFLAGS: 00000202 ORIG_RAX: 000000000000013c
> RAX: ffffffffffffffda RBX: 0000563e5f50654f RCX: 00007efd945e8e9f
> RDX: 00000000ffffff9c RSI: 0000563e5ff284e0 RDI: 00000000ffffff9c
> RBP: 00007fff55bc0140 R08: 0000000000000004 R09: feff7efdff2f3562
> R10: 0000563e5ff286c0 R11: 0000000000000202 R12: 0000563e5f4fb630
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> ---[ end trace 6d859f8beaa17680 ]---
>

The following patch fixed the bug,

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e0905ad171f0..585f7e795023 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3586,7 +3586,8 @@ xfs_bmap_exact_minlen_extent_alloc(
 	args.fsbno = ap->blkno;
 	args.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
 	args.type = XFS_ALLOCTYPE_FIRST_AG;
-	args.total = args.minlen = args.maxlen = ap->minlen;
+	args.minlen = args.maxlen = ap->minlen;
+	args.total = ap->total;
 
 	args.alignment = 1;
 	args.minalignslop = 0;

Without the above change, the space allocator could allocate a block from an
AG which has less than ap->total blocks available. Hence, future allocation
requests from the same transaction could fail.

I will have to go through some more code to confirm the correctness of the
patch. I will post the patch to the mailing list if I find it to be indeed
correct.

-- 
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-22 17:56   ` Darrick J. Wong
@ 2021-03-23 15:51     ` Chandan Babu R
  2021-03-23 20:57       ` Darrick J. Wong
  0 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-23 15:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 22 Mar 2021 at 23:26, Darrick J. Wong wrote:
> On Tue, Mar 09, 2021 at 10:31:16AM +0530, Chandan Babu R wrote:
>> Verify that XFS does not cause realtime bitmap/summary inode fork's
>> extent count to overflow when growing the realtime volume associated
>> with a filesystem.
>>
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>
> Soo... I discovered that this test doesn't pass with multiblock
> directories:

Thanks for the bug report and the description of the corresponding solution. I
am fixing the tests and will soon post corresponding patches to the mailing
list.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-23 15:51     ` Chandan Babu R
@ 2021-03-23 20:57       ` Darrick J. Wong
  2021-03-24 10:46         ` Chandan Babu R
  0 siblings, 1 reply; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-23 20:57 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Tue, Mar 23, 2021 at 09:21:27PM +0530, Chandan Babu R wrote:
> On 22 Mar 2021 at 23:26, Darrick J. Wong wrote:
> > On Tue, Mar 09, 2021 at 10:31:16AM +0530, Chandan Babu R wrote:
> >> Verify that XFS does not cause realtime bitmap/summary inode fork's
> >> extent count to overflow when growing the realtime volume associated
> >> with a filesystem.
> >>
> >> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> >> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> >
> > Soo... I discovered that this test doesn't pass with multiblock
> > directories:
> 
> Thanks for the bug report and the description of the corresponding solution. I
> am fixing the tests and will soon post corresponding patches to the mailing
> list.

Also, I found a problem with xfs/534 when it does the direct write tests
to a pmem volume with DAX enabled:

--- /tmp/fstests/tests/xfs/534.out      2021-03-21 11:44:09.384407426 -0700
+++ /var/tmp/fstests/xfs/534.out.bad    2021-03-23 13:32:15.898301839 -0700
@@ -5,7 +5,4 @@
 Fallocate 15 blocks
 Buffered write to every other block of fallocated space
 Verify $testfile's extent count
-* Direct write to unwritten extent
-Fallocate 15 blocks
-Direct write to every other block of fallocated space
-Verify $testfile's extent count
+Extent count overflow check failed: nextents = 11

looking at the xfs_bmap output for $testfile shows:

/opt/testfile:
 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
   0: [0..7]:          208..215          0 (208..215)           8 010000
   1: [8..15]:         216..223          0 (216..223)           8 000000
   2: [16..23]:        224..231          0 (224..231)           8 010000
   3: [24..31]:        232..239          0 (232..239)           8 000000
   4: [32..39]:        240..247          0 (240..247)           8 010000
   5: [40..47]:        248..255          0 (248..255)           8 000000
   6: [48..55]:        256..263          0 (256..263)           8 010000
   7: [56..63]:        264..271          0 (264..271)           8 000000
   8: [64..71]:        272..279          0 (272..279)           8 010000
   9: [72..79]:        280..287          0 (280..287)           8 000000
  10: [80..119]:       288..327          0 (288..327)          40 010000

Which is ... odd since the same direct write gets cut off after writing
to block 7 (like you'd expect since it's the same function) when DAX
isn't enabled...

...OH, I see the problem.  For a non-DAX direct write,
xfs_iomap_write_direct will allocate an unwritten block into a hole, but
if the block was already mapped (written or unwritten) it won't do
anything at all.  For that case, XFS_IEXT_ADD_NOSPLIT_CNT is sufficient,
because in the worst case we add one extent to the data fork.

For DAX writes, however, the behavior is different:

	if (IS_DAX(VFS_I(ip))) {
		bmapi_flags = XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO;
		if (imap->br_state == XFS_EXT_UNWRITTEN) {
			force = true;
			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0) << 1;
		}
	}

This tells xfs_bmapi_write that we want to /convert/ an unwritten extent
to written, and we want to zero the blocks.  If we're dax-writing into
the middle of an unwritten range, this will cause a split.  The correct
parameter there would be XFS_IEXT_WRITE_UNWRITTEN_CNT.  Would you mind
sending a kernel patch to fix that?

--D

> --
> chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-23 20:57       ` Darrick J. Wong
@ 2021-03-24 10:46         ` Chandan Babu R
  2021-03-24 14:17           ` Chandan Babu R
  0 siblings, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-24 10:46 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 24 Mar 2021 at 02:27, Darrick J. Wong wrote:
> On Tue, Mar 23, 2021 at 09:21:27PM +0530, Chandan Babu R wrote:
>> On 22 Mar 2021 at 23:26, Darrick J. Wong wrote:
>> > On Tue, Mar 09, 2021 at 10:31:16AM +0530, Chandan Babu R wrote:
>> >> Verify that XFS does not cause realtime bitmap/summary inode fork's
>> >> extent count to overflow when growing the realtime volume associated
>> >> with a filesystem.
>> >>
>> >> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> >> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> >
>> > Soo... I discovered that this test doesn't pass with multiblock
>> > directories:
>>
>> Thanks for the bug report and the description of the corresponding solution. I
>> am fixing the tests and will soon post corresponding patches to the mailing
>> list.
>
> Also, I found a problem with xfs/534 when it does the direct write tests
> to a pmem volume with DAX enabled:
>
> --- /tmp/fstests/tests/xfs/534.out      2021-03-21 11:44:09.384407426 -0700
> +++ /var/tmp/fstests/xfs/534.out.bad    2021-03-23 13:32:15.898301839 -0700
> @@ -5,7 +5,4 @@
>  Fallocate 15 blocks
>  Buffered write to every other block of fallocated space
>  Verify $testfile's extent count
> -* Direct write to unwritten extent
> -Fallocate 15 blocks
> -Direct write to every other block of fallocated space
> -Verify $testfile's extent count
> +Extent count overflow check failed: nextents = 11

The inode extent overflow reported above was actually due to the buffered
write operation. But it does occur with direct write operation as well.

I was able to recreate the bug with an emulated pmem device on my qemu guest.

>
> looking at the xfs_bmap output for $testfile shows:
>
> /opt/testfile:
>  EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
>    0: [0..7]:          208..215          0 (208..215)           8 010000
>    1: [8..15]:         216..223          0 (216..223)           8 000000
>    2: [16..23]:        224..231          0 (224..231)           8 010000
>    3: [24..31]:        232..239          0 (232..239)           8 000000
>    4: [32..39]:        240..247          0 (240..247)           8 010000
>    5: [40..47]:        248..255          0 (248..255)           8 000000
>    6: [48..55]:        256..263          0 (256..263)           8 010000
>    7: [56..63]:        264..271          0 (264..271)           8 000000
>    8: [64..71]:        272..279          0 (272..279)           8 010000
>    9: [72..79]:        280..287          0 (280..287)           8 000000
>   10: [80..119]:       288..327          0 (288..327)          40 010000
>
> Which is ... odd since the same direct write gets cut off after writing
> to block 7 (like you'd expect since it's the same function) when DAX
> isn't enabled...
>
> ...OH, I see the problem.  For a non-DAX direct write,
> xfs_iomap_write_direct will allocate an unwritten block into a hole, but
> if the block was already mapped (written or unwritten) it won't do
> anything at all.  For that case, XFS_IEXT_ADD_NOSPLIT_CNT is sufficient,
> because in the worst case we add one extent to the data fork.
>
> For DAX writes, however, the behavior is different:
>
> 	if (IS_DAX(VFS_I(ip))) {
> 		bmapi_flags = XFS_BMAPI_CONVERT | XFS_BMAPI_ZERO;
> 		if (imap->br_state == XFS_EXT_UNWRITTEN) {
> 			force = true;
> 			dblocks = XFS_DIOSTRAT_SPACE_RES(mp, 0) << 1;
> 		}
> 	}
>
> This tells xfs_bmapi_write that we want to /convert/ an unwritten extent
> to written, and we want to zero the blocks.  If we're dax-writing into
> the middle of an unwritten range, this will cause a split.  The correct
> parameter there would be XFS_IEXT_WRITE_UNWRITTEN_CNT.  Would you mind
> sending a kernel patch to fix that?

Sure, I will work on fixing both the buffered and direct IO extent overflow
issues.

Thanks for reporting the bug.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes
  2021-03-24 10:46         ` Chandan Babu R
@ 2021-03-24 14:17           ` Chandan Babu R
  0 siblings, 0 replies; 46+ messages in thread
From: Chandan Babu R @ 2021-03-24 14:17 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 24 Mar 2021 at 16:16, Chandan Babu R wrote:
> On 24 Mar 2021 at 02:27, Darrick J. Wong wrote:
>> On Tue, Mar 23, 2021 at 09:21:27PM +0530, Chandan Babu R wrote:
>>> On 22 Mar 2021 at 23:26, Darrick J. Wong wrote:
>>> > On Tue, Mar 09, 2021 at 10:31:16AM +0530, Chandan Babu R wrote:
>>> >> Verify that XFS does not cause realtime bitmap/summary inode fork's
>>> >> extent count to overflow when growing the realtime volume associated
>>> >> with a filesystem.
>>> >>
>>> >> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>>> >> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>>> >
>>> > Soo... I discovered that this test doesn't pass with multiblock
>>> > directories:
>>>
>>> Thanks for the bug report and the description of the corresponding solution. I
>>> am fixing the tests and will soon post corresponding patches to the mailing
>>> list.
>>
>> Also, I found a problem with xfs/534 when it does the direct write tests
>> to a pmem volume with DAX enabled:
>>
>> --- /tmp/fstests/tests/xfs/534.out      2021-03-21 11:44:09.384407426 -0700
>> +++ /var/tmp/fstests/xfs/534.out.bad    2021-03-23 13:32:15.898301839 -0700
>> @@ -5,7 +5,4 @@
>>  Fallocate 15 blocks
>>  Buffered write to every other block of fallocated space
>>  Verify $testfile's extent count
>> -* Direct write to unwritten extent
>> -Fallocate 15 blocks
>> -Direct write to every other block of fallocated space
>> -Verify $testfile's extent count
>> +Extent count overflow check failed: nextents = 11
>
> The inode extent overflow reported above was actually due to the buffered
> write operation. But it does occur with direct write operation as well.

I just found out that xfs_direct_write_iomap_ops is used for both buffered and
direct IO w.r.t dax devices. Please ignore the above statement.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-23  5:28     ` Chandan Babu R
  2021-03-23 11:27       ` Chandan Babu R
@ 2021-03-26  4:05       ` Chandan Babu R
  2021-03-26  4:21         ` Darrick J. Wong
  1 sibling, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-03-26  4:05 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 23 Mar 2021 at 10:58, Chandan Babu R wrote:
> On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
>> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
>>> This commit adds a stress test that executes fsstress with
>>> bmap_alloc_minlen_extent error tag enabled.
>>
>> Continuing along the theme of watching the magic smoke come out when dir
>> block size > fs block size, I also observed the following assertion when
>> running this test:

Apart from "xfs_dir2_shrink_inode only calls xfs_bunmapi once" bug, I
noticed that scrub has detected metadata inconsistency,

FSTYP         -- xfs (debug)
PLATFORM      -- Linux/x86_64 debian-guest 5.12.0-rc4-chandan #30 SMP Thu Mar 25 11:00:08 IST 2021
MKFS_OPTIONS  -- -f -b size=1k -m rmapbt=0,reflink=0 -n size=64k /dev/vdc2
MOUNT_OPTIONS -- /dev/vdc2 /mnt/scratch

xfs/538 43s ... _check_xfs_filesystem: filesystem on /dev/vdc2 failed scrub
(see /root/repos/xfstests-dev/results//xfs/538.full for details)

Ran: xfs/538
Failures: xfs/538
Failed 1 of 1 tests

I will work on fixing this one as well.

-- 
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-26  4:05       ` Chandan Babu R
@ 2021-03-26  4:21         ` Darrick J. Wong
  0 siblings, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-03-26  4:21 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Fri, Mar 26, 2021 at 09:35:33AM +0530, Chandan Babu R wrote:
> On 23 Mar 2021 at 10:58, Chandan Babu R wrote:
> > On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
> >> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
> >>> This commit adds a stress test that executes fsstress with
> >>> bmap_alloc_minlen_extent error tag enabled.
> >>
> >> Continuing along the theme of watching the magic smoke come out when dir
> >> block size > fs block size, I also observed the following assertion when
> >> running this test:
> 
> Apart from "xfs_dir2_shrink_inode only calls xfs_bunmapi once" bug, I
> noticed that scrub has detected metadata inconsistency,
> 
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 debian-guest 5.12.0-rc4-chandan #30 SMP Thu Mar 25 11:00:08 IST 2021
> MKFS_OPTIONS  -- -f -b size=1k -m rmapbt=0,reflink=0 -n size=64k /dev/vdc2
> MOUNT_OPTIONS -- /dev/vdc2 /mnt/scratch
> 
> xfs/538 43s ... _check_xfs_filesystem: filesystem on /dev/vdc2 failed scrub
> (see /root/repos/xfstests-dev/results//xfs/538.full for details)
> 
> Ran: xfs/538
> Failures: xfs/538
> Failed 1 of 1 tests
> 
> I will work on fixing this one as well.

Yeah, I noticed that too.  There are two bugs -- the first one is that
the "Block directories only have a single block at offset 0" check in
xchk_directory_blocks is wrong -- they can have a single *dir* block at
offset 0.  The correct check is (I think):

	if (is_block && got.br_startoff >= args.geo->fsbcount) {
		xchk_fblock_set_corrupt(...);
		break;
	}

The second problem is ... somewhere in the bmbt scrubber.  It looks like
there's a file with a data fork in btree format; the data fork has a
single child block; and there are few enough key/ptrs in that child
block that it /could/ fit in the root.  For some reason the btree code
didn't promote it, however.  Seeing as nobody's touched that code
lately, that probably means that scrub is wrong, but OTOH that /does/
seem like a waste of a block.

But, that's as far as I've gotten on that second one and it's late.
Thanks for taking a look! :)

--D

> -- 
> chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-03-22 18:54   ` Darrick J. Wong
  2021-03-23  5:28     ` Chandan Babu R
@ 2021-04-15  9:33     ` Chandan Babu R
  2021-04-17  0:26       ` Darrick J. Wong
  1 sibling, 1 reply; 46+ messages in thread
From: Chandan Babu R @ 2021-04-15  9:33 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: fstests, linux-xfs

On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
>> This commit adds a stress test that executes fsstress with
>> bmap_alloc_minlen_extent error tag enabled.
>
> Continuing along the theme of watching the magic smoke come out when dir
> block size > fs block size, I also observed the following assertion when
> running this test:
>
>  XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
>  ------------[ cut here ]------------
>  WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
>  Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
>  CPU: 0 PID: 3892 Comm: fsstress Tainted: G           O      5.12.0-rc4-xfsx #rc4
>  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>  RIP: 0010:assfail+0x3c/0x40 [xfs]
>  Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
>  RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
>  RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
>  RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
>  R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
>  R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
>  FS:  00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
>  Call Trace:
>   xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
>   xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
>   xfs_dir2_block_removename+0x221/0x290 [xfs]
>   xfs_dir_removename+0x1a0/0x220 [xfs]
>   xfs_dir_rename+0x343/0x3b0 [xfs]
>   xfs_rename+0x79e/0xae0 [xfs]
>   xfs_vn_rename+0xdb/0x150 [xfs]
>   vfs_rename+0x4e2/0x8e0
>   do_renameat2+0x393/0x550
>   __x64_sys_rename+0x40/0x50
>   do_syscall_64+0x2d/0x40
>   entry_SYSCALL_64_after_hwframe+0x44/0xae
>  RIP: 0033:0x7fe7b5e9800b
>  Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
>  RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
>  RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
>  RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
>  RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
>  R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
>  R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
>  ---[ end trace 98f99784621d65fe ]---
>
> It looks to me as though we return from xfs_bunmapi having not completed
> all the unmapping work, though I can't tell if that's because bunmapi
> returned early because it thought it would overflow the extent count; or
> some other reason.
>
> OH CRAP, I just realized that xfs_dir2_shrink_inode only calls
> xfs_bunmapi once, which means that if the directory block it's removing
> is a multi-fsb block, it will remove the last extent map.  It then trips
> the assertion, having left the rest of the directory block still mapped.
>
> This is also what's going on when xfs_inactive_symlink_rmt trips the
> same ASSERT(done), because the symlink remote block can span multiple
> (two?) fs blocks but we only ever call xfs_bunmapi once.
>
> So, no, there's nothing wrong with this test, but it _did_ shake loose
> a couple of XFS bugs.  Congratulations!
>
> So... who wants to tackle this one?  This isn't trivial to clean up
> because you'll have to clean up all callers of xfs_dir2_shrink_inode to
> handle rolling of the transaction, and I bet the only way to fix this is
> to use deferred bunmap items to make sure the unmap always completes.
>

I was wondering as to why the above described bug does not occur when
allocating blocks via xfs_bmap_btalloc(). This led me to the following,

1. When using xfs_bmap_btalloc() to allocate a directory block,
   xfs_bmalloca->total is set to total number of fs blocks required for the
   transaction to complete successfully. This includes blocks required to
   allocate
   - Data block
   - Free index block
   - Dabtree blocks and
   - Bmbt blocks.

2. Most of the time (please refer to step #5 for a description of the
   exceptional case), xfs_bmap_btalloc() chooses an AG for space allocation
   only when the AG has atleast xfs_bmalloca->total number of free blocks. On
   finding such an AG, the corresponding AGF buffer is locked by the
   transaction and this guarantees that the fs blocks that make up a directory
   block are allocated from within the same AG. This is probably the reason
   for xfs_dir2_shrink_inode() to assume that __xfs_bunmapi() will be able to
   unmap all the constituent fs blocks.

3. The call trace posted above occurs when __xfs_bunmapi() starts unmapping
   the fs blocks of a directory block and one of the fs blocks happens to be
   from an AG whose AG number is less than that of fs block that was unmapped
   earlier.

4. The xfs_bmap_exact_minlen_extent_alloc() allocator can cause allocation of
   a directory block whose constituent fs blocks are from different AGs. This
   occurs because,
   - xfs_bmap_exact_minlen_extent_alloc() gets an AG which has atleast
     xfs_bmalloca->total free fs blocks.
   - However some of those free fs blocks do not correspond to one-block sized
     extents (NOTE: xfs/538 test fragments 90% of the fs free space).
   - Once the current AG runs out of one-block sized extents, we move onto the
     next AG. This happens because xfs_bmap_exact_minlen_extent_alloc() uses
     XFS_ALLOCTYPE_FIRST_AG as the allocation type and this in turn causes the
     allocator code to iterate across AGs to get free blocks.

5. From code reading, I noticed that the scenario described in step #4 could
   also occur when using xfs_bmap_btalloc(). This happens when the filesystem
   is highly fragmented and is also running low on free space. In such a
   scenario, XFS_TRANS_LOWMODE is enabled causing xfs_bmap_btalloc() to
   execute the same sequence of steps described in step #4. This scenario
   (i.e. fragmented fs and running low on free space) is probably quite rare
   to occur in practice and hence this may be the reason as to why this
   problem was not observed earlier.

--
chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
  2021-04-15  9:33     ` Chandan Babu R
@ 2021-04-17  0:26       ` Darrick J. Wong
  0 siblings, 0 replies; 46+ messages in thread
From: Darrick J. Wong @ 2021-04-17  0:26 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: fstests, linux-xfs

On Thu, Apr 15, 2021 at 03:03:47PM +0530, Chandan Babu R wrote:
> On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
> > On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
> >> This commit adds a stress test that executes fsstress with
> >> bmap_alloc_minlen_extent error tag enabled.
> >
> > Continuing along the theme of watching the magic smoke come out when dir
> > block size > fs block size, I also observed the following assertion when
> > running this test:
> >
> >  XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
> >  ------------[ cut here ]------------
> >  WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
> >  Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
> >  CPU: 0 PID: 3892 Comm: fsstress Tainted: G           O      5.12.0-rc4-xfsx #rc4
> >  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> >  RIP: 0010:assfail+0x3c/0x40 [xfs]
> >  Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
> >  RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
> >  RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
> >  RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
> >  RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
> >  R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
> >  R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
> >  FS:  00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
> >  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >  CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
> >  Call Trace:
> >   xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
> >   xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
> >   xfs_dir2_block_removename+0x221/0x290 [xfs]
> >   xfs_dir_removename+0x1a0/0x220 [xfs]
> >   xfs_dir_rename+0x343/0x3b0 [xfs]
> >   xfs_rename+0x79e/0xae0 [xfs]
> >   xfs_vn_rename+0xdb/0x150 [xfs]
> >   vfs_rename+0x4e2/0x8e0
> >   do_renameat2+0x393/0x550
> >   __x64_sys_rename+0x40/0x50
> >   do_syscall_64+0x2d/0x40
> >   entry_SYSCALL_64_after_hwframe+0x44/0xae
> >  RIP: 0033:0x7fe7b5e9800b
> >  Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
> >  RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
> >  RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
> >  RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
> >  RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
> >  R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
> >  R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
> >  ---[ end trace 98f99784621d65fe ]---
> >
> > It looks to me as though we return from xfs_bunmapi having not completed
> > all the unmapping work, though I can't tell if that's because bunmapi
> > returned early because it thought it would overflow the extent count; or
> > some other reason.
> >
> > OH CRAP, I just realized that xfs_dir2_shrink_inode only calls
> > xfs_bunmapi once, which means that if the directory block it's removing
> > is a multi-fsb block, it will remove the last extent map.  It then trips
> > the assertion, having left the rest of the directory block still mapped.
> >
> > This is also what's going on when xfs_inactive_symlink_rmt trips the
> > same ASSERT(done), because the symlink remote block can span multiple
> > (two?) fs blocks but we only ever call xfs_bunmapi once.
> >
> > So, no, there's nothing wrong with this test, but it _did_ shake loose
> > a couple of XFS bugs.  Congratulations!
> >
> > So... who wants to tackle this one?  This isn't trivial to clean up
> > because you'll have to clean up all callers of xfs_dir2_shrink_inode to
> > handle rolling of the transaction, and I bet the only way to fix this is
> > to use deferred bunmap items to make sure the unmap always completes.
> >
> 
> I was wondering as to why the above described bug does not occur when
> allocating blocks via xfs_bmap_btalloc(). This led me to the following,
> 
> 1. When using xfs_bmap_btalloc() to allocate a directory block,
>    xfs_bmalloca->total is set to total number of fs blocks required for the
>    transaction to complete successfully. This includes blocks required to
>    allocate
>    - Data block
>    - Free index block
>    - Dabtree blocks and
>    - Bmbt blocks.
> 
> 2. Most of the time (please refer to step #5 for a description of the
>    exceptional case), xfs_bmap_btalloc() chooses an AG for space allocation
>    only when the AG has atleast xfs_bmalloca->total number of free blocks. On
>    finding such an AG, the corresponding AGF buffer is locked by the
>    transaction and this guarantees that the fs blocks that make up a directory
>    block are allocated from within the same AG. This is probably the reason
>    for xfs_dir2_shrink_inode() to assume that __xfs_bunmapi() will be able to
>    unmap all the constituent fs blocks.

That sounds right to me...

> 
> 3. The call trace posted above occurs when __xfs_bunmapi() starts unmapping
>    the fs blocks of a directory block and one of the fs blocks happens to be
>    from an AG whose AG number is less than that of fs block that was unmapped
>    earlier.
> 
> 4. The xfs_bmap_exact_minlen_extent_alloc() allocator can cause allocation of
>    a directory block whose constituent fs blocks are from different AGs. This
>    occurs because,
>    - xfs_bmap_exact_minlen_extent_alloc() gets an AG which has atleast
>      xfs_bmalloca->total free fs blocks.
>    - However some of those free fs blocks do not correspond to one-block sized
>      extents (NOTE: xfs/538 test fragments 90% of the fs free space).
>    - Once the current AG runs out of one-block sized extents, we move onto the
>      next AG. This happens because xfs_bmap_exact_minlen_extent_alloc() uses
>      XFS_ALLOCTYPE_FIRST_AG as the allocation type and this in turn causes the
>      allocator code to iterate across AGs to get free blocks.

...and the effect of your minlen debug knob is that we will pound on the
lowmode allocator much more frequently.  Multi-fsb directories are
already kind of rare, which means there probably aren't a lot of people
dealing with this combination.

IIRC the /only/ way you can have a multiblock symlink is on a v5
filesystem with 1k blocks, since the max symlink target length is 1024
bytes.

> 5. From code reading, I noticed that the scenario described in step #4 could
>    also occur when using xfs_bmap_btalloc(). This happens when the filesystem
>    is highly fragmented and is also running low on free space. In such a
>    scenario, XFS_TRANS_LOWMODE is enabled causing xfs_bmap_btalloc() to
>    execute the same sequence of steps described in step #4. This scenario
>    (i.e. fragmented fs and running low on free space) is probably quite rare
>    to occur in practice and hence this may be the reason as to why this
>    problem was not observed earlier.

Yes, I think this is accurate.

--D

> 
> --
> chandan

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, back to index

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09  5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
2021-03-09  5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
2021-03-09  5:03   ` Darrick J. Wong
2021-03-10  6:12   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
2021-03-09  5:04   ` Darrick J. Wong
2021-03-10  6:12   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
2021-03-09  5:04   ` Darrick J. Wong
2021-03-10  6:13   ` Allison Henderson
2021-03-10 19:54     ` Darrick J. Wong
2021-03-11  2:54     ` Chandan Babu R
2021-03-11  8:52   ` [PATCH V6.1] " Chandan Babu R
2021-03-11 18:36     ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
2021-03-10 19:54   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
2021-03-10 19:55   ` Allison Henderson
2021-03-22 17:56   ` Darrick J. Wong
2021-03-23 15:51     ` Chandan Babu R
2021-03-23 20:57       ` Darrick J. Wong
2021-03-24 10:46         ` Chandan Babu R
2021-03-24 14:17           ` Chandan Babu R
2021-03-09  5:01 ` [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole Chandan Babu R
2021-03-10 19:55   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
2021-03-10 19:55   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
2021-03-10 22:41   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
2021-03-10 22:41   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
2021-03-10 22:41   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent Chandan Babu R
2021-03-10 23:49   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents Chandan Babu R
2021-03-10 23:49   ` Allison Henderson
2021-03-09  5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
2021-03-10 23:49   ` Allison Henderson
2021-03-22 18:54   ` Darrick J. Wong
2021-03-23  5:28     ` Chandan Babu R
2021-03-23 11:27       ` Chandan Babu R
2021-03-26  4:05       ` Chandan Babu R
2021-03-26  4:21         ` Darrick J. Wong
2021-04-15  9:33     ` Chandan Babu R
2021-04-17  0:26       ` Darrick J. Wong

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git