Skip to contentSkip to conversation

Addition of new tests for extsize hints

PATCH In review Nirjhar Roy 2 participants 6 messages Nov 27, 2024

lore
Share & tools
lore
#0 lore raw

v5 cover Addition of new tests for extsize hints

This test checks the behaviour of xfs/ext4 filesystems when extsize hint is set on files with inode size as 0, non-empty
files with allocated and delalloc extents and so on.
This test set also tests one of the scenarios where setting extsize hints on files with delayed allocation incorrectly succeeds and there is a recent patch series[1] that got merged[2] which fixes this.

Currently this test only runs in xfs but there is a patch series[3] in review that adds support for the extsize hints for ext4 as well.

I have tested it on ppc64le (with page size 64k) and x86_64 (with page size 4k). The block sizes I have tested with are 2k, 4k, 8k, 16k, 32k, 64k with extsize being twice of the block size or 4 * block size(if the default extsize is 2 * blocksize). I have also tested with configurations where I added extszinherit mkfs option on the root. I had to enable CONFIG_TRANSPARENT_HUGEPAGE to enable block size greater than 4k on x86_64.

Changes since [v4]
- Added RBs from Darrick on Patch-2.
- Added RBs from Christoph on all the patches.
- Correcting test id in the comment section of the test added.
- Correction in the parameter arrangement in _fixed_by_kernel_commit in the test added.

Changes since [v3]
- Added RBs from Darrick on Patch-1 & 3
- Added _require_xfs_io_command "extsize" in _require_scratch_extsize() in patch 2/3.
- Renamed tests/generic/366 to tests/generic/367 since the former is now taken.
- Removed an extra blank line in patch 2/3.

Changes since [v2]
- Renamed _test_fsx_xflags_field() to _test_fsxattr_xflag().
- Changed the definition of _test_fsx_xflags_field (now renamed to _test_fsxattr_xflag) to accept xflag names (like extsize, cowextsize) instead of single characters (like 'e') and modified the callers of this function accordingly (in xfs/207 and generic/366)
- Some changes in the comments in the setup() function of tests/generic/366.
- Added another helper function _require_scratch_extsize().
- Replaced the usage of _require_scratch() with _require_scratch_extsize() in tests/generic/366.
- Fixed some indentation issues in the test scripts(xfs/207, generic/366) and the commit messages.

Changes since [v1]
- Making the definition of _test_xfs_xflags_field() more compact and renamed _test_xfs_xflags_field() to _test_fsx_xflags_field() since I moved it to common/rc.
- Removed the explicit import of common/xfs from the test script.
- Renamed the test file from generic/365 to generic/366 since generic/365 was taken.
- Made the following functions in tests/generic/366 accept a second parameter (extsize)
a) check_extsz_xflag_across_remount()
b) check_extsz_and_xflag()
c) read_file_extsize()
d) filter_extsz
This will help filter out any extsize that the test wants. This is required because, this version no longer assumes any particular fixed value of the default extsize.
- Removed the check for xflags in test_data_allocated() and test_data_delayed() test
- Added an extra line in output that denotes that we are checking that the extent size hasn't changed if the xfs_io command fails (in test_data_allocated() and test_data_delayed() test).
- Changed the new extsize to 4*blocksize if default extsize size is 2*blocksize to make sure that the new extsize is not the same as the default extsize so that we can observe it changing.

[1] https://lore.kernel.org/all/[email protected]/
[2] kernel commit - 2a492ff66673
[3] https://lore.kernel.org/linux-ext4/[email protected]/
[v1] https://lore.kernel.org/all/[email protected]/
[v2] https://lore.kernel.org/all/[email protected]/
[v3] https://lore.kernel.org/all/[email protected]/
[v4] https://lore.kernel.org/all/[email protected]/

Nirjhar Roy (3):
common/rc,xfs/207: Add a common helper function to check xflag bits common/rc: Add a new _require_scratch_extsize helper function
generic: Addition of new tests for extsize hints

 common/rc             |  24 ++++++
 tests/generic/367     | 174 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/367.out |  26 +++++++
 tests/xfs/207         |  15 +---
 4 files changed, 228 insertions(+), 11 deletions(-)
 create mode 100755 tests/generic/367
 create mode 100644 tests/generic/367.out
Signature · 1 line
2.43.5
#1 lore raw

PATCH v5 1/3 common/rc,xfs/207: Add a common helper function to check xflag bits

This patch defines a common helper function to test whether any of fsxattr xflags field is set or not. We will use this helper in an upcoming patch for checking extsize (e) flag.

---
 common/rc     |  7 +++++++
 tests/xfs/207 | 15 ++++-----------
 2 files changed, 11 insertions(+), 11 deletions(-)
common/rc +7 -0
index 2ee46e51..f94bee5e 100644
@@ -41,6 +41,13 @@ _md5_checksum()
 	md5sum $1 | cut -d ' ' -f1
 }
 
+# Check whether a fsxattr xflags name ($2) field is set on a given file ($1).
+# e.g, fsxattr.xflags =  0x80000800 [extsize, has-xattr]
+_test_fsxattr_xflag()
+{
+	grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
+}
+
 # Write a byte into a range of a file
 _pwrite_byte() {
 	local pattern="$1"
tests/xfs/207 +4 -11
index bbe21307..394e7e55 100755
@@ -21,15 +21,6 @@ _require_cp_reflink
 _require_xfs_io_command "fiemap"
 _require_xfs_io_command "cowextsize"
 
-# Takes the fsxattr.xflags line,
-# i.e. fsxattr.xflags = 0x0 [--------------C-]
-# and tests whether a flag character is set
-test_xflag()
-{
-    local flg=$1
-    grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset"
-}
-
 echo "Format and mount"
 _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount >> $seqres.full 2>&1
@@ -65,14 +56,16 @@ echo "Set cowextsize and check flag"
 $XFS_IO_PROG -c "cowextsize 1048576" $testdir/file3 | _filter_scratch
 _scratch_cycle_mount
 
-$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C"
+_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \
+	echo "C flag unset"
 $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch
 
 echo "Unset cowextsize and check flag"
 $XFS_IO_PROG -c "cowextsize 0" $testdir/file3 | _filter_scratch
 _scratch_cycle_mount
 
-$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C"
+_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \
+	echo "C flag unset"
 $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch
 
 status=0
Signature · 1 line
2.43.5
#2 lore raw

PATCH v5 2/3 common/rc: Add a new _require_scratch_extsize helper function

In reply to Nirjhar Roy

_require_scratch_extsize helper function will be used in the the next patch to make the test run only on filesystems with extsize support.

---
 common/rc | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
common/rc +17 -0
index f94bee5e..e6c6047d 100644
@@ -48,6 +48,23 @@ _test_fsxattr_xflag()
 	grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
 }
 
+# This test requires extsize support on the  filesystem
+_require_scratch_extsize()
+{
+	_require_scratch
+	_require_xfs_io_command "extsize"
+	_scratch_mkfs > /dev/null
+	_scratch_mount
+	local filename=$SCRATCH_MNT/$RANDOM
+	local blksz=$(_get_block_size $SCRATCH_MNT)
+	local extsz=$(( blksz*2 ))
+	local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \
+		-c "extsize")
+	_scratch_unmount
+	grep -q "\[$extsz\] $filename" <(echo $res) || \
+		_notrun "this test requires extsize support on the filesystem"
+}
+
 # Write a byte into a range of a file
 _pwrite_byte() {
 	local pattern="$1"
Signature · 1 line
2.43.5
Reviewed-by on v5 2/3
#3 lore raw

Re: [PATCH v5 2/3] common/rc: Add a new _require_scratch_extsize helper function

Quoted 4 lines · Nirjhar Roy

_require_scratch_extsize helper function will be used in the the next patch to make the test run only on filesystems with extsize support.

Sure. Thanks for addressing the review comments. The patch looks good to me. Please feel free to add -

#4 lore raw

Re: [PATCH v5 2/3] common/rc: Add a new _require_scratch_extsize helper function

Quoted 10 lines · Ritesh Harjani (IBM)
Quoted 4 lines · Nirjhar Roy

_require_scratch_extsize helper function will be used in the the next patch to make the test run only on filesystems with extsize support.

Sure. Thanks for addressing the review comments. The patch looks good to me. Please feel free to add -

Thank you.

Signature · 4 lines
---
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#5 lore raw

PATCH v5 3/3 generic: Addition of new tests for extsize hints

In reply to Nirjhar Roy

This commit adds new tests that checks the behaviour of xfs/ext4 filesystems when extsize hint is set on file with inode size as 0, non-empty files with allocated and delalloc extents and so on. Although currently this test is placed under tests/generic, it only runs on xfs and there is an ongoing patch series[1] to enable extsize hints for ext4 as well.

[1] https://lore.kernel.org/linux-ext4/[email protected]/

Reviewed-by Ritesh Harjani (IBM) <[email protected]>

---
 tests/generic/367     | 174 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/367.out |  26 +++++++
 2 files changed, 200 insertions(+)
 create mode 100755 tests/generic/367
 create mode 100644 tests/generic/367.out
tests/generic/367 new file +174 -0
new file mode 100755
index 00000000..7cf90695
@@ -0,0 +1,174 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Nirjhar Roy ([email protected]).  All Rights Reserved.
+#
+# FS QA Test 367
+#
+# This test verifies that extent allocation hint setting works correctly on
+# files with no extents allocated and non-empty files which are truncated.
+# It also checks that the  extent hints setting fails with non-empty file
+# i.e, with any file with allocated extents or delayed allocation. We also
+# check if the extsize value and the xflag bit actually got reflected after
+# setting/re-setting the extsize value.
+
+. ./common/config
+. ./common/filter
+. ./common/preamble
+
+_begin_fstest ioctl quick
+
+_fixed_by_kernel_commit 2a492ff66673 \
+	"xfs: Check for delayed allocations before setting extsize"
+
+_require_scratch_extsize
+
+FILE_DATA_SIZE=1M
+
+get_default_extsize()
+{
+    if [ -z $1 ] || [ ! -d $1 ]; then
+        echo "Missing mount point argument for get_default_extsize"
+        exit 1
+    fi
+    $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'
+}
+
+filter_extsz()
+{
+    sed "s/\[$1\]/\[EXTSIZE\]/g"
+}
+
+setup()
+{
+    _scratch_mkfs >> "$seqres.full"  2>&1
+    _scratch_mount >> "$seqres.full" 2>&1
+    BLKSZ=`_get_block_size $SCRATCH_MNT`
+    DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT`
+    EXTSIZE=$(( BLKSZ*2 ))
+    # Make sure the new extsize is not the same as the default
+    # extsize so that we can observe it changing
+    [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 ))
+}
+
+read_file_extsize()
+{
+    $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2
+}
+
+check_extsz_and_xflag()
+{
+    local filename=$1
+    local extsize=$2
+    read_file_extsize $filename $extsize
+    _test_fsxattr_xflag $filename "extsize" && echo "e flag set" || \
+	    echo "e flag unset"
+}
+
+check_extsz_xflag_across_remount()
+{
+    local filename=$1
+    local extsize=$2
+    _scratch_cycle_mount
+    check_extsz_and_xflag $filename $extsize
+}
+
+# Extsize flag should be cleared when extsize is reset, so this function
+# checks that this behavior is followed.
+reset_extsz_and_recheck_extsz_xflag()
+{
+    local filename=$1
+    echo "Re-setting extsize hint to 0"
+    $XFS_IO_PROG -c "extsize 0" $filename
+    check_extsz_xflag_across_remount $filename "0"
+}
+
+check_extsz_xflag_before_and_after_reset()
+{
+    local filename=$1
+    local extsize=$2
+    check_extsz_xflag_across_remount $filename $extsize
+    reset_extsz_and_recheck_extsz_xflag $filename
+}
+
+test_empty_file()
+{
+    echo "TEST: Set extsize on empty file"
+    local filename=$1
+    $XFS_IO_PROG \
+        -c "open -f $filename" \
+        -c "extsize $EXTSIZE" \
+
+    check_extsz_xflag_before_and_after_reset $filename $EXTSIZE
+    echo
+}
+
+test_data_delayed()
+{
+    echo "TEST: Set extsize on non-empty file with delayed allocation"
+    local filename=$1
+    $XFS_IO_PROG \
+        -c "open -f $filename" \
+        -c "pwrite -q  0 $FILE_DATA_SIZE" \
+        -c "extsize $EXTSIZE" | _filter_scratch
+
+    echo "test for default extsize setting if any"
+    read_file_extsize $filename $DEFAULT_EXTSIZE
+    echo
+}
+
+test_data_allocated()
+{
+    echo "TEST: Set extsize on non-empty file with allocated extents"
+    local filename=$1
+    $XFS_IO_PROG \
+        -c "open -f $filename" \
+        -c "pwrite -qW  0 $FILE_DATA_SIZE" \
+        -c "extsize $EXTSIZE" | _filter_scratch
+
+    echo "test for default extsize setting if any"
+    read_file_extsize $filename $DEFAULT_EXTSIZE
+    echo
+}
+
+test_truncate_allocated()
+{
+    echo "TEST: Set extsize after truncating a file with allocated extents"
+    local filename=$1
+    $XFS_IO_PROG \
+        -c "open -f $filename" \
+        -c "pwrite -qW  0 $FILE_DATA_SIZE" \
+        -c "truncate 0" \
+        -c "extsize $EXTSIZE" \
+
+    check_extsz_xflag_across_remount $filename $EXTSIZE
+    echo
+}
+
+test_truncate_delayed()
+{
+    echo "TEST: Set extsize after truncating a file with delayed allocation"
+    local filename=$1
+    $XFS_IO_PROG \
+        -c "open -f $filename" \
+        -c "pwrite -q  0 $FILE_DATA_SIZE" \
+        -c "truncate 0" \
+        -c "extsize $EXTSIZE" \
+
+    check_extsz_xflag_across_remount $filename $EXTSIZE
+    echo
+}
+
+setup
+echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE \
+	BLOCKSIZE = $BLKSZ\n" >> "$seqres.full"
+
+NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file-
+
+test_empty_file "$NEW_FILE_NAME_PREFIX"00
+test_data_delayed "$NEW_FILE_NAME_PREFIX"01
+test_data_allocated "$NEW_FILE_NAME_PREFIX"02
+test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03
+test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04
+
+status=0
+exit
tests/generic/367.out new file +26 -0
new file mode 100644
index 00000000..f94e8545
@@ -0,0 +1,26 @@
+QA output created by 367
+TEST: Set extsize on empty file
+[EXTSIZE] SCRATCH_MNT/new-file-00
+e flag set
+Re-setting extsize hint to 0
+[EXTSIZE] SCRATCH_MNT/new-file-00
+e flag unset
+
+TEST: Set extsize on non-empty file with delayed allocation
+xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument
+test for default extsize setting if any
+[EXTSIZE] SCRATCH_MNT/new-file-01
+
+TEST: Set extsize on non-empty file with allocated extents
+xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument
+test for default extsize setting if any
+[EXTSIZE] SCRATCH_MNT/new-file-02
+
+TEST: Set extsize after truncating a file with allocated extents
+[EXTSIZE] SCRATCH_MNT/new-file-03
+e flag set
+
+TEST: Set extsize after truncating a file with delayed allocation
+[EXTSIZE] SCRATCH_MNT/new-file-04
+e flag set
+
Signature · 1 line
2.43.5