Addition of new tests for extsize hints
v2 cover Addition of new tests for extsize hints
This test checks the behaviour of xfs/ext4 filesystems when extsize hint is set on files with inode size as 0, non-empty
files with allocated and delalloc extents and so on.
This test set also tests one of the scenarios where setting extsize hints on files with delayed allocation (incorrectly) succeeds and there is a recent patch series[1] that got merged[2] which fixes this.
Currently this test only runs in xfs but there is a patch series[3] in review that adds support for the extsize hints for ext4 as well.
I have tested it on ppc64le (with page size 64k) and x86_64 (with page size 4k). The block sizes I have tested with are 2k, 4k, 8k, 16k, 32k, 64k with extsize being twice of the block size or 4 * block size(if the default extsize is 2*blocksize). I have also tested with configurations where I added extszinherit mkfs option on the root. I had to enable CONFIG_TRANSPARENT_HUGEPAGE to enable block size greater than 4k on x86_64.
Changes since [v1]
- Making the definition of _test_xfs_xflags_field() more compact and renamed _test_xfs_xflags_field() to _test_fsx_xflags_field() since I moved it to common/rc.
- Removed the explicit import of common/xfs from the test script.
- Renamed the test file from generic/365 to generic/366 since generic/365 was taken.
- Made the following functions in tests/generic/366 accept a second parameter (extsize)
a) check_extsz_xflag_across_remount()
b) check_extsz_and_xflag()
c) read_file_extsize()
d) filter_extsz
This will help filter out any extsize that the test wants. This is required because, this version no longer assumes any particular fixed value of the default extsize.
- Removed the check for xflags in test_data_allocated() and test_data_delayed() test
- Added an extra line in output that denotes that we are checking that the extent size hasn't changed if the xfs_io command fails (xflags in test_data_allocated() and test_data_delayed() test)
- Changing the new extsize to 4*blocksize if default extsize size is 2*blocksize
[1] https://lore.kernel.org/all/[email protected]/
[2] kernel commit - 2a492ff66673
[3] https://lore.kernel.org/linux-ext4/[email protected]/
[v1] https://lore.kernel.org/all/[email protected]/
Nirjhar Roy (2):
common/rc,xfs/207: Adding a common helper function to check xflag bits
on a given file
generic: Addition of new tests for extsize hints
common/rc | 7 ++ tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ tests/xfs/207 | 13 +--- 4 files changed, 207 insertions(+), 11 deletions(-) create mode 100755 tests/generic/366 create mode 100644 tests/generic/366.out
Signature · 1 line
2.43.5
PATCH v2 1/2 common/rc,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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)
common/rc +7 -0
@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field() +{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +} + # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"
tests/xfs/207 +2 -11
@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1
@@ -65,14 +56,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_fsx_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_fsx_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 v2 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+) create mode 100755 tests/generic/366 create mode 100644 tests/generic/366.out
tests/generic/366 new file +172 -0
@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# This test verifies that extent allocation hint setting works correctly on files with +# no extents allocated and non-empty files which are truncated. It also checks that the +# extent hints setting fails with non-empty file i.e, with any file with allocated +# extents or delayed allocation. We also check if the extsize value and the +# xflag bit actually got reflected after setting/re-setting the extsize value. + +. ./common/config +. ./common/filter +. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfs + +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratch + +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/' +} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g" +} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsize + [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exit
tests/generic/366.out new file +26 -0
@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +
Signature · 1 line
2.43.5
Re: [PATCH v2 1/2] common/rc,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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)common/rc +0
-0
common/rc _md5_checksum() · line 46
Quoted 6 lines
common/rc +3
-0@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field()
How about we call this "_test_fsxattr_xflag" instead?
fsx is already something else in fstests.
common/rc _md5_checksum() · line 49
Quoted 3 lines
common/rc +3
-0+{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +}
Not sure why this lost the xfs_io | grep -q structure. The return value of the whole expression will always be the return value of the last command in the pipeline.
(Correct? I hate bash...)
--D
Quoted 45 lines
common/rc +1
-0+ # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"tests/xfs/207 +2
-11@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1@@ -65,14 +56,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_fsx_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_fsx_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 v2 2/2] generic: Addition of new tests for extsize hints
In reply to Nirjhar RoyQuoted 35 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+) create mode 100755 tests/generic/366 create mode 100644 tests/generic/366.outtests/generic/366 new file +9
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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
tests/generic/366 line 19
Quoted 10 lines
tests/generic/366 new file +10
-0+# extents or delayed allocation. We also check if the extsize value and the +# xflag bit actually got reflected after setting/re-setting the extsize value. + +. ./common/config +. ./common/filter +. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfs
Aren't you all adding extsize support for ext4? I would've expected some kind of _require_extsize helper to _notrun on filesystems that don't support it.
Quoted 5 lines
tests/generic/366 new file +5
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratch
tests/generic/366 line 34
Quoted 10 lines
tests/generic/366 new file +10
-0+ +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'
Doesn't this need to check for extszinherit on $SCRATCH_MNT?
Quoted 5 lines
tests/generic/366 new file +5
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g"
tests/generic/366 line 49
Quoted 10 lines
tests/generic/366 new file +10
-0+} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsize
Er... why?
Quoted 4 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()
tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"
I almost asked in the last patch if the _test_fsxattr_flag function should be running xfs_io -c 'stat -v' so that you could grep for whole words instead of individual letters.
"extsize flag unset"
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Quoted 145 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5
Re: [PATCH v2 1/2] common/rc,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Darrick J. WongQuoted 29 lines · Darrick J. Wong
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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)common/rc +0
-0common/rc _md5_checksum() · line 46
Quoted 6 lines
common/rc +3
-0@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field()How about we call this "
_test_fsxattr_xflag" instead?fsx is already something else in fstests.
Quoted 8 lines
common/rc _md5_checksum() · line 49
Quoted 3 lines
common/rc +3
-0+{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +}Not sure why this lost the
xfs_io| grep -q structure. The return value of the whole expression will always be the return value of the last command in the pipeline.
I guess it was suggested here [1]
root-> grep -q "hello" <(echo "hello world"); echo $? 0
The cmd is not using the unnamed pipes ("|") any more. It's spawning the process () which does echo "hello world" and creating a named pipe or say temporary FD <() which is being read by grep now. So we still will have the correct return value. Slightly inefficitent compared to unnamed pipes though I agree.
Quoted 2 lines
(Correct? I hate bash...)
root-> ls -la <(echo "hello world");
lr-x------ 1 root root 64 Nov 16 00:42 /dev/fd/63 -> 'pipe:[74211850]'
Did I make you hate it more? ;)
-ritesh
Quoted 47 lines
--D
Quoted 45 lines
common/rc +1
-0+ # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"tests/xfs/207 +2
-11@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1@@ -65,14 +56,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_fsx_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_fsx_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 v2 2/2] generic: Addition of new tests for extsize hints
In reply to Darrick J. WongQuoted 41 lines · Darrick J. Wong
Quoted 40 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+)create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.outtests/generic/366 new file +14
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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
Quoted 8 lines
tests/generic/366 line 19
Quoted 5 lines
tests/generic/366 new file +5
-0+. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfsAren't you all adding extsize support for ext4? I would've expected some kind of
_require_extsizehelper to_notrunon filesystems that don't support it.
Yes, this is a good idea. I will try to have something like this. Thank you.
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratchtests/generic/366 line 34
Quoted 10 lines
tests/generic/366 new file +10
-0+ +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'Doesn't this need to check for extszinherit on $SCRATCH_MNT?
The above function tries to get the default extsize set on a directory ($SCRATCH_MNT for this test). Even if there is an extszinherit set or extsize (with -d extsize=<size> [1]), the function will get the extsize (in bytes) which is what the function intends to do. In case there is no extszinherit or extsize set on the directory, it will return 0. Does this answer your question, or are you asking something else?
[1] https://lore.kernel.org/all/[email protected]/
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g"tests/generic/366 line 49
Quoted 10 lines
tests/generic/366 new file +10
-0+} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsizeEr... why?
The test behaves a bit differently when the new and old extsizes are equal and the intention of this test is to check if the kernel behaves as expected
when we are trying to *change* the extsize. Two of the sub-tests (test_data_delayed(), test_data_allocated()) test whether extsize settting fails if there are allocated extents or delayed allocation. The failure doesn't take place when the new and the default extsizes are equal, i.e, when the extsize is not changing. If the default and the new extsize are equal, the xfs_io command succeeds, which is not what we want the test to do. So we are always ensuring that the new extsize is not equal to the default extsize. Does this answer your question?
Quoted 21 lines
Quoted 5 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"I almost asked in the last patch if the
_test_fsxattr_flagfunction should be runningxfs_io-c 'stat -v' so that you could grep for whole words instead of individual letters."extsize flag unset"
Quoted 7 lines
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Yes, that makes sense. So do you mean something like the following?
# Check whether a fsxattr xflags name ($2) field is set on a given file ($1). # e.g, fsxattr.xflags = 0x80000800 [extsize, has-xattr] _test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") }
and the call sites can be like
_test_fsx_xflags_field $filename "extsize" && echo "e flag set" || echo "e flag unset"
THE OTHER OPTION IS:
We can embed the "<flag name> flag set/unset" message, inside the _test_fsx_xflags_field() function. Something like
_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") && echo "$2 flag set" || echo "$2 flag unset" }
Which one do you prefer?
Quoted 146 lines
Quoted 146 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH v2 1/2] common/rc,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Darrick J. WongQuoted 21 lines · Darrick J. Wong
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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)common/rc +0
-0@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 }
Quoted 6 lines
common/rc _md5_checksum() · line 46
Quoted 3 lines
common/rc +3
-0+# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field()How about we call this "
_test_fsxattr_xflag" instead?fsx is already something else in fstests.
Noted.
Quoted 57 lines
common/rc _md5_checksum() · line 49
Quoted 4 lines
common/rc +3
-0+{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +}Not sure why this lost the
xfs_io| grep -q structure. The return value of the whole expression will always be the return value of the last command in the pipeline.(Correct? I hate bash...)
--D
Quoted 45 lines
common/rc +1
-0+ # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"tests/xfs/207 +2
-11@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1@@ -65,14 +56,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_fsx_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_fsx_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 v2 2/2] generic: Addition of new tests for extsize hints
In reply to Nirjhar RoyQuoted 77 lines · Nirjhar Roy
Quoted 41 lines · Darrick J. Wong
Quoted 40 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+)create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.outtests/generic/366 new file +14
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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/filterQuoted 8 lines
tests/generic/366 line 19
Quoted 5 lines
tests/generic/366 new file +5
-0+. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfsAren't you all adding extsize support for ext4? I would've expected some kind of
_require_extsizehelper to_notrunon filesystems that don't support it.Yes, this is a good idea. I will try to have something like this. Thank you.
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratchtests/generic/366 line 34
Quoted 10 lines
tests/generic/366 new file +10
-0+ +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'Doesn't this need to check for extszinherit on $SCRATCH_MNT?
The above function tries to get the default extsize set on a directory ($SCRATCH_MNT for this test). Even if there is an extszinherit set or extsize (with -d extsize=<size> [1]), the function will get the extsize (in bytes) which is what the function intends to do. In case there is no extszinherit or extsize set on the directory, it will return 0. Does this answer your question, or are you asking something else?
Quoted 2 lines
Nah, I think I got confused there. Disregard the question. :(
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g"tests/generic/366 line 49
Quoted 10 lines
tests/generic/366 new file +10
-0+} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsizeEr... why?
Quoted 10 lines
The test behaves a bit differently when the new and old extsizes are equal and the intention of this test is to check if the kernel behaves as expected when we are trying to *change* the extsize. Two of the sub-tests (
test_data_delayed(),test_data_allocated()) test whether extsize settting fails if there are allocated extents or delayed allocation. The failure doesn't take place when the new and the default extsizes are equal, i.e, when the extsize is not changing. If the default and the new extsize are equal, thexfs_iocommand succeeds, which is not what we want the test to do. So we are always ensuring that the new extsize is not equal to the default extsize. Does this answer your question?
Yep. Can you add that ("Make sure the new extsize is not the same as the default extsize so that we can observe it changing") to the comment?
Quoted 48 lines
Quoted 20 lines
Quoted 4 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"I almost asked in the last patch if the
_test_fsxattr_flagfunction should be runningxfs_io-c 'stat -v' so that you could grep for whole words instead of individual letters."extsize flag unset"
Quoted 7 lines
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Yes, that makes sense. So do you mean something like the following?
# Check whether a fsxattr xflags name ($2) field is set on a given file ($1).
# e.g, fsxattr.xflags =0x80000800[extsize, has-xattr]_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") }and the call sites can be like
_test_fsx_xflags_field$filename "extsize" && echo "e flag set" || echo "e flag unset"THE OTHER OPTION IS:
We can embed the "<flag name> flag set/unset" message, inside the
_test_fsx_xflags_field()function. Something like
Quoted 7 lines
_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") && echo "$2 flag set" || echo "$2 flag unset"
}Which one do you prefer?
You might as well go for this second form since that's how all the callers behave.
--D
Quoted 153 lines
Quoted 146 lines
Quoted 146 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH v2 1/2] common/rc,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Ritesh Harjani (IBM)Quoted 40 lines · Ritesh Harjani
Quoted 29 lines · Darrick J. Wong
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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)common/rc +0
-0common/rc _md5_checksum() · line 46
Quoted 6 lines
common/rc +3
-0@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field()How about we call this "
_test_fsxattr_xflag" instead?fsx is already something else in fstests.
Quoted 8 lines
common/rc _md5_checksum() · line 49
Quoted 3 lines
common/rc +3
-0+{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +}Not sure why this lost the
xfs_io| grep -q structure. The return value of the whole expression will always be the return value of the last command in the pipeline.
Quoted 3 lines
I guess it was suggested here [1]
Ah.
Quoted 8 lines
root-> grep -q "hello" <(echo "hello world"); echo $? 0
The cmd is not using the unnamed pipes ("|") any more. It's spawning the process () which does echo "hello world" and creating a named pipe or say temporary FD <() which is being read by grep now. So we still will have the correct return value. Slightly inefficitent compared to unnamed pipes though I agree.
Well... it's subtle, being bash, right? :)
bash creates a pipe and a subprocess for the "echo hello world", then hooks its stdout to the pipe, just like a regular "|" pipe.
But for "grep -q hello" things are different -- for the grep process, the pipe is added as a new fd (e.g. /dev/fd/63), and then that path is provided on the command line. So what bash is doing is:
grep -q "hello" /dev/fd/63
AFAICT for grep this makes no difference unless you want it to tell you filenames:
$ grep -l hello <(echo hello world) /dev/fd/63 $ echo hello world | grep -l hello (standard input)
and I'm sure there's other weird implications that I'm not remembering.
Quoted 7 lines
Quoted 2 lines
(Correct? I hate bash...)
root-> ls -la <(echo "hello world");
lr-x------ 1 root root 64 Nov 16 00:42/dev/fd/63-> 'pipe:[74211850]'Did I make you hate it more? ;)
Yep!
--D
Quoted 51 lines
-ritesh
Quoted 47 lines
--D
Quoted 45 lines
common/rc +1
-0+ # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"tests/xfs/207 +2
-11@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1@@ -65,14 +56,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_fsx_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_fsx_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 v2 2/2] generic: Addition of new tests for extsize hints
In reply to Darrick J. WongQuoted 168 lines · Darrick J. Wong
Quoted 77 lines · Nirjhar Roy
Quoted 41 lines · Darrick J. Wong
Quoted 40 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+)create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.outtests/generic/366 new file +14
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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/filterQuoted 8 lines
tests/generic/366 line 19
Quoted 5 lines
tests/generic/366 new file +5
-0+. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfsAren't you all adding extsize support for ext4? I would've expected some kind of
_require_extsizehelper to_notrunon filesystems that don't support it.Yes, this is a good idea. I will try to have something like this. Thank you.
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratchtests/generic/366 line 34
Quoted 10 lines
tests/generic/366 new file +10
-0+ +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'Doesn't this need to check for extszinherit on $SCRATCH_MNT?
The above function tries to get the default extsize set on a directory ($SCRATCH_MNT for this test). Even if there is an extszinherit set or extsize (with -d extsize=<size> [1]), the function will get the extsize (in bytes) which is what the function intends to do. In case there is no extszinherit or extsize set on the directory, it will return 0. Does this answer your question, or are you asking something else?
Quoted 2 lines
Nah, I think I got confused there. Disregard the question. :(
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g"tests/generic/366 line 49
Quoted 10 lines
tests/generic/366 new file +10
-0+} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsizeEr... why?
Quoted 10 lines
The test behaves a bit differently when the new and old extsizes are equal and the intention of this test is to check if the kernel behaves as expected when we are trying to *change* the extsize. Two of the sub-tests (
test_data_delayed(),test_data_allocated()) test whether extsize settting fails if there are allocated extents or delayed allocation. The failure doesn't take place when the new and the default extsizes are equal, i.e, when the extsize is not changing. If the default and the new extsize are equal, thexfs_iocommand succeeds, which is not what we want the test to do. So we are always ensuring that the new extsize is not equal to the default extsize. Does this answer your question?Yep. Can you add that ("Make sure the new extsize is not the same as the default extsize so that we can observe it changing") to the comment?
Quoted 54 lines
Quoted 20 lines
Quoted 4 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"I almost asked in the last patch if the
_test_fsxattr_flagfunction should be runningxfs_io-c 'stat -v' so that you could grep for whole words instead of individual letters."extsize flag unset"
Quoted 7 lines
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Yes, that makes sense. So do you mean something like the following?
# Check whether a fsxattr xflags name ($2) field is set on a given file ($1).
# e.g, fsxattr.xflags =0x80000800[extsize, has-xattr]_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") }and the call sites can be like
_test_fsx_xflags_field$filename "extsize" && echo "e flag set" || echo "e flag unset"THE OTHER OPTION IS:
We can embed the "<flag name> flag set/unset" message, inside the
_test_fsx_xflags_field()function. Something like
_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") && echo "$2 flag set" || echo "$2 flag unset"
}
Quoted 5 lines
Quoted 1 line
Which one do you prefer?
You might as well go for this second form since that's how all the callers behave.
Sorry, I missed reading the rest of the email from Nirjhar or else I would have responded earlier.
Adding an echo command in the common helper routine for it to go into the .out file might become confusing. So if folks don't have a hard preference here, then can we please keep the same previous logic of returning success or failure from ("_test_fsxattr_xflag" common helper) and let the caller decide whatever string it wants to add (or even not) for it's .out file please?
-ritesh
Quoted 155 lines
--D
Quoted 153 lines
Quoted 146 lines
Quoted 146 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH v2 1/2] common/rc,xfs/207: Adding a common helper function to check xflag bits on a given file
In reply to Darrick J. WongQuoted 65 lines · Darrick J. Wong
Quoted 40 lines · Ritesh Harjani
Quoted 29 lines · Darrick J. Wong
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/rc | 7 +++++++ tests/xfs/207 | 13 ++----------- 2 files changed, 9 insertions(+), 11 deletions(-)common/rc +0
-0common/rc _md5_checksum() · line 46
Quoted 6 lines
common/rc +3
-0@@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags character ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [----------e-----X] +_test_fsx_xflags_field()How about we call this "
_test_fsxattr_xflag" instead?fsx is already something else in fstests.
Quoted 8 lines
common/rc _md5_checksum() · line 49
Quoted 3 lines
common/rc +3
-0+{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat" "$1") +}Not sure why this lost the
xfs_io| grep -q structure. The return value of the whole expression will always be the return value of the last command in the pipeline.Quoted 3 lines
I guess it was suggested here [1]
Ah.
Quoted 8 lines
root-> grep -q "hello" <(echo "hello world"); echo $? 0
The cmd is not using the unnamed pipes ("|") any more. It's spawning the process () which does echo "hello world" and creating a named pipe or say temporary FD <() which is being read by grep now. So we still will have the correct return value. Slightly inefficitent compared to unnamed pipes though I agree.
Well... it's subtle, being bash, right? :)
bash creates a pipe and a subprocess for the "echo hello world", then hooks its stdout to the pipe, just like a regular "|" pipe.
But for "grep -q hello" things are different -- for the grep process, the pipe is added as a new fd (e.g.
/dev/fd/63), and then that path is provided on the command line. So what bash is doing is:
Quoted 7 lines
grep -q "hello" /dev/fd/63AFAICT for grep this makes no difference unless you want it to tell you filenames:
$ grep -l hello <(echo hello world) /dev/fd/63
aah yes, I see (from strace)
pipe2([3, 4], 0) = 0 fcntl(63, F_GETFD) = 0 fcntl(62, F_GETFD) = -1 EBADF (Bad file descriptor) dup2(3, 62) = 62 close(3) = 0
Thanks!
-ritesh
Quoted 68 lines
$ echo hello world | grep -l hello (standard input)and I'm sure there's other weird implications that I'm not remembering.
Quoted 7 lines
Quoted 2 lines
(Correct? I hate bash...)
root-> ls -la <(echo "hello world");
lr-x------ 1 root root 64 Nov 16 00:42/dev/fd/63-> 'pipe:[74211850]'Did I make you hate it more? ;)
Yep!
--D
Quoted 51 lines
-ritesh
Quoted 47 lines
--D
Quoted 45 lines
common/rc +1
-0+ # Write a byte into a range of a file _pwrite_byte() { local pattern="$1"tests/xfs/207 +2
-11@@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1@@ -65,14 +56,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_fsx_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_fsx_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 v2 2/2] generic: Addition of new tests for extsize hints
In reply to Ritesh Harjani (IBM)Quoted 179 lines · Ritesh Harjani
Quoted 168 lines · Darrick J. Wong
Quoted 77 lines · Nirjhar Roy
Quoted 41 lines · Darrick J. Wong
Quoted 40 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+)create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.outtests/generic/366 new file +14
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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/filterQuoted 8 lines
tests/generic/366 line 19
Quoted 5 lines
tests/generic/366 new file +5
-0+. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfsAren't you all adding extsize support for ext4? I would've expected some kind of
_require_extsizehelper to_notrunon filesystems that don't support it.Yes, this is a good idea. I will try to have something like this. Thank you.
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratchtests/generic/366 line 34
Quoted 10 lines
tests/generic/366 new file +10
-0+ +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'Doesn't this need to check for extszinherit on $SCRATCH_MNT?
The above function tries to get the default extsize set on a directory ($SCRATCH_MNT for this test). Even if there is an extszinherit set or extsize (with -d extsize=<size> [1]), the function will get the extsize (in bytes) which is what the function intends to do. In case there is no extszinherit or extsize set on the directory, it will return 0. Does this answer your question, or are you asking something else?
Quoted 2 lines
Nah, I think I got confused there. Disregard the question. :(
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
tests/generic/366 new file +5
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g"tests/generic/366 line 49
Quoted 10 lines
tests/generic/366 new file +10
-0+} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsizeEr... why?
Quoted 10 lines
The test behaves a bit differently when the new and old extsizes are equal and the intention of this test is to check if the kernel behaves as expected when we are trying to *change* the extsize. Two of the sub-tests (
test_data_delayed(),test_data_allocated()) test whether extsize settting fails if there are allocated extents or delayed allocation. The failure doesn't take place when the new and the default extsizes are equal, i.e, when the extsize is not changing. If the default and the new extsize are equal, thexfs_iocommand succeeds, which is not what we want the test to do. So we are always ensuring that the new extsize is not equal to the default extsize. Does this answer your question?Yep. Can you add that ("Make sure the new extsize is not the same as the default extsize so that we can observe it changing") to the comment?
Quoted 54 lines
Quoted 20 lines
Quoted 4 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"I almost asked in the last patch if the
_test_fsxattr_flagfunction should be runningxfs_io-c 'stat -v' so that you could grep for whole words instead of individual letters."extsize flag unset"
Quoted 7 lines
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Yes, that makes sense. So do you mean something like the following?
# Check whether a fsxattr xflags name ($2) field is set on a given file ($1).
# e.g, fsxattr.xflags =0x80000800[extsize, has-xattr]_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") }and the call sites can be like
_test_fsx_xflags_field$filename "extsize" && echo "e flag set" || echo "e flag unset"THE OTHER OPTION IS:
We can embed the "<flag name> flag set/unset" message, inside the
_test_fsx_xflags_field()function. Something like
_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") && echo "$2 flag set" || echo "$2 flag unset"
}Quoted 5 lines
Quoted 1 line
Which one do you prefer?
You might as well go for this second form since that's how all the callers behave.
Sorry, I missed reading the rest of the email from Nirjhar or else I would have responded earlier.
Quoted 6 lines
Adding an echo command in the common helper routine for it to go into the .out file might become confusing. So if folks don't have a hard preference here, then can we please keep the same previous logic of returning success or failure from ("
_test_fsxattr_xflag" common helper) and let the caller decide whatever string it wants to add (or even not) for it's .out file please?
That's fine with me. :)
--D
Quoted 158 lines
-ritesh
Quoted 155 lines
--D
Quoted 153 lines
Quoted 146 lines
Quoted 146 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Re: [PATCH v2 2/2] generic: Addition of new tests for extsize hints
In reply to Darrick J. WongQuoted 105 lines · Darrick J. Wong
Quoted 74 lines · Nirjhar Roy
Quoted 41 lines · Darrick J. Wong
Quoted 40 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]/
Reviewed-by Ritesh Harjani (IBM) <[email protected]>
Reviewed-by Ojaswin Mujoo <[email protected]>Suggested-by Ojaswin Mujoo <[email protected]>Signed-off-by Nirjhar Roy <[email protected]>--- tests/generic/366 | 172 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 198 insertions(+)create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.outtests/generic/366 new file +14
-0@@ -0,0 +1,172 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy ([email protected]). All Rights Reserved. +# +# FS QA Test 366 +# +# 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/filterQuoted 8 lines
tests/generic/366 line 19
Quoted 5 lines
tests/generic/366 new file +5
-0+. ./common/preamble + +_begin_fstest ioctl quick + +_supported_fs xfsAren't you all adding extsize support for ext4? I would've expected some kind of
_require_extsizehelper to_notrunon filesystems that don't support it.Yes, this is a good idea. I will try to have something like this. Thank you.
Quoted 8 lines
Quoted 8 lines
tests/generic/366 new file +8
-0+ +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting extsize" + +_require_scratch + +FILE_DATA_SIZE=1M +Quoted 8 lines
tests/generic/366 line 34
Quoted 7 lines
tests/generic/366 new file +7
-0+get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/'Doesn't this need to check for extszinherit on $SCRATCH_MNT?
The above function tries to get the default extsize set on a directory ($SCRATCH_MNT for this test). Even if there is an extszinherit set or extsize (with -d extsize=<size> [1]), the function will get the extsize (in bytes) which is what the function intends to do. In case there is no extszinherit or extsize set on the directory, it will return 0. Does this answer your question, or are you asking something else?
Quoted 2 lines
Nah, I think I got confused there. Disregard the question. :(
Quoted 16 lines
Quoted 8 lines
Quoted 8 lines
tests/generic/366 new file +8
-0+} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g" +} + +setup()Quoted 8 lines
tests/generic/366 line 49
Quoted 7 lines
tests/generic/366 new file +7
-0+{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Making sure the new extsize is not the same as the default extsizeEr... why?
Quoted 10 lines
The test behaves a bit differently when the new and old extsizes are equal and the intention of this test is to check if the kernel behaves as expected when we are trying to *change* the extsize. Two of the sub-tests (
test_data_delayed(),test_data_allocated()) test whether extsize settting fails if there are allocated extents or delayed allocation. The failure doesn't take place when the new and the default extsizes are equal, i.e, when the extsize is not changing. If the default and the new extsize are equal, thexfs_iocommand succeeds, which is not what we want the test to do. So we are always ensuring that the new extsize is not equal to the default extsize. Does this answer your question?
Quoted 2 lines
Yep. Can you add that ("Make sure the new extsize is not the same as the default extsize so that we can observe it changing") to the comment?
Yes. I can modify the comment to make it more clear.
--NR
Quoted 212 lines
Quoted 48 lines
Quoted 21 lines
Quoted 5 lines
tests/generic/366 new file +4
-0+ [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize()tests/generic/366 line 63
Quoted 10 lines
tests/generic/366 new file +10
-0+{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsx_xflags_field $filename "e" && echo "e flag set" || echo "e flag unset"I almost asked in the last patch if the
_test_fsxattr_flagfunction should be runningxfs_io-c 'stat -v' so that you could grep for whole words instead of individual letters."extsize flag unset"
Quoted 7 lines
"cowextsize flag set"
is a bit easier to figure out what's going wrong.
The rest of the logic looks reasonable to me.
--D
Yes, that makes sense. So do you mean something like the following?
# Check whether a fsxattr xflags name ($2) field is set on a given file ($1).
# e.g, fsxattr.xflags =0x80000800[extsize, has-xattr]_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
}and the call sites can be like
_test_fsx_xflags_field$filename "extsize" && echo "e flag set" || echo "e flag unset"THE OTHER OPTION IS:
We can embed the "<flag name> flag set/unset" message, inside the
_test_fsx_xflags_field()function. Something likeQuoted 7 lines
_test_fsxattr_flag_field()
{
grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1")
&& echo "$2 flag set" || echo "$2 flag unset"
}Which one do you prefer?
You might as well go for this second form since that's how all the callers behave.
--D
Quoted 152 lines
Quoted 145 lines
Quoted 145 lines
tests/generic/366 new file +109
-0+} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exittests/generic/366.out new file +26
-0@@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set +Signature · 1 line
2.43.5Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore
Signature · 4 lines
--- Nirjhar Roy Linux Kernel Developer IBM, Bangalore