[PATCHv2 0/9] xfstests: 64K blocksize related fixes

[PATCHv2 0/9] xfstests: 64K blocksize related fixes

[Ritesh Harjani]

Hello,

Below are the list of fixes mostly centered around 64K blocksize
and with ext4 filesystem. Tested this with both 64K & 4K blocksize on Power
with (ext4/ext3/ext2/xfs/btrfs).

v1 -> v2
1. Address comments from Ted and Darrick mentioned at [1]

[1]: https://patchwork.kernel.org/cover/12318137

Ritesh Harjani (9):
  ext4/003: Fix this test on 64K platform for dax config
  ext4/027: Correct the right code of block and inode bitmap
  ext4/306: Add -b blocksize parameter too to avoid failure with DAX config
  ext4/022: exclude this test for dax config on 64KB pagesize platform
  generic/031: Fix the test case for 64k blocksize config
  common/rc: Add _mkfs_dev_blocksized functionality
  generic/620: Use _mkfs_dev_blocksized to use 4k bs
  common/attr: Cleanup end of line whitespaces issues
  common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize

 common/attr           | 57 ++++++++++++++++++++++++++++++++++++-------
 common/rc             | 47 +++++++++++++++++++++++++++++++++++
 tests/ext4/003        |  3 ++-
 tests/ext4/022        |  7 ++++--
 tests/ext4/027        |  4 +--
 tests/ext4/306        |  5 +++-
 tests/generic/031     | 14 +++++++----
 tests/generic/031.out | 16 ++++++------
 tests/generic/620     |  4 ++-
 9 files changed, 128 insertions(+), 29 deletions(-)

--
2.31.1

[PATCHv2 1/9] ext4/003: Fix this test on 64K platform for dax config

[Ritesh Harjani]

mkfs.ext4 by default uses 4K blocksize which doesn't mount when testing
with dax config and the test fails. This patch fixes it.

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/ext4/003 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/ext4/003 b/tests/ext4/003
index 3c9a8486..773bcb03 100755
--- a/tests/ext4/003
+++ b/tests/ext4/003
@@ -26,7 +26,8 @@ _supported_fs ext4
 _require_scratch
 _require_scratch_ext4_feature "bigalloc"
 
-$MKFS_EXT4_PROG -F -O bigalloc -C 65536  -g 256 $SCRATCH_DEV 512m \
+BLOCK_SIZE=$(get_page_size)
+$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
 	>> $seqres.full 2>&1
 _scratch_mount
 
-- 
2.31.1

[PATCHv2 2/9] ext4/027: Correct the right code of block and inode bitmap

[Ritesh Harjani]

Observed occasional failure of this test sometimes say with 64k config
and small device size. Reason is we were grepping for wrong values for
inode and block bitmap.

Correct those values according to [1] to fix this test.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/fsmap.h#n53

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/ext4/027 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ext4/027 b/tests/ext4/027
index 84e11a29..5bcb2d55 100755
--- a/tests/ext4/027
+++ b/tests/ext4/027
@@ -39,11 +39,11 @@ x=$(grep -c 'static fs metadata' $TEST_DIR/fsmap)
 test $x -gt 0 || echo "No fs metadata?"
 
 echo "Check block bitmap" | tee -a $seqres.full
-x=$(grep -c 'special 102:1' $TEST_DIR/fsmap)
+x=$(grep -c 'special 102:3' $TEST_DIR/fsmap)
 test $x -gt 0 || echo "No block bitmaps?"
 
 echo "Check inode bitmap" | tee -a $seqres.full
-x=$(grep -c 'special 102:2' $TEST_DIR/fsmap)
+x=$(grep -c 'special 102:4' $TEST_DIR/fsmap)
 test $x -gt 0 || echo "No inode bitmaps?"
 
 echo "Check inodes" | tee -a $seqres.full
-- 
2.31.1

[PATCHv2 3/9] ext4/306: Add -b blocksize parameter too to avoid failure with DAX config

[Ritesh Harjani]

mkfs.ext4 by default uses 4K blocksize. On DAX config with a 64K
pagesize platform (PPC64), this will fail to mount since DAX requires bs
== ps.
Hence add the -b blocksize paramter in ext4/306.

Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/ext4/306 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/ext4/306 b/tests/ext4/306
index 4a339570..2ff88537 100755
--- a/tests/ext4/306
+++ b/tests/ext4/306
@@ -33,7 +33,10 @@ features="^extents"
 if grep -q 64bit /etc/mke2fs.conf ; then
     features="^extents,^64bit"
 fi
-$MKFS_EXT4_PROG -F -O "$features" $SCRATCH_DEV 512m >> $seqres.full 2>&1
+
+blksz=$(get_page_size)
+
+$MKFS_EXT4_PROG -F -b $blksz -O "$features" $SCRATCH_DEV 512m >> $seqres.full 2>&1
 _scratch_mount
 
 # Create a small non-extent-based file
-- 
2.31.1

[PATCHv2 4/9] ext4/022: exclude this test for dax config on 64KB pagesize platform

[Ritesh Harjani]

This test case assumes blocksize to be 4KB and hence it fails
to mount with "-o dax" option on a 64kb pagesize platform (e.g. PPC64).
This leads to test case reported as failed with dax config on PPC64.

This patch exclude this test when pagesize is 64KB and for dax config.

Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/ext4/022 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/ext4/022 b/tests/ext4/022
index fdc19d93..321050b3 100755
--- a/tests/ext4/022
+++ b/tests/ext4/022
@@ -25,10 +25,13 @@ _require_dumpe2fs
 _require_command "$DEBUGFS_PROG" debugfs
 _require_attrs
 
-# Use large inodes to have enough space for experimentation
-INODE_SIZE=1024
 # Block size
 BLOCK_SIZE=4096
+if [[ $(get_page_size) -ne $BLOCK_SIZE ]]; then
+       _exclude_scratch_mount_option dax
+fi
+# Use large inodes to have enough space for experimentation
+INODE_SIZE=1024
 # We leave this amount of bytes for xattrs
 XATTR_SPACE=256
 # We grow extra_isize by this much
-- 
2.31.1

[PATCHv2 5/9] generic/031: Fix the test case for 64k blocksize config

[Ritesh Harjani]

This test fails with blocksize 64k since the test assumes 4k blocksize
in fcollapse param. This patch fixes that and also tests for 64k
blocksize.

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/generic/031     | 14 +++++++++-----
 tests/generic/031.out | 16 ++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/tests/generic/031 b/tests/generic/031
index 313ce9ff..11961c54 100755
--- a/tests/generic/031
+++ b/tests/generic/031
@@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 
+# fcollapse need offset and len to be multiple of blocksize for filesystems
+# So let's make the offsets and len required for fcollapse multiples of 64K
+# so that it works for all configurations (including on dax on 64K page size
+# systems)
+fact=$((65536/4096))
 $XFS_IO_PROG -f \
-	-c "pwrite 185332 55756" \
-	-c "fcollapse 28672 40960" \
-	-c "pwrite 133228 63394" \
-	-c "fcollapse 0 4096" \
+	-c "pwrite $((185332*fact + 12)) $((55756*fact + 12))" \
+	-c "fcollapse $((28672 * fact)) $((40960 * fact))" \
+	-c "pwrite $((133228 * fact + 12)) $((63394 * fact + 12))" \
+	-c "fcollapse 0 $((4096 * fact))" \
 $testfile | _filter_xfs_io
 
 echo "==== Pre-Remount ==="
@@ -41,4 +46,3 @@ hexdump -C $testfile
 
 status=0
 exit
-
diff --git a/tests/generic/031.out b/tests/generic/031.out
index 194bfa45..7dfcfe41 100644
--- a/tests/generic/031.out
+++ b/tests/generic/031.out
@@ -1,19 +1,19 @@
 QA output created by 031
-wrote 55756/55756 bytes at offset 185332
+wrote 892108/892108 bytes at offset 2965324
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 63394/63394 bytes at offset 133228
+wrote 1014316/1014316 bytes at offset 2131660
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 ==== Pre-Remount ===
 00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
-0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
-0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
+001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
+001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
 *
-0002fdc0
+002fdc18
 ==== Post-Remount ==
 00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
-0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
-0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
+001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
+001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
 *
-0002fdc0
+002fdc18
-- 
2.31.1

[PATCHv2 6/9] common/rc: Add _mkfs_dev_blocksized functionality

[Ritesh Harjani]

This adds _mkfs_dev_blocksized functionality.

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 common/rc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/common/rc b/common/rc
index d4b1f21f..b5fe5c71 100644
--- a/common/rc
+++ b/common/rc
@@ -722,6 +722,53 @@ _mkfs_dev()
     rm -f $tmp.mkfserr $tmp.mkfsstd
 }
 
+_set_mkfs_options_blocksized()
+{
+	local blocksize=$1
+	local re='^[0-9]+$'
+
+	if ! [[ $blocksize =~ $re ]]; then
+		_notrun "error _set_mkfs_options_blocksized: blocksize \"$blocksize\" not an integer"
+	fi
+
+	case $FSTYP in
+	btrfs)
+		test -f /sys/fs/btrfs/features/supported_sectorsizes || \
+		_notrun "Subpage sectorsize support is not found in $FSTYP"
+
+		grep -wq $blocksize /sys/fs/btrfs/features/supported_sectorsizes || \
+		_notrun "$FSTYP does not support sectorsize=$blocksize yet"
+
+		MKFS_OPTIONS=" --sectorsize=$blocksize"
+		;;
+	xfs)
+		MKFS_OPTIONS=" -b size=$blocksize"
+		;;
+	ext2|ext3|ext4)
+		MKFS_OPTIONS=" -b $blocksize"
+		;;
+	gfs2)
+		MKFS_OPTIONS=" -O -b $blocksize"
+		;;
+	ocfs2)
+		MKFS_OPTIONS=" -b $blocksize -C $blocksize"
+		;;
+	bcachefs)
+		MKFS_OPTIONS=" --block_size=$blocksize"
+		;;
+	*)
+		# do nothing for other FS.
+		;;
+	esac
+}
+
+_mkfs_dev_blocksized()
+{
+	_set_mkfs_options_blocksized $1
+	shift
+	_mkfs_dev $*
+}
+
 # remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
 _scratch_cleanup_files()
 {
-- 
2.31.1

[PATCHv2 7/9] generic/620: Use _mkfs_dev_blocksized to use 4k bs

[Ritesh Harjani]

ext4 with 64k blocksize (passed by user config) fails with below error for
this given test which requires dmhugedisk. Since this test anyways only
requires 4k bs, so use _mkfs_dev_blocksized() to fix this.

<error log with 64k bs>
mkfs.ext4: Input/output error while writing out and closing file system

Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 tests/generic/620 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/generic/620 b/tests/generic/620
index b052376f..444e682d 100755
--- a/tests/generic/620
+++ b/tests/generic/620
@@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
 chunk_size=128
 
 _dmhugedisk_init $sectors $chunk_size
-_mkfs_dev $DMHUGEDISK_DEV
+
+# Use 4k blocksize.
+_mkfs_dev_blocksized 4096 $DMHUGEDISK_DEV
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
 testfile=$SCRATCH_MNT/testfile-$seq
 
-- 
2.31.1

[PATCHv2 8/9] common/attr: Cleanup end of line whitespaces issues

[Ritesh Harjani]

This patch clears the end of line whitespace issues in this file.
Mostly since many kernel developers also keep this editor config to clear
any end of line whitespaces on file save.

Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 common/attr | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/common/attr b/common/attr
index 42ceab92..d3902346 100644
--- a/common/attr
+++ b/common/attr
@@ -59,10 +59,10 @@ _acl_setup_ids()
         j=1
         for(i=1; i<1000000 && j<=3;i++){
           if (! (i in ids)) {
-	     printf "acl%d=%d;", j, i;		 
+	     printf "acl%d=%d;", j, i;
 	     j++
           }
-        }	
+        }
       }'`
 }
 
@@ -101,7 +101,7 @@ _getfacl_filter_id()
 _acl_ls()
 {
     _ls_l -n $* | awk '{ print $1, $3, $4, $NF }' | _acl_filter_id
-} 
+}
 
 # create an ACL with n ACEs in it
 #
@@ -128,7 +128,7 @@ _filter_aces()
 	BEGIN {
 	    FS=":"
 	    while ( getline <tmpfile > 0 ) {
-		idlist[$1] = $3 
+		idlist[$1] = $3
 	    }
 	}
 	/^user/ { if ($2 in idlist) sub($2, idlist[$2]); print; next}
@@ -180,17 +180,17 @@ _require_attrs()
 {
 	local args
 	local nsp
-	
+
 	if [ $# -eq 0 ]; then
 		args="user"
 	else
 	  	args="$*"
 	fi
-	
+
 	[ -n "$ATTR_PROG" ] || _notrun "attr command not found"
 	[ -n "$GETFATTR_PROG" ] || _notrun "getfattr command not found"
 	[ -n "$SETFATTR_PROG" ] || _notrun "setfattr command not found"
-	
+
 	for nsp in $args; do
 		#
 		# Test if chacl is able to write an attribute on the target
@@ -204,14 +204,14 @@ _require_attrs()
 		touch $TEST_DIR/syscalltest
 		$SETFATTR_PROG -n "$nsp.xfstests" -v "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
 		cat $TEST_DIR/syscalltest.out >> $seqres.full
-		
+
 		if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then
 			_notrun "kernel does not support attrs"
 		fi
 		if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then
 			_notrun "attr namespace $nsp not supported by this filesystem type: $FSTYP"
 		fi
-		
+
 		rm -f $TEST_DIR/syscalltest.out
 	done
 }
-- 
2.31.1

[PATCHv2 9/9] common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize

[Ritesh Harjani]

Test generic/020 fails for ext4 with 64K blocksize.
This adds changes in common/attr for MAX_ATTRS calculations for
ext2|ext3|ext4 along with comments explaining the calculations.

Suggested-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
 common/attr | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/common/attr b/common/attr
index d3902346..35682d7c 100644
--- a/common/attr
+++ b/common/attr
@@ -256,6 +256,45 @@ case "$FSTYP" in
 xfs|udf|pvfs2|9p|ceph|nfs)
 	MAX_ATTRS=1000
 	;;
+ext2|ext3|ext4)
+	# For 4k blocksizes, most of the attributes have an attr_name of
+	# "attribute_NN" which is 12, and "value_NN" which is 8.
+	# But for larger block sizes, we start having extended attributes of the
+	# form "attribute_NNN" or "attribute_NNNN", and "value_NNN" and
+	# "value_NNNN", which causes the round(len(..), 4) to jump up by 4
+	# bytes.  So round_up(len(attr_name, 4)) becomes 16 instead of 12, and
+	# round_up(len(value, 4)) becomes 12 instead of 8.
+	#
+	# For 64K blocksize the calculation becomes
+	# 	max_attrs = (block_size - 32) / (16 + 12 + 16)
+	# or
+	# 	max_attrs = (block_size - 32) / 44
+	#
+	# For 4K blocksize:-
+	# 	max_attrs = (block_size - 32) / (16 + 8 + 12)
+	# or
+	# 	max_attrs = (block_size - 32) / 36
+	#
+	# Note (for 4K bs) above are exact calculations for attrs of type
+	# attribute_NN with values of type value_NN.
+	# With above calculations, for 4k blocksize max_attrs becomes 112.
+	# This means we can have few attrs of type attribute_NNN with values of
+	# type value_NNN. To avoid/handle this we need to add extra 4 bytes of
+	# headroom.
+	#
+	# So for 4K, the calculations becomes:-
+	# 	max_attrs = (block_size - 32) / (16 + 8 + 12 + 4)
+	# or
+	# 	max_attrs = (block_size - 32) / 40
+	#
+	# Assume max ~1 block of attrs
+	BLOCK_SIZE=`_get_block_size $TEST_DIR`
+	if [ $BLOCK_SIZE -le 4096 ]; then
+		let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 8 + 12 + 4)))
+	else
+		let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 12 + 16 )))
+	fi
+	;;
 *)
 	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
-- 
2.31.1

Re: [PATCHv2 5/9] generic/031: Fix the test case for 64k blocksize config

[Eryu Guan]

On Wed, Jul 21, 2021 at 10:57:58AM +0530, Ritesh Harjani wrote:
> This test fails with blocksize 64k since the test assumes 4k blocksize
> in fcollapse param. This patch fixes that and also tests for 64k
> blocksize.
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>  tests/generic/031     | 14 +++++++++-----
>  tests/generic/031.out | 16 ++++++++--------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/generic/031 b/tests/generic/031
> index 313ce9ff..11961c54 100755
> --- a/tests/generic/031
> +++ b/tests/generic/031
> @@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
>  _scratch_mkfs > /dev/null 2>&1
>  _scratch_mount
>  
> +# fcollapse need offset and len to be multiple of blocksize for filesystems
> +# So let's make the offsets and len required for fcollapse multiples of 64K
> +# so that it works for all configurations (including on dax on 64K page size
> +# systems)
> +fact=$((65536/4096))
>  $XFS_IO_PROG -f \
> -	-c "pwrite 185332 55756" \
> -	-c "fcollapse 28672 40960" \
> -	-c "pwrite 133228 63394" \
> -	-c "fcollapse 0 4096" \
> +	-c "pwrite $((185332*fact + 12)) $((55756*fact + 12))" \

Where does this 12 come from? And I'm wondering if this still reproduces
the original bug.

And looks like that the original test setups came from a specific
fsstress or fsx run, and aimed to the specific bug, perhaps we could
require the test with <= 4k block size, and _notrun in 64k case.

Thanks,
Eryu

> +	-c "fcollapse $((28672 * fact)) $((40960 * fact))" \
> +	-c "pwrite $((133228 * fact + 12)) $((63394 * fact + 12))" \
> +	-c "fcollapse 0 $((4096 * fact))" \
>  $testfile | _filter_xfs_io
>  
>  echo "==== Pre-Remount ==="
> @@ -41,4 +46,3 @@ hexdump -C $testfile
>  
>  status=0
>  exit
> -
> diff --git a/tests/generic/031.out b/tests/generic/031.out
> index 194bfa45..7dfcfe41 100644
> --- a/tests/generic/031.out
> +++ b/tests/generic/031.out
> @@ -1,19 +1,19 @@
>  QA output created by 031
> -wrote 55756/55756 bytes at offset 185332
> +wrote 892108/892108 bytes at offset 2965324
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 63394/63394 bytes at offset 133228
> +wrote 1014316/1014316 bytes at offset 2131660
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  ==== Pre-Remount ===
>  00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>  *
> -0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> -0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> +001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> +001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
>  *
> -0002fdc0
> +002fdc18
>  ==== Post-Remount ==
>  00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>  *
> -0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> -0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> +001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> +001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
>  *
> -0002fdc0
> +002fdc18
> -- 
> 2.31.1

Re: [PATCHv2 7/9] generic/620: Use _mkfs_dev_blocksized to use 4k bs

[Eryu Guan]

On Wed, Jul 21, 2021 at 10:58:00AM +0530, Ritesh Harjani wrote:
> ext4 with 64k blocksize (passed by user config) fails with below error for
> this given test which requires dmhugedisk. Since this test anyways only
> requires 4k bs, so use _mkfs_dev_blocksized() to fix this.
> 
> <error log with 64k bs>
> mkfs.ext4: Input/output error while writing out and closing file system
> 
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
>  tests/generic/620 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/generic/620 b/tests/generic/620
> index b052376f..444e682d 100755
> --- a/tests/generic/620
> +++ b/tests/generic/620
> @@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
>  chunk_size=128
>  
>  _dmhugedisk_init $sectors $chunk_size
> -_mkfs_dev $DMHUGEDISK_DEV
> +
> +# Use 4k blocksize.
> +_mkfs_dev_blocksized 4096 $DMHUGEDISK_DEV

We run the test by forcing 4k blocksize, which could be tested in 4k
blocksize setup. Maybe it's another case that should _notrun in 64k
blocksize setup.

Thanks,
Eryu

>  _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
>  testfile=$SCRATCH_MNT/testfile-$seq
>  
> -- 
> 2.31.1

Re: [PATCHv2 0/9] xfstests: 64K blocksize related fixes

[Eryu Guan]

On Wed, Jul 21, 2021 at 10:57:53AM +0530, Ritesh Harjani wrote:
> Hello,
> 
> Below are the list of fixes mostly centered around 64K blocksize
> and with ext4 filesystem. Tested this with both 64K & 4K blocksize on Power
> with (ext4/ext3/ext2/xfs/btrfs).
> 
> v1 -> v2
> 1. Address comments from Ted and Darrick mentioned at [1]

Thanks for the fixes! I've applied patch 1-4 and patch 8-9. Patch 5-7
may need more discusstions.

Thanks,
Eryu

> 
> [1]: https://patchwork.kernel.org/cover/12318137
> 
> Ritesh Harjani (9):
>   ext4/003: Fix this test on 64K platform for dax config
>   ext4/027: Correct the right code of block and inode bitmap
>   ext4/306: Add -b blocksize parameter too to avoid failure with DAX config
>   ext4/022: exclude this test for dax config on 64KB pagesize platform
>   generic/031: Fix the test case for 64k blocksize config
>   common/rc: Add _mkfs_dev_blocksized functionality
>   generic/620: Use _mkfs_dev_blocksized to use 4k bs
>   common/attr: Cleanup end of line whitespaces issues
>   common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize
> 
>  common/attr           | 57 ++++++++++++++++++++++++++++++++++++-------
>  common/rc             | 47 +++++++++++++++++++++++++++++++++++
>  tests/ext4/003        |  3 ++-
>  tests/ext4/022        |  7 ++++--
>  tests/ext4/027        |  4 +--
>  tests/ext4/306        |  5 +++-
>  tests/generic/031     | 14 +++++++----
>  tests/generic/031.out | 16 ++++++------
>  tests/generic/620     |  4 ++-
>  9 files changed, 128 insertions(+), 29 deletions(-)
> 
> --
> 2.31.1

Re: [PATCHv2 5/9] generic/031: Fix the test case for 64k blocksize config

[Ritesh Harjani]

On 21/08/02 12:00AM, Eryu Guan wrote:
> On Wed, Jul 21, 2021 at 10:57:58AM +0530, Ritesh Harjani wrote:
> > This test fails with blocksize 64k since the test assumes 4k blocksize
> > in fcollapse param. This patch fixes that and also tests for 64k
> > blocksize.
> >
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > ---
> >  tests/generic/031     | 14 +++++++++-----
> >  tests/generic/031.out | 16 ++++++++--------
> >  2 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/generic/031 b/tests/generic/031
> > index 313ce9ff..11961c54 100755
> > --- a/tests/generic/031
> > +++ b/tests/generic/031
> > @@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
> >  _scratch_mkfs > /dev/null 2>&1
> >  _scratch_mount
> >
> > +# fcollapse need offset and len to be multiple of blocksize for filesystems
> > +# So let's make the offsets and len required for fcollapse multiples of 64K
> > +# so that it works for all configurations (including on dax on 64K page size
> > +# systems)
> > +fact=$((65536/4096))
> >  $XFS_IO_PROG -f \
> > -	-c "pwrite 185332 55756" \
> > -	-c "fcollapse 28672 40960" \
> > -	-c "pwrite 133228 63394" \
> > -	-c "fcollapse 0 4096" \
> > +	-c "pwrite $((185332*fact + 12)) $((55756*fact + 12))" \
>
> Where does this 12 come from?
A random number so that the offset and length are not bocksize aligned.
If you see the final .out file, you will see the offset of the writes
remains the same with and before this patch.

> And I'm wondering if this still reproduces the original bug.
I am not sure how to trigger this. I know that this test was intended for
bs < ps cases. If someone can help me / point me to the kernel fix for this,
I can try to reproduce the original bug too.

I found this link for this test patch series. Couldn't find the kernel fixes
link though.
https://www.spinics.net/lists/fstests/msg00340.html


>
> And looks like that the original test setups came from a specific
> fsstress or fsx run, and aimed to the specific bug, perhaps we could
> require the test with <= 4k block size, and _notrun in 64k case.

It would be good to know whether this code could trigger the original bug or
not. Then we need not make _notrun for 64k case.

>
> Thanks,
> Eryu
>
> > +	-c "fcollapse $((28672 * fact)) $((40960 * fact))" \
> > +	-c "pwrite $((133228 * fact + 12)) $((63394 * fact + 12))" \
> > +	-c "fcollapse 0 $((4096 * fact))" \
> >  $testfile | _filter_xfs_io
> >
> >  echo "==== Pre-Remount ==="
> > @@ -41,4 +46,3 @@ hexdump -C $testfile
> >
> >  status=0
> >  exit
> > -
> > diff --git a/tests/generic/031.out b/tests/generic/031.out
> > index 194bfa45..7dfcfe41 100644
> > --- a/tests/generic/031.out
> > +++ b/tests/generic/031.out
> > @@ -1,19 +1,19 @@
> >  QA output created by 031
> > -wrote 55756/55756 bytes at offset 185332
> > +wrote 892108/892108 bytes at offset 2965324
> >  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > -wrote 63394/63394 bytes at offset 133228
> > +wrote 1014316/1014316 bytes at offset 2131660
> >  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >  ==== Pre-Remount ===
> >  00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> >  *
> > -0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> > -0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> > +001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> > +001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> >  *
> > -0002fdc0
> > +002fdc18
> >  ==== Post-Remount ==
> >  00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> >  *
> > -0001f860  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> > -0001f870  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> > +001f86c0  00 00 00 00 00 00 00 00  00 00 00 00 cd cd cd cd  |................|
> > +001f86d0  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  |................|
> >  *
> > -0002fdc0
> > +002fdc18
> > --
> > 2.31.1

Re: [PATCHv2 7/9] generic/620: Use _mkfs_dev_blocksized to use 4k bs

[Ritesh Harjani]

On 21/08/02 12:03AM, Eryu Guan wrote:
> On Wed, Jul 21, 2021 at 10:58:00AM +0530, Ritesh Harjani wrote:
> > ext4 with 64k blocksize (passed by user config) fails with below error for
> > this given test which requires dmhugedisk. Since this test anyways only
> > requires 4k bs, so use _mkfs_dev_blocksized() to fix this.
> >
> > <error log with 64k bs>
> > mkfs.ext4: Input/output error while writing out and closing file system
> >
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > ---
> >  tests/generic/620 | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/generic/620 b/tests/generic/620
> > index b052376f..444e682d 100755
> > --- a/tests/generic/620
> > +++ b/tests/generic/620
> > @@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
> >  chunk_size=128
> >
> >  _dmhugedisk_init $sectors $chunk_size
> > -_mkfs_dev $DMHUGEDISK_DEV
> > +
> > +# Use 4k blocksize.
> > +_mkfs_dev_blocksized 4096 $DMHUGEDISK_DEV
>
> We run the test by forcing 4k blocksize, which could be tested in 4k
> blocksize setup. Maybe it's another case that should _notrun in 64k
> blocksize setup.

So for testing that, first I should mkfs and mount a scratch device with the
passed mount/mkfs options and then see if the blocksize passed is 64K, if yes
I should _notrun this case.

Isn't the current approach of (_mkfs_dev_blocksized 4096) is better then above
approach?

-ritesh

> Thanks,
> Eryu
>
> >  _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
> >  testfile=$SCRATCH_MNT/testfile-$seq
> >
> > --
> > 2.31.1

Re: [PATCHv2 5/9] generic/031: Fix the test case for 64k blocksize config

[Eryu Guan]

On Tue, Aug 03, 2021 at 10:30:33AM +0530, Ritesh Harjani wrote:
> On 21/08/02 12:00AM, Eryu Guan wrote:
> > On Wed, Jul 21, 2021 at 10:57:58AM +0530, Ritesh Harjani wrote:
> > > This test fails with blocksize 64k since the test assumes 4k blocksize
> > > in fcollapse param. This patch fixes that and also tests for 64k
> > > blocksize.
> > >
> > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > > ---
> > >  tests/generic/031     | 14 +++++++++-----
> > >  tests/generic/031.out | 16 ++++++++--------
> > >  2 files changed, 17 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/tests/generic/031 b/tests/generic/031
> > > index 313ce9ff..11961c54 100755
> > > --- a/tests/generic/031
> > > +++ b/tests/generic/031
> > > @@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
> > >  _scratch_mkfs > /dev/null 2>&1
> > >  _scratch_mount
> > >
> > > +# fcollapse need offset and len to be multiple of blocksize for filesystems
> > > +# So let's make the offsets and len required for fcollapse multiples of 64K
> > > +# so that it works for all configurations (including on dax on 64K page size
> > > +# systems)
> > > +fact=$((65536/4096))
> > >  $XFS_IO_PROG -f \
> > > -	-c "pwrite 185332 55756" \
> > > -	-c "fcollapse 28672 40960" \
> > > -	-c "pwrite 133228 63394" \
> > > -	-c "fcollapse 0 4096" \
> > > +	-c "pwrite $((185332*fact + 12)) $((55756*fact + 12))" \
> >
> > Where does this 12 come from?
> A random number so that the offset and length are not bocksize aligned.
> If you see the final .out file, you will see the offset of the writes
> remains the same with and before this patch.
> 
> > And I'm wondering if this still reproduces the original bug.
> I am not sure how to trigger this. I know that this test was intended for
> bs < ps cases. If someone can help me / point me to the kernel fix for this,
> I can try to reproduce the original bug too.
> 
> I found this link for this test patch series. Couldn't find the kernel fixes
> link though.
> https://www.spinics.net/lists/fstests/msg00340.html

I think it's a regression test for this patchset.

https://www.spinics.net/lists/xfs/msg29807.html

> 
> 
> >
> > And looks like that the original test setups came from a specific
> > fsstress or fsx run, and aimed to the specific bug, perhaps we could
> > require the test with <= 4k block size, and _notrun in 64k case.
> 
> It would be good to know whether this code could trigger the original bug or
> not. Then we need not make _notrun for 64k case.

Agreed, if we could make sure that updated test still triggers the
original bug, there's no reason _notrun for 64k case.

Thanks,
Eryu

Re: [PATCHv2 7/9] generic/620: Use _mkfs_dev_blocksized to use 4k bs

[Eryu Guan]

On Tue, Aug 03, 2021 at 10:36:22AM +0530, Ritesh Harjani wrote:
> On 21/08/02 12:03AM, Eryu Guan wrote:
> > On Wed, Jul 21, 2021 at 10:58:00AM +0530, Ritesh Harjani wrote:
> > > ext4 with 64k blocksize (passed by user config) fails with below error for
> > > this given test which requires dmhugedisk. Since this test anyways only
> > > requires 4k bs, so use _mkfs_dev_blocksized() to fix this.

I don't see how this test always requires 4k blocksize, 1k blocksized
xfs also passes the test.

> > >
> > > <error log with 64k bs>
> > > mkfs.ext4: Input/output error while writing out and closing file system

Is this a bug in mkfs.ext4 or expected error (unsupported config)? If
it's an expected error, it'd be better to explain it in commit log as
well.

> > >
> > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > > ---
> > >  tests/generic/620 | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/generic/620 b/tests/generic/620
> > > index b052376f..444e682d 100755
> > > --- a/tests/generic/620
> > > +++ b/tests/generic/620
> > > @@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
> > >  chunk_size=128
> > >
> > >  _dmhugedisk_init $sectors $chunk_size
> > > -_mkfs_dev $DMHUGEDISK_DEV
> > > +
> > > +# Use 4k blocksize.
> > > +_mkfs_dev_blocksized 4096 $DMHUGEDISK_DEV
> >
> > We run the test by forcing 4k blocksize, which could be tested in 4k
> > blocksize setup. Maybe it's another case that should _notrun in 64k
> > blocksize setup.
> 
> So for testing that, first I should mkfs and mount a scratch device with the
> passed mount/mkfs options and then see if the blocksize passed is 64K, if yes
> I should _notrun this case.
> 
> Isn't the current approach of (_mkfs_dev_blocksized 4096) is better then above
> approach?

If the test always requires 4k blocksize, forcing creating a 4k
blocksize filesystem doesn't increase any test coverage, I don't see any
point introducing a new _mkfs_dev_blocksized helper just to do so.

And even if we decide to force 4k blocksize config, I think it'd be
better to update _scratch_mkfs_blocksized() to take device as argument,
like what _check_scratch_fs() does, so we don't duplicate all the code
to create fs with specified blocksize.

Thanks,
Eryu

> 
> -ritesh
> 
> > Thanks,
> > Eryu
> >
> > >  _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
> > >  testfile=$SCRATCH_MNT/testfile-$seq
> > >
> > > --
> > > 2.31.1