[PATCH 0/2] generic: Addition of new tests for extsize hints

[PATCH 0/2] generic: Addition of new tests for extsize hints

[Nirjhar Roy]

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/20241015094509.678082-1-ojaswin@linux.ibm.com/
[2] https://lore.kernel.org/linux-ext4/cover.1726034272.git.ojaswin@linux.ibm.com/

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

-- 
2.43.5

[PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[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) <ritesh.list@gmail.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
---
 common/xfs    |  9 +++++++++
 tests/xfs/207 | 14 +++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/common/xfs b/common/xfs
index 62e3100e..7340ccbf 100644
--- a/common/xfs
+++ b/common/xfs
@@ -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
diff --git a/tests/xfs/207 b/tests/xfs/207
index bbe21307..adb925df 100755
--- a/tests/xfs/207
+++ b/tests/xfs/207
@@ -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
-- 
2.43.5

[PATCH 2/2] generic: Addition of new tests for extsize hints

[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/cover.1726034272.git.ojaswin@linux.ibm.com/

Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
---
 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

diff --git a/tests/generic/365 b/tests/generic/365
new file mode 100755
index 00000000..85a7ce9a
--- /dev/null
+++ b/tests/generic/365
@@ -0,0 +1,156 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com).  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
diff --git a/tests/generic/365.out b/tests/generic/365.out
new file mode 100644
index 00000000..38cd0885
--- /dev/null
+++ b/tests/generic/365.out
@@ -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
+
-- 
2.43.5

Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[Darrick J. Wong]

On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
> 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) <ritesh.list@gmail.com>
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> ---
>  common/xfs    |  9 +++++++++
>  tests/xfs/207 | 14 +++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 62e3100e..7340ccbf 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -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.

> +{
> +    $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

> +}
> +
>  _setup_large_xfs_fs()
>  {
>  	fs_size=$1
> diff --git a/tests/xfs/207 b/tests/xfs/207
> index bbe21307..adb925df 100755
> --- a/tests/xfs/207
> +++ b/tests/xfs/207
> @@ -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
> -- 
> 2.43.5
> 
> 

Re: [PATCH 2/2] generic: Addition of new tests for extsize hints

[Darrick J. Wong]

On Wed, Oct 23, 2024 at 12:56:20AM +0530, Nirjhar Roy wrote:
> 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/cover.1726034272.git.ojaswin@linux.ibm.com/
> 
> Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> ---
>  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
> 
> diff --git a/tests/generic/365 b/tests/generic/365
> new file mode 100755
> index 00000000..85a7ce9a
> --- /dev/null
> +++ b/tests/generic/365
> @@ -0,0 +1,156 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com).  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

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.

> +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.

> +}
> +
> +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
> +}

Does this work for filesystems that don't have delalloc?  Like fsdax
filesystems?

--D

> +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
> diff --git a/tests/generic/365.out b/tests/generic/365.out
> new file mode 100644
> index 00000000..38cd0885
> --- /dev/null
> +++ b/tests/generic/365.out
> @@ -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
> +
> -- 
> 2.43.5
> 
> 

Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[Zorro Lang]

On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
> 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) <ritesh.list@gmail.com>
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> ---
>  common/xfs    |  9 +++++++++
>  tests/xfs/207 | 14 +++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 62e3100e..7340ccbf 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -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).

> +# 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")
}



> +}
> +
>  _setup_large_xfs_fs()
>  {
>  	fs_size=$1
> diff --git a/tests/xfs/207 b/tests/xfs/207
> index bbe21307..adb925df 100755
> --- a/tests/xfs/207
> +++ b/tests/xfs/207
> @@ -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

>  
>  _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
> -- 
> 2.43.5
> 
> 

Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[Darrick J. Wong]

On Fri, Oct 25, 2024 at 10:56:51AM +0800, Zorro Lang wrote:
> On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
> > 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) <ritesh.list@gmail.com>
> > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> > ---
> >  common/xfs    |  9 +++++++++
> >  tests/xfs/207 | 14 +++-----------
> >  2 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/common/xfs b/common/xfs
> > index 62e3100e..7340ccbf 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -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).
> 
> > +# 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")
> }
> 
> 
> 
> > +}
> > +
> >  _setup_large_xfs_fs()
> >  {
> >  	fs_size=$1
> > diff --git a/tests/xfs/207 b/tests/xfs/207
> > index bbe21307..adb925df 100755
> > --- a/tests/xfs/207
> > +++ b/tests/xfs/207
> > @@ -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.

If the generic helper goes in common/rc instead then it's not necessary
at all.

--D

> Thanks,
> Zorro
> 
> >  
> >  _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
> > -- 
> > 2.43.5
> > 
> > 
> 

Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[Zorro Lang]

On Thu, Oct 24, 2024 at 09:07:03PM -0700, Darrick J. Wong wrote:
> On Fri, Oct 25, 2024 at 10:56:51AM +0800, Zorro Lang wrote:
> > On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
> > > 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) <ritesh.list@gmail.com>
> > > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> > > ---
> > >  common/xfs    |  9 +++++++++
> > >  tests/xfs/207 | 14 +++-----------
> > >  2 files changed, 12 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/common/xfs b/common/xfs
> > > index 62e3100e..7340ccbf 100644
> > > --- a/common/xfs
> > > +++ b/common/xfs
> > > @@ -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).
> > 
> > > +# 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")
> > }
> > 
> > 
> > 
> > > +}
> > > +
> > >  _setup_large_xfs_fs()
> > >  {
> > >  	fs_size=$1
> > > diff --git a/tests/xfs/207 b/tests/xfs/207
> > > index bbe21307..adb925df 100755
> > > --- a/tests/xfs/207
> > > +++ b/tests/xfs/207
> > > @@ -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.
> 
> 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?

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > >  
> > >  _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
> > > -- 
> > > 2.43.5
> > > 
> > > 
> > 
> 

Re: [PATCH 1/2] common/xfs,xfs/207: Adding a common helper function to check xflag bits on a given file

[Darrick J. Wong]

On Fri, Oct 25, 2024 at 12:15:01PM +0800, Zorro Lang wrote:
> On Thu, Oct 24, 2024 at 09:07:03PM -0700, Darrick J. Wong wrote:
> > On Fri, Oct 25, 2024 at 10:56:51AM +0800, Zorro Lang wrote:
> > > On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
> > > > 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) <ritesh.list@gmail.com>
> > > > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
> > > > ---
> > > >  common/xfs    |  9 +++++++++
> > > >  tests/xfs/207 | 14 +++-----------
> > > >  2 files changed, 12 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/common/xfs b/common/xfs
> > > > index 62e3100e..7340ccbf 100644
> > > > --- a/common/xfs
> > > > +++ b/common/xfs
> > > > @@ -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).
> > > 
> > > > +# 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")
> > > }
> > > 
> > > 
> > > 
> > > > +}
> > > > +
> > > >  _setup_large_xfs_fs()
> > > >  {
> > > >  	fs_size=$1
> > > > diff --git a/tests/xfs/207 b/tests/xfs/207
> > > > index bbe21307..adb925df 100755
> > > > --- a/tests/xfs/207
> > > > +++ b/tests/xfs/207
> > > > @@ -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.
> > 
> > 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

> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > >  
> > > >  _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
> > > > -- 
> > > > 2.43.5
> > > > 
> > > > 
> > > 
> > 
> 
> 

Re: [PATCH 2/2] generic: Addition of new tests for extsize hints

[Nirjhar Roy]

On 10/24/24 23:44, Darrick J. Wong wrote:
> On Wed, Oct 23, 2024 at 12:56:20AM +0530, Nirjhar Roy wrote:
>> 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/cover.1726034272.git.ojaswin@linux.ibm.com/
>>
>> Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
>> ---
>>   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
>>
>> diff --git a/tests/generic/365 b/tests/generic/365
>> new file mode 100755
>> index 00000000..85a7ce9a
>> --- /dev/null
>> +++ b/tests/generic/365
>> @@ -0,0 +1,156 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com).  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
> 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.
Yes I have tested this with 2k, 4k, 16k, 64k on ppc64le and x86_64
>> +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.
Noted.
>
>> +}
>> +
>> +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
>> +}
> Does this work for filesystems that don't have delalloc?  Like fsdax
> filesystems?
>
> --D
I haven't tested this on fsdax filesystem. Only tested on xfs with 
various bs.
>
>> +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
>> diff --git a/tests/generic/365.out b/tests/generic/365.out
>> new file mode 100644
>> index 00000000..38cd0885
>> --- /dev/null
>> +++ b/tests/generic/365.out
>> @@ -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
>> +
>> -- 
>> 2.43.5
>>
>>
-- 
---
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

[Nirjhar Roy]

On 10/25/24 08:26, Zorro Lang wrote:
> On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
>> 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) <ritesh.list@gmail.com>
>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
>> ---
>>   common/xfs    |  9 +++++++++
>>   tests/xfs/207 | 14 +++-----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/common/xfs b/common/xfs
>> index 62e3100e..7340ccbf 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -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).
Noted.
>
>> +# 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")
> }
>
Yes. This looks better. Thank you for the suggestion.
>
>> +}
>> +
>>   _setup_large_xfs_fs()
>>   {
>>   	fs_size=$1
>> diff --git a/tests/xfs/207 b/tests/xfs/207
>> index bbe21307..adb925df 100755
>> --- a/tests/xfs/207
>> +++ b/tests/xfs/207
>> @@ -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
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.
>
>>   
>>   _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
>> -- 
>> 2.43.5
>>
>>
-- 
---
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

[Nirjhar Roy]

On 10/25/24 09:37, Darrick J. Wong wrote:
> On Fri, Oct 25, 2024 at 10:56:51AM +0800, Zorro Lang wrote:
>> On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
>>> 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) <ritesh.list@gmail.com>
>>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
>>> ---
>>>   common/xfs    |  9 +++++++++
>>>   tests/xfs/207 | 14 +++-----------
>>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/common/xfs b/common/xfs
>>> index 62e3100e..7340ccbf 100644
>>> --- a/common/xfs
>>> +++ b/common/xfs
>>> @@ -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).
>>
>>> +# 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")
>> }
>>
>>
>>
>>> +}
>>> +
>>>   _setup_large_xfs_fs()
>>>   {
>>>   	fs_size=$1
>>> diff --git a/tests/xfs/207 b/tests/xfs/207
>>> index bbe21307..adb925df 100755
>>> --- a/tests/xfs/207
>>> +++ b/tests/xfs/207
>>> @@ -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.
> If the generic helper goes in common/rc instead then it's not necessary
> at all.
>
> --D
Yeah, this makes sense.
>
>> Thanks,
>> Zorro
>>
>>>   
>>>   _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
>>> -- 
>>> 2.43.5
>>>
>>>
-- 
---
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

[Nirjhar Roy]

On 10/25/24 09:45, Zorro Lang wrote:
> On Thu, Oct 24, 2024 at 09:07:03PM -0700, Darrick J. Wong wrote:
>> On Fri, Oct 25, 2024 at 10:56:51AM +0800, Zorro Lang wrote:
>>> On Wed, Oct 23, 2024 at 12:56:19AM +0530, Nirjhar Roy wrote:
>>>> 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) <ritesh.list@gmail.com>
>>>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>>>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
>>>> ---
>>>>   common/xfs    |  9 +++++++++
>>>>   tests/xfs/207 | 14 +++-----------
>>>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/common/xfs b/common/xfs
>>>> index 62e3100e..7340ccbf 100644
>>>> --- a/common/xfs
>>>> +++ b/common/xfs
>>>> @@ -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).
>>>
>>>> +# 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")
>>> }
>>>
>>>
>>>
>>>> +}
>>>> +
>>>>   _setup_large_xfs_fs()
>>>>   {
>>>>   	fs_size=$1
>>>> diff --git a/tests/xfs/207 b/tests/xfs/207
>>>> index bbe21307..adb925df 100755
>>>> --- a/tests/xfs/207
>>>> +++ b/tests/xfs/207
>>>> @@ -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.
>> 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.
>
>> --D
>>
>>> Thanks,
>>> Zorro
>>>
>>>>   
>>>>   _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
>>>> -- 
>>>> 2.43.5
>>>>
>>>>
-- 
---
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore

Re: [PATCH 2/2] generic: Addition of new tests for extsize hints

[Nirjhar Roy]

On 10/24/24 23:44, Darrick J. Wong wrote:
> On Wed, Oct 23, 2024 at 12:56:20AM +0530, Nirjhar Roy wrote:
>> 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/cover.1726034272.git.ojaswin@linux.ibm.com/
>>
>> Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com>
>> ---
>>   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
>>
>> diff --git a/tests/generic/365 b/tests/generic/365
>> new file mode 100755
>> index 00000000..85a7ce9a
>> --- /dev/null
>> +++ b/tests/generic/365
>> @@ -0,0 +1,156 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com).  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
> 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.
>
>> +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/20230929095342.2976587-7-john.g.garry@oracle.com/

>
>> +}
>> +
>> +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
>> +}
> Does this work for filesystems that don't have delalloc?  Like fsdax
> filesystems?
>
> --D
>
>> +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
>> diff --git a/tests/generic/365.out b/tests/generic/365.out
>> new file mode 100644
>> index 00000000..38cd0885
>> --- /dev/null
>> +++ b/tests/generic/365.out
>> @@ -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
>> +
>> -- 
>> 2.43.5
>>
>>
-- 
---
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore