generic: Addition of new tests for extsize hints
cover generic: Addition of new tests for extsize hints
This test is basically to check the functionality of setting extsize hints for xfs filesystem through an ioctl. It covers various scenarios like setting and then getting extsize hints for various types of files like an empty file, non-empty file (with delayed allocation and allocated extents). This test set also tests one of scenarios where setting extsize hints on files with delayed allocated (incorrectly) succeeds and there is an ongoing patch series[1] that fixes it.
Currently this test only runs in xfs but there is an patch series[2] 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, 64k with extsize being twice of the block size. I had to enable CONFIG_TRANSPARENT_HUGEPAGE to enable block size greater than 4k on x86_64.
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/linux-ext4/[email protected]/
Nirjhar Roy (2):
common/xfs,xfs/207: Adding a common helper function to check xflag
bits on a given file
generic: Addition of new tests for extsize hints
common/xfs | 9 +++ tests/generic/365 | 156 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/365.out | 26 +++++++ tests/xfs/207 | 14 +--- 4 files changed, 194 insertions(+), 11 deletions(-) create mode 100755 tests/generic/365 create mode 100644 tests/generic/365.out
Signature · 1 line
2.43.5
PATCH 1/2 common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
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 the next patch for checking extsize (e) flag.
--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)
common/xfs +9 -0
@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file. +# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1 +} + _setup_large_xfs_fs() { fs_size=$1
tests/xfs/207 +3 -11
@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfs _require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && 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
PATCH 2/2 generic: Addition of new tests for extsize hints
In reply to Nirjhar RoyThis 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]/
--- tests/generic/365 | 156 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/365.out | 26 +++++++ 2 files changed, 182 insertions(+) create mode 100755 tests/generic/365 create mode 100644 tests/generic/365.out
tests/generic/365 new file +156 -0
@@ -0,0 +1,156 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 365 +# +# 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 +. ./common/xfs + +_begin_fstest ioctl quick + +_supported_fs xfs + +_fixed_by_kernel_commit XXXXXXXXXXXX \ + "xfs: Check for delayed allocations before setting extsize", + +_require_scratch + +FILE_DATA_SIZE=1M + +filter_extsz() +{ + sed "s/$EXTSIZE/EXTSIZE/g" +} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) +} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz +} + +check_extsz_and_xflag() +{ + local filename=$1 + read_file_extsize $filename + _test_xfs_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + _scratch_cycle_mount + check_extsz_and_xflag $filename +} + +# 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 +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + check_extsz_xflag_across_remount $filename + 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 + 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 + + check_extsz_xflag_across_remount $filename + 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 + + check_extsz_xflag_across_remount $filename + 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 + 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 + echo +} + +setup +echo -e "EXTSIZE = $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/365.out new file +26 -0
@@ -0,0 +1,26 @@ +QA output created by 365 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[0] 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 +[0] SCRATCH_MNT/new-file-01 +e flag unset + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +[0] SCRATCH_MNT/new-file-02 +e flag unset + +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
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Nirjhar RoyQuoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0
common/xfs __generate_xfs_report_vars() { · line 19
Quoted 7 lines
common/xfs +4
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file. +# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field()
Seeing as fsxattr got added to ext4 and others, this probably should be called _test_fsxattr_xflags() and live in common/rc.
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 3 lines
common/xfs +3
-0+{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1
No need for this bit, the grep -q will set the return value to 0 or 1 and bash will leave that set for the caller.
--D
Quoted 53 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +3
-11@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfs _require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Quoted 43 lines · 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]/
Suggested-by Ojaswin Mujoo <[email protected]>Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/365 | 156 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/365.out | 26 +++++++ 2 files changed, 182 insertions(+) create mode 100755 tests/generic/365 create mode 100644 tests/generic/365.outtests/generic/365 new file +17
-0@@ -0,0 +1,156 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 365 +# +# 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 +. ./common/xfs +
tests/generic/365 line 27
Quoted 10 lines
tests/generic/365 new file +10
-0+_begin_fstest ioctl quick + +_supported_fs xfs + +_fixed_by_kernel_commit XXXXXXXXXXXX \ + "xfs: Check for delayed allocations before setting extsize", + +_require_scratch + +FILE_DATA_SIZE=1M
Do these tests work correctly with fsblock size of 64k? Just curious since Pankaj just sent a series doing 1M -> 4M bumps to fix quota issues.
tests/generic/365 line 39
Quoted 11 lines
tests/generic/365 new file +11
-0+filter_extsz() +{ + sed "s/$EXTSIZE/EXTSIZE/g" +} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 ))
Might want to check that there isn't an extsize/cowextsize set on the root directory due to mkfs options.
Quoted 93 lines
tests/generic/365 new file +93
-0+} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz +} + +check_extsz_and_xflag() +{ + local filename=$1 + read_file_extsize $filename + _test_xfs_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + _scratch_cycle_mount + check_extsz_and_xflag $filename +} + +# 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 +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + check_extsz_xflag_across_remount $filename + 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 + 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 + + check_extsz_xflag_across_remount $filename + 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 + + check_extsz_xflag_across_remount $filename + 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 + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation"
tests/generic/365 line 142
Quoted 10 lines
tests/generic/365 new file +10
-0+ 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 + echo +}
Does this work for filesystems that don't have delalloc? Like fsdax filesystems?
--D
Quoted 49 lines
tests/generic/365 new file +13
-0+setup +echo -e "EXTSIZE = $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 +exittests/generic/365.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 365 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[0] 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 +[0] SCRATCH_MNT/new-file-01 +e flag unset + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +[0] SCRATCH_MNT/new-file-02 +e flag unset + +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
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Nirjhar RoyQuoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0
common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.
Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1
That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1")
}Quoted 10 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0
tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 4 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfs
Is this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Thanks,
Zorro
Quoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
Quoted 53 lines · Zorro Lang
Quoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }Quoted 9 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0
Quoted 8 lines
tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 5 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
If the generic helper goes in common/rc instead then it's not necessary at all.
--D
Quoted 43 lines
Thanks,
ZorroQuoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
Quoted 63 lines · Darrick J. Wong
Quoted 53 lines · Zorro Lang
Quoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }Quoted 9 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0Quoted 8 lines
tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 5 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Quoted 2 lines
If the generic helper goes in common/rc instead then it's not necessary at all.
Won't the "_source_specific_fs $FSTYP" in common/rc helps to import common/xfs?
Quoted 47 lines
--D
Quoted 43 lines
Thanks,
ZorroQuoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
Quoted 64 lines · Zorro Lang
Quoted 63 lines · Darrick J. Wong
Quoted 61 lines · Zorro Lang
Quoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }Quoted 10 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 4 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Quoted 4 lines
Quoted 2 lines
If the generic helper goes in common/rc instead then it's not necessary at all.
Won't the "
_source_specific_fs$FSTYP" in common/rc helps to import common/xfs?
Yeah, that too.
--D
Quoted 49 lines
Quoted 47 lines
--D
Quoted 43 lines
Thanks,
ZorroQuoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Quoted 49 lines · Darrick J. Wong
Quoted 48 lines · 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]/
Suggested-by Ojaswin Mujoo <[email protected]>Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/365 | 156 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/365.out | 26 +++++++ 2 files changed, 182 insertions(+)create mode 100755 tests/generic/365
create mode 100644 tests/generic/365.outtests/generic/365 new file +22
-0@@ -0,0 +1,156 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 365 +# +# 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 +. ./common/xfs + +_begin_fstest ioctl quick + +_supported_fs xfs + +_fixed_by_kernel_commit XXXXXXXXXXXX \
Quoted 8 lines
tests/generic/365 line 27
Quoted 5 lines
tests/generic/365 new file +5
-0+ "xfs: Check for delayed allocations before setting extsize", + +_require_scratch + +FILE_DATA_SIZE=1MDo these tests work correctly with fsblock size of 64k? Just curious since Pankaj just sent a series doing 1M -> 4M bumps to fix quota issues.
Yes I have tested this with 2k, 4k, 16k, 64k on ppc64le and x86_64
Quoted 5 lines
Quoted 5 lines
tests/generic/365 new file +5
-0+filter_extsz() +{ + sed "s/$EXTSIZE/EXTSIZE/g" +} +
Quoted 8 lines
tests/generic/365 line 39
Quoted 6 lines
tests/generic/365 new file +6
-0+setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 ))Might want to check that there isn't an extsize/cowextsize set on the root directory due to mkfs options.
Noted.
Quoted 107 lines
Quoted 94 lines
tests/generic/365 new file +93
-0+} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz +} + +check_extsz_and_xflag() +{ + local filename=$1 + read_file_extsize $filename + _test_xfs_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + _scratch_cycle_mount + check_extsz_and_xflag $filename +} + +# 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 +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + check_extsz_xflag_across_remount $filename + 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 + 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 + + check_extsz_xflag_across_remount $filename + 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 + + check_extsz_xflag_across_remount $filename + 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 + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation"tests/generic/365 line 142
Quoted 10 lines
tests/generic/365 new file +10
-0+ 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 + echo +}Does this work for filesystems that don't have delalloc? Like fsdax filesystems?
Quoted 1 line
--D
I haven't tested this on fsdax filesystem. Only tested on xfs with various bs.
Quoted 50 lines
Quoted 50 lines
tests/generic/365 new file +13
-0+setup +echo -e "EXTSIZE = $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 +exittests/generic/365.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 365 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[0] 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 +[0] SCRATCH_MNT/new-file-01 +e flag unset + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +[0] SCRATCH_MNT/new-file-02 +e flag unset + +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
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Zorro LangQuoted 21 lines · Zorro Lang
Quoted 20 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") }
Quoted 4 lines
common/xfs __generate_xfs_report_vars() { · line 16
Quoted 1 line
common/xfs +1
-0+# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
Noted.
Quoted 10 lines
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 7 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
Quoted 5 lines
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }
Yes. This looks better. Thank you for the suggestion.
Quoted 18 lines
Quoted 11 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 4 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Quoted 2 lines
Thanks,
Zorro
No, actually, it isn't. I have verified. Also, I am moving the helper function to common/rc, so this will not be required anyway.
Quoted 40 lines
Quoted 40 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Darrick J. WongQuoted 59 lines · Darrick J. Wong
Quoted 42 lines · Zorro Lang
Quoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }Quoted 16 lines
Quoted 10 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 4 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Quoted 4 lines
If the generic helper goes in common/rc instead then it's not necessary at all.
--D
Yeah, this makes sense.
Quoted 43 lines
Quoted 43 lines
Thanks,
ZorroQuoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Zorro LangQuoted 60 lines · Zorro Lang
Quoted 59 lines · Darrick J. Wong
Quoted 58 lines · Zorro Lang
Quoted 17 lines · Nirjhar Roy
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 the next patch for checking extsize (e) flag.
Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- common/xfs | 9 +++++++++ tests/xfs/207 | 14 +++----------- 2 files changed, 12 insertions(+), 11 deletions(-)common/xfs +0
-0common/xfs __generate_xfs_report_vars() { · line 16
Quoted 4 lines
common/xfs +1
-0@@ -13,6 +13,15 @@ __generate_xfs_report_vars() { REPORT_ENV_LIST_OPT+=("TEST_XFS_REPAIR_REBUILD" "TEST_XFS_SCRUB_REBUILD") } +# Check whether a fsxattr xflags character field is set on a given file.Better to explain the arguments, e.g.
# Check whether a fsxattr xflags character ($2) field is set on a given file ($1).
common/xfs __generate_xfs_report_vars() { · line 22
Quoted 6 lines
common/xfs +6
-0+# e.g. fsxattr.xflags = 0x0 [--------------C-] +# Returns 0 if passed flag character is set, otherwise returns 1 +_test_xfs_xflags_field() +{ + $XFS_IO_PROG -c "stat" "$1" | grep "fsxattr.xflags" | grep -q "\[.*$2.*\]" \ + && return 0 || return 1That's too complex. Those "return" aren't needed as Darrick metioned. About that two "grep", how about combine them, e.g.
_test_xfs_xflags_field(){ grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") }Quoted 10 lines
common/xfs +2
-0+} + _setup_large_xfs_fs() { fs_size=$1tests/xfs/207 +0
-0tests/xfs/207 _begin_fstest auto quick clone fiemap · line 18
Quoted 4 lines
tests/xfs/207 +1
-0@@ -15,21 +15,13 @@ _begin_fstest auto quick clone fiemap # Import common functions. . ./common/filter . ./common/reflink +. ./common/xfsIs this really necessary? Will this test fail without this line? The common/$FSTYP file is imported automatically, if it's not, that a bug.
Quoted 3 lines
Quoted 2 lines
If the generic helper goes in common/rc instead then it's not necessary at all.
Won't the "
_source_specific_fs$FSTYP" in common/rc helps to import common/xfs?
Yeah, it gets imported automatically(I have verified). Anyway, I am moving the helper function to common/rc, so this won't be required.
Quoted 45 lines
Quoted 45 lines
--D
Quoted 42 lines
Thanks,
ZorroQuoted 39 lines
tests/xfs/207 +2
-11_require_scratch_reflink _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 +57,14 @@ 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_xfs_xflags_field "$testdir/file3" "C" && 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_xfs_xflags_field "$testdir/file3" "C" && echo "C flag set" || echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0Signature · 1 line
2.43.5
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Quoted 58 lines · Darrick J. Wong
Quoted 43 lines · 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]/
Suggested-by Ojaswin Mujoo <[email protected]>Reviewed-by Ritesh Harjani (IBM) <[email protected]>Reviewed-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/365 | 156 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/365.out | 26 +++++++ 2 files changed, 182 insertions(+)create mode 100755 tests/generic/365
create mode 100644 tests/generic/365.outtests/generic/365 new file +17
-0@@ -0,0 +1,156 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 365 +# +# 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 +. ./common/xfs +tests/generic/365 line 27
Quoted 10 lines
tests/generic/365 new file +10
-0+_begin_fstest ioctl quick + +_supported_fs xfs + +_fixed_by_kernel_commit XXXXXXXXXXXX \ + "xfs: Check for delayed allocations before setting extsize", + +_require_scratch + +FILE_DATA_SIZE=1MDo these tests work correctly with fsblock size of 64k? Just curious since Pankaj just sent a series doing 1M -> 4M bumps to fix quota issues.
Quoted 13 lines
tests/generic/365 line 39
Quoted 11 lines
tests/generic/365 new file +11
-0+filter_extsz() +{ + sed "s/$EXTSIZE/EXTSIZE/g" +} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 ))Might want to check that there isn't an extsize/cowextsize set on the root directory due to mkfs options.
We have made some changes in the tests in v2 so that the test behaves as expected with preset extsize/extszinherit mkfs options on the root. Also tested with mkfs.xfs from [1]. I will send PATCH v2 shortly.
[1]. https://lore.kernel.org/all/[email protected]/
Quoted 158 lines
Quoted 94 lines
tests/generic/365 new file +93
-0+} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz +} + +check_extsz_and_xflag() +{ + local filename=$1 + read_file_extsize $filename + _test_xfs_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + _scratch_cycle_mount + check_extsz_and_xflag $filename +} + +# 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 +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + check_extsz_xflag_across_remount $filename + 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 + 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 + + check_extsz_xflag_across_remount $filename + 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 + + check_extsz_xflag_across_remount $filename + 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 + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation"tests/generic/365 line 142
Quoted 10 lines
tests/generic/365 new file +10
-0+ 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 + echo +}Does this work for filesystems that don't have delalloc? Like fsdax filesystems?
--D
Quoted 49 lines
tests/generic/365 new file +13
-0+setup +echo -e "EXTSIZE = $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 +exittests/generic/365.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 365 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[0] 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 +[0] SCRATCH_MNT/new-file-01 +e flag unset + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +[0] SCRATCH_MNT/new-file-02 +e flag unset + +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
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore