64K blocksize related fixes

PATCH Superseded Ritesh Harjani 3 participants 28 messages Jun 14, 2021 – Jul 9, 2021

Share & tools

Ritesh Harjani

author

2021-06-14 06:28 UTC #0 lore raw

cover 64K blocksize related fixes

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

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
gitignore: Add 031.out file to .gitignore
generic/620: Remove -b blocksize option for ext4
common/attr: Cleanup end of line whitespaces issues common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize

 .gitignore                                 |  1 +
 common/attr                                | 20 ++++++------
 tests/ext4/003                             |  3 +-
 tests/ext4/022                             |  7 ++--
 tests/ext4/027                             |  4 +--
 tests/ext4/306                             |  5 ++-
 tests/generic/031                          | 37 ++++++++++++++++++----
 tests/generic/031.out.64k                  | 19 +++++++++++
 tests/generic/{031.out => 031.out.default} |  0
 tests/generic/620                          |  7 ++++
 10 files changed, 80 insertions(+), 23 deletions(-)
 create mode 100644 tests/generic/031.out.64k
 rename tests/generic/{031.out => 031.out.default} (100%)

--
2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #1 lore raw

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

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 <[email protected]>

---
 tests/ext4/003 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tests/ext4/003 +2 -1

index 00ea9150..1ddb3063 100755
@@ -31,7 +31,8 @@ _require_scratch_ext4_feature "bigalloc"
 
 rm -f $seqres.full
 
-$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 65536  -g 256 $SCRATCH_DEV 512m \
 	>> $seqres.full 2>&1
 _scratch_mount

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #5 lore raw

PATCH 2/9 ext4/027: Correct the right code of block and inode bitmap

In reply to 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

Signed-off-by Ritesh Harjani <[email protected]>

---
 tests/ext4/027 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tests/ext4/027 +2 -2

index 97c14cf5..83d5a413 100755
@@ -45,11 +45,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

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #7 lore raw

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

In reply to 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.

Signed-off-by Ritesh Harjani <[email protected]>

---
 tests/ext4/306 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

tests/ext4/306 +4 -1

index 146fdb39..1d45a9d0 100755
@@ -38,7 +38,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

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #9 lore raw

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

In reply to 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.

Signed-off-by Ritesh Harjani <[email protected]>

---
 tests/ext4/022 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

tests/ext4/022 +5 -2

index 3de7619a..ca58f34e 100755
@@ -41,10 +41,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

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #11 lore raw

PATCH 5/9 generic/031: Fix the test case for 64k blocksize config

In reply to 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 <[email protected]>

---
 tests/generic/031                          | 37 ++++++++++++++++++----
 tests/generic/031.out.64k                  | 19 +++++++++++
 tests/generic/{031.out => 031.out.default} |  0
 3 files changed, 49 insertions(+), 7 deletions(-)
 create mode 100644 tests/generic/031.out.64k
 rename tests/generic/{031.out => 031.out.default} (100%)

tests/generic/031 +30 -7

index db84031b..40cb23af 100755
@@ -8,6 +8,7 @@
 # correctly written and aren't left behind causing invalidation or data
 # corruption issues.
 #
+seqfull=$0
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
 echo "QA output created by $seq"

@@ -39,12 +40,35 @@ testfile=$SCRATCH_MNT/testfile
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 
-$XFS_IO_PROG -f \
-	-c "pwrite 185332 55756" \
-	-c "fcollapse 28672 40960" \
-	-c "pwrite 133228 63394" \
-	-c "fcollapse 0 4096" \
-$testfile | _filter_xfs_io
+# fcollapse need offset and len to be multiple of blocksize for filesystems
+# hence make this test work with 64k blocksize as well.
+blksz=$(_get_block_size $SCRATCH_MNT)
+
+rm -f $seqfull.out
+if [ "$blksz" -eq 65536 ]; then
+	ln -s $seq.out.64k $seqfull.out
+else
+	ln -s $seq.out.default $seqfull.out
+fi
+
+if [[ $blksz -le 4096 ]]; then
+	$XFS_IO_PROG -f \
+		-c "pwrite 185332 55756" \
+		-c "fcollapse 28672 40960" \
+		-c "pwrite 133228 63394" \
+		-c "fcollapse 0 4096" \
+	$testfile | _filter_xfs_io
+elif [[ $blksz -eq 65536 ]]; then
+	fact=$blksz/4096
+	$XFS_IO_PROG -f \
+		-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
+else
+	_notrun "blocksize not supported"
+fi
 
 echo "==== Pre-Remount ==="
 hexdump -C $testfile

@@ -54,4 +78,3 @@ hexdump -C $testfile
 
 status=0
 exit
-

tests/generic/031.out.64k new file +19 -0

new file mode 100644
index 00000000..7dfcfe41
@@ -0,0 +1,19 @@
+QA output created by 031
+wrote 892108/892108 bytes at offset 2965324
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+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  |................|
+*
+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  |................|
+*
+002fdc18
+==== Post-Remount ==
+00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+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  |................|
+*
+002fdc18

tests/generic/031.out.default renamed from tests/generic/031.out +0 -0

similarity index 100%
rename from tests/generic/031.out
rename to tests/generic/031.out.default

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #15 lore raw

PATCH 6/9 gitignore: Add 031.out file to .gitignore

In reply to Ritesh Harjani

Add 031.out file to .gitignore

Signed-off-by Ritesh Harjani <[email protected]>

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

.gitignore +1 -0

index d3194e76..7a3be5e3 100644
@@ -191,6 +191,7 @@ tags
 # Symlinked files
 /tests/generic/035.out
 /tests/generic/050.out
+/tests/generic/031.out
 /tests/xfs/033.out
 /tests/xfs/071.out
 /tests/xfs/096.out

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #17 lore raw

PATCH 7/9 generic/620: Remove -b blocksize option for ext4

In reply to Ritesh Harjani

ext4 with 64k blocksize fails with below error for this given test which requires dmhugedisk. Also since dax is not supported for this test, so make sure to remove -b option, if set by config file for ext4 FSTYP for the test to then use 4K blocksize by default.

mkfs.ext4: Input/output error while writing out and closing file system

Signed-off-by Ritesh Harjani <[email protected]>

---
 tests/generic/620 | 7 +++++++
 1 file changed, 7 insertions(+)

tests/generic/620 +7 -0

index 60559441..3ccda5e4 100755
@@ -50,6 +50,13 @@ _require_dmhugedisk
 sectors=$((2*1024*1024*1024*17))
 chunk_size=128
 
+# ext4 with 64k blocksize fails to mkfs with below error.
+# So remove -b option, if set by config file.
+# mkfs.ext4: Input/output error while writing out and closing file system
+if [[ $FSTYP = "ext4" ]]; then
+	MKFS_OPTIONS=$(echo $MKFS_OPTIONS | sed -rn 's/(.*)(-b ?+[0-9]+)(.*)/\1 \3/p')
+fi
+
 _dmhugedisk_init $sectors $chunk_size
 _mkfs_dev $DMHUGEDISK_DEV
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #20 lore raw

PATCH 8/9 common/attr: Cleanup end of line whitespaces issues

In reply to 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.

Signed-off-by Ritesh Harjani <[email protected]>

---
 common/attr | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

common/attr +9 -9

index 42ceab92..d3902346 100644
@@ -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
 }

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-14 06:28 UTC #23 lore raw

PATCH 9/9 common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize

In reply to Ritesh Harjani

Test generic/020 fails for ext4 with 64K blocksize. So increase some overhead value to reduce the MAX_ATTRS so that it can accomodate for 64K blocksize.

Signed-off-by Ritesh Harjani <[email protected]>

---
 common/attr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

common/attr +1 -1

index d3902346..e8661d80 100644
@@ -260,7 +260,7 @@ xfs|udf|pvfs2|9p|ceph|nfs)
 	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
-	let MAX_ATTRS=$BLOCK_SIZE/40
+	let MAX_ATTRS=$BLOCK_SIZE/48
 esac
 
 export MAX_ATTRS

Signature · 1 line

2.31.1

Darrick J. Wong

Reviewed-by on v1 2/9

2021-06-14 16:41 UTC #6 lore raw

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

In reply to Ritesh Harjani

Quoted 18 lines · 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
Signed-off-by Ritesh Harjani <[email protected]>
---
 tests/ext4/027 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tests/ext4/027 +0 -0
index 97c14cf5..83d5a413 100755

tests/ext4/027 x=$(grep -c 'static fs metadata' $TEST_DIR/fsmap) · line 52

Quoted 10 lines

tests/ext4/027 +2 -2

@@ -45,11 +45,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)

Maaaaybe I should have added textual descriptions for the ext4 getfsmap owners. Sorry... :(

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 6 lines

tests/ext4/027 +0 -0

 test $x -gt 0 || echo "No inode bitmaps?"
 
 echo "Check inodes" | tee -a $seqres.full

Signature · 1 line

2.31.1

Ritesh Harjani

author

2021-06-30 13:30 UTC #27 lore raw

In reply to Ritesh Harjani

Quoted 3 lines · Ritesh Harjani

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

Gentle reminder on this patch series.

-ritesh

Quoted 29 lines

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
gitignore: Add 031.out file to .gitignore
generic/620: Remove -b blocksize option for ext4
common/attr: Cleanup end of line whitespaces issues common/attr: Reduce MAX_ATTRS to leave some overhead for 64K blocksize
 .gitignore                                 |  1 +
 common/attr                                | 20 ++++++------
 tests/ext4/003                             |  3 +-
 tests/ext4/022                             |  7 ++--
 tests/ext4/027                             |  4 +--
 tests/ext4/306                             |  5 ++-
 tests/generic/031                          | 37 ++++++++++++++++++----
 tests/generic/031.out.64k                  | 19 +++++++++++
 tests/generic/{031.out => 031.out.default} |  0
 tests/generic/620                          |  7 ++++
 10 files changed, 80 insertions(+), 23 deletions(-)
 create mode 100644 tests/generic/031.out.64k
 rename tests/generic/{031.out => 031.out.default} (100%)
--
2.31.1

Darrick J. Wong

2021-06-30 15:50 UTC #12 lore raw

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

In reply to Ritesh Harjani

Quoted 45 lines · 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 <[email protected]>

---
 tests/generic/031                          | 37 ++++++++++++++++++----
 tests/generic/031.out.64k                  | 19 +++++++++++
 tests/generic/{031.out => 031.out.default} |  0
 3 files changed, 49 insertions(+), 7 deletions(-)
 create mode 100644 tests/generic/031.out.64k
 rename tests/generic/{031.out => 031.out.default} (100%)

tests/generic/031 +11 -6

index db84031b..40cb23af 100755
@@ -8,6 +8,7 @@
 # correctly written and aren't left behind causing invalidation or data
 # corruption issues.
 #
+seqfull=$0
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
 echo "QA output created by $seq"

@@ -39,12 +40,35 @@ testfile=$SCRATCH_MNT/testfile
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 
-$XFS_IO_PROG -f \
-	-c "pwrite 185332 55756" \
-	-c "fcollapse 28672 40960" \
-	-c "pwrite 133228 63394" \
-	-c "fcollapse 0 4096" \
-$testfile | _filter_xfs_io
+# fcollapse need offset and len to be multiple of blocksize for filesystems
+# hence make this test work with 64k blocksize as well.
+blksz=$(_get_block_size $SCRATCH_MNT)
+
+rm -f $seqfull.out
+if [ "$blksz" -eq 65536 ]; then
+	ln -s $seq.out.64k $seqfull.out
+else
+	ln -s $seq.out.default $seqfull.out
+fi

tests/generic/031 testfile=$SCRATCH_MNT/testfile · line 62

Quoted 10 lines

tests/generic/031 +10 -0

+
+if [[ $blksz -le 4096 ]]; then
+	$XFS_IO_PROG -f \
+		-c "pwrite 185332 55756" \
+		-c "fcollapse 28672 40960" \
+		-c "pwrite 133228 63394" \
+		-c "fcollapse 0 4096" \
+	$testfile | _filter_xfs_io
+elif [[ $blksz -eq 65536 ]]; then
+	fact=$blksz/4096

What if the blocksize is 32k?

--D

Quoted 49 lines

tests/generic/031 +9 -1

+	$XFS_IO_PROG -f \
+		-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
+else
+	_notrun "blocksize not supported"
+fi
 
 echo "==== Pre-Remount ==="
 hexdump -C $testfile

@@ -54,4 +78,3 @@ hexdump -C $testfile
 
 status=0
 exit
-

tests/generic/031.out.64k new file +19 -0

new file mode 100644
index 00000000..7dfcfe41
@@ -0,0 +1,19 @@
+QA output created by 031
+wrote 892108/892108 bytes at offset 2965324
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+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  |................|
+*
+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  |................|
+*
+002fdc18
+==== Post-Remount ==
+00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
+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  |................|
+*
+002fdc18

tests/generic/031.out.default renamed from tests/generic/031.out +0 -0

similarity index 100%
rename from tests/generic/031.out
rename to tests/generic/031.out.default

Signature · 1 line

2.31.1

Darrick J. Wong

Reviewed-by on v1 8/9

2021-06-30 15:50 UTC #21 lore raw

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

In reply to Ritesh Harjani

Quoted 5 lines · 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.
Signed-off-by Ritesh Harjani <[email protected]>

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 80 lines

---
 common/attr | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

common/attr +9 -9

index 42ceab92..d3902346 100644
@@ -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
 }

Signature · 1 line

2.31.1

Darrick J. Wong

2021-06-30 15:51 UTC #24 lore raw

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

In reply to Ritesh Harjani

Quoted 13 lines · Ritesh Harjani

Test generic/020 fails for ext4 with 64K blocksize. So increase some overhead value to reduce the MAX_ATTRS so that it can accomodate for 64K blocksize.
Signed-off-by Ritesh Harjani <[email protected]>
---
 common/attr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/attr +0 -0
index d3902346..e8661d80 100644

Quoted 5 lines

common/attr +1 -1

@@ -260,7 +260,7 @@ xfs|udf|pvfs2|9p|ceph|nfs)
 	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
-	let MAX_ATTRS=$BLOCK_SIZE/40
+	let MAX_ATTRS=$BLOCK_SIZE/48

50% is quite a lot of overhead; maybe we should special-case this?

--D

Quoted 6 lines

common/attr +0 -0
 esac
 
 export MAX_ATTRS
Signature · 1 line
2.31.1

Theodore Ts'o

2021-06-30 16:27 UTC #2 lore raw

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

In reply to Ritesh Harjani

Quoted 13 lines · 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 <[email protected]>
---
 tests/ext4/003 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tests/ext4/003 +0 -0
index 00ea9150..1ddb3063 100755

tests/ext4/003 _require_scratch_ext4_feature "bigalloc" · line 37

Quoted 8 lines

tests/ext4/003 +2 -1

@@ -31,7 +31,8 @@ _require_scratch_ext4_feature "bigalloc"
 
 rm -f $seqres.full
 
-$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 65536  -g 256 $SCRATCH_DEV 512m \
 	>> $seqres.full 2>&1
 _scratch_mount

Thanks for the patch!

If the block size is 64k, then the cluster_size == block_size at which point ext4/003 won't be able to test for the regression its designed to test. So we probably need to scale the cluster size and file system size relative to the block size.

- Ted

Theodore Ts'o

Reviewed-by on v1 3/9

2021-06-30 16:29 UTC #8 lore raw

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

In reply to Ritesh Harjani

Quoted 6 lines · 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.
Signed-off-by Ritesh Harjani <[email protected]>

Looks good, thanks.

Reviewed-by Theodore Ts'o <[email protected]>

- Ted

Theodore Ts'o

Reviewed-by on v1 4/9

2021-06-30 16:36 UTC #10 lore raw

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

In reply to Ritesh Harjani

Quoted 7 lines · 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.
Signed-off-by Ritesh Harjani <[email protected]>

Looks good, thanks.

Reviewed-by Theodore Ts'o <[email protected]>

Theodore Ts'o

Reviewed-by on v1 6/9

2021-06-30 16:36 UTC #16 lore raw

Re: [PATCH 6/9] gitignore: Add 031.out file to .gitignore

In reply to Ritesh Harjani

Quoted 3 lines · Ritesh Harjani

Add 031.out file to .gitignore
Signed-off-by Ritesh Harjani <[email protected]>

Reviewed-by Theodore Ts'o <[email protected]>

Theodore Ts'o

2021-06-30 17:07 UTC #18 lore raw

Re: [PATCH 7/9] generic/620: Remove -b blocksize option for ext4

In reply to Ritesh Harjani

Quoted 8 lines · Ritesh Harjani

ext4 with 64k blocksize fails with below error for this given test which requires dmhugedisk. Also since dax is not supported for this test, so make sure to remove -b option, if set by config file for ext4 FSTYP for the test to then use 4K blocksize by default.
mkfs.ext4: Input/output error while writing out and closing file system
Signed-off-by Ritesh Harjani <[email protected]>

Looking at this test, I'm not convinced it actually does the right thing when the block size is 64k, since the whole point is to test what happens when the block number > INT_MAX. So we should be able to fix the block size to be 1k, which would allow us to use a smaller dmhugedisk, and then skip this test if dax is enabled.

OTOH, generic/620 runs pretty quicky, so perhaps it's better to do thie quick fix: hardcode the block size to 4k, and then skip it if dax && page_size != 4k.

- Ted

Theodore Ts'o

2021-06-30 17:18 UTC #13 lore raw

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

In reply to Darrick J. Wong

Quoted 5 lines · Darrick J. Wong

tests/generic/031 testfile=$SCRATCH_MNT/testfile · line 44
Quoted 2 lines
tests/generic/031 +2 -0
+# fcollapse need offset and len to be multiple of blocksize for filesystems
+# hence make this test work with 64k blocksize as well.
...
What if the blocksize is 32k?

... or 8k? or 16k? (which might be more likely)

How bout if we change the offsets and lengths in the test so they are all multiples of 64k, and adjusting 31.out appropriately? That will allow the test to work for block sizes up to 64k without needing to having a special case for 031.out.64k.

I don't know of architectures with a page size > 64k (yet), so this should hold us for a while.

- Ted

Theodore Ts'o

Reviewed-by on v1 8/9

2021-06-30 17:19 UTC #22 lore raw

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

In reply to Ritesh Harjani

Quoted 5 lines · 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.
Signed-off-by Ritesh Harjani <[email protected]>

Reviewed-by Theodore Ts'o <[email protected]>

Theodore Ts'o

2021-06-30 19:20 UTC #25 lore raw

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

In reply to Darrick J. Wong

Quoted 9 lines · Darrick J. Wong

Quoted 8 lines · Ritesh Harjani
Test generic/020 fails for ext4 with 64K blocksize. So increase some overhead value to reduce the MAX_ATTRS so that it can accomodate for 64K blocksize.
Signed-off-by Ritesh Harjani <[email protected]>
---
 common/attr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 12 lines

common/attr xfs|udf|pvfs2|9p|ceph|nfs) · line 263
Quoted 10 lines
common/attr +1 -1
index d3902346..e8661d80 100644
@@ -260,7 +260,7 @@ xfs|udf|pvfs2|9p|ceph|nfs)
 	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
-	let MAX_ATTRS=$BLOCK_SIZE/40
+	let MAX_ATTRS=$BLOCK_SIZE/48
50% is quite a lot of overhead; maybe we should special-case this?

The problem is that 32 bytes is an underestimate when i > 99 for user.attribute_$i=value_$i. And with a 4k blocksize, MAX_ATTRS = 4096 / 40 = 102.

The exact calculation for ext4 is:

fixed_block_overhead = 32
fixed_entry_overhead = 16
max_attr = (block_size - fixed_block_overhead) /

	(fixed_entry_overhead + round_up(len(attr_name), 4) +
	 round_up(len(value), 4))

For 4k blocksizes, most of the attributes have an attr_name of "attribute_NN" which is 8, and "value_NN" which is 12.

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 12 instead of 8, and round_up(len(value, 4)) becomes 16 instead of 12. So:

	max_attrs = (block_size - 32) / (16 + 12 + 16)
or
	max_attrs = (block_size - 32) / 44

instead of:

	max_attrs = (block_size - 32) / (16 + 8 + 12)
or
	max_attrs = (block_size - 32) / 36

So special casing things for block sizes > 4k may very well make sense. Perhaps it's even worth it to put in an ext[234] specific, exalc calculation for MAX_ATTRS in common/attr.

Cheers,

- Ted

Ritesh Harjani

author

2021-07-08 06:24 UTC #3 lore raw

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

In reply to Theodore Ts'o

Quoted 17 lines · Theodore Ts'o

Quoted 16 lines · 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 <[email protected]>
---
 tests/ext4/003 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tests/ext4/003 +0 -0
index 00ea9150..1ddb3063 100755
@@ -31,7 +31,8 @@ _require_scratch_ext4_feature "bigalloc"
 
 rm -f $seqres.full
 

Quoted 7 lines

tests/ext4/003 _require_scratch_ext4_feature "bigalloc" · line 37
Quoted 5 lines
tests/ext4/003 +2 -1
-$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 65536  -g 256 $SCRATCH_DEV 512m \
 	>> $seqres.full 2>&1
 _scratch_mount
Thanks for the patch!

Thanks for the review, sorry about the delay (- Last week was short a week for me).

Quoted 5 lines

If the block size is 64k, then the cluster_size == block_size at which point ext4/003 won't be able to test for the regression its designed to test. So we probably need to scale the cluster size and file system size relative to the block size.

Yes, thanks for catching it. I think if make below change, i.e. scale cluster size, we should be good. Since this will make blocks_per_group = 4096 and clusters_per_group = 256. This is the condition, which I guess the original kernel patch fixed it for. So, we need not increase the filesystem size.

$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $((BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \

-ritesh

Quoted 2 lines

- Ted

Ritesh Harjani

author

2021-07-08 09:49 UTC #14 lore raw

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

In reply to Theodore Ts'o

Quoted 9 lines · Theodore Ts'o

Quoted 5 lines · Darrick J. Wong
tests/generic/031 testfile=$SCRATCH_MNT/testfile · line 44
Quoted 2 lines
tests/generic/031 +2 -0
+# fcollapse need offset and len to be multiple of blocksize for filesystems
+# hence make this test work with 64k blocksize as well.
...
What if the blocksize is 32k?
... or 8k? or 16k? (which might be more likely)

Quoted 8 lines

How bout if we change the offsets and lengths in the test so they are all multiples of 64k, and adjusting 31.out appropriately? That will allow the test to work for block sizes up to 64k without needing to having a special case for 031.out.64k.
I don't know of architectures with a page size > 64k (yet), so this should hold us for a while.

yes, so I already had done the changes in such a way that we can adapt to any blocksize. I will make the changes such that we take fact=65536/4096 by default. Then this should cover all the cases for all blocksizes and we don't have to change 031.out file for different blocksizes.

And the test tries to test non-aligned writes, but since I am adding some additional unaligned bytes and also I have kept the layout of the writes same as before, so I think the test should still cover the regression it is meant for.

fact=65536/4096

    $XFS_IO_PROG -f \
        -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

-ritesh

Ritesh Harjani

author

2021-07-08 10:01 UTC #19 lore raw

Re: [PATCH 7/9] generic/620: Remove -b blocksize option for ext4

In reply to Theodore Ts'o

Quoted 16 lines · Theodore Ts'o

Quoted 8 lines · Ritesh Harjani
ext4 with 64k blocksize fails with below error for this given test which requires dmhugedisk. Also since dax is not supported for this test, so make sure to remove -b option, if set by config file for ext4 FSTYP for the test to then use 4K blocksize by default.
mkfs.ext4: Input/output error while writing out and closing file system
Signed-off-by Ritesh Harjani <[email protected]>
Looking at this test, I'm not convinced it actually does the right thing when the block size is 64k, since the whole point is to test what happens when the block number > INT_MAX. So we should be able to fix the block size to be 1k, which would allow us to use a smaller dmhugedisk, and then skip this test if dax is enabled.

Quoted 3 lines

OTOH, generic/620 runs pretty quicky, so perhaps it's better to do thie quick fix: hardcode the block size to 4k, and then skip it if dax && page_size != 4k.

Ok, so it is time to implement _mkfs_dev_blocksized() something like how we have for _scratch_mkfs_blocksized(). This is since we can have different way of passing blocksize parameter for mkfs prog for different filesystems.

-ritesh

Theodore Ts'o

2021-07-08 12:51 UTC #4 lore raw

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

In reply to Ritesh Harjani

Quoted 7 lines · Ritesh Harjani

Yes, thanks for catching it. I think if make below change, i.e. scale cluster size, we should be good. Since this will make blocks_per_group = 4096 and clusters_per_group = 256. This is the condition, which I guess the original kernel patch fixed it for. So, we need not increase the filesystem size.
$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $((BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \

Agreed, it looks like that should work.

Cheers,

- Ted

Ritesh Harjani

author

2021-07-09 05:12 UTC #26 lore raw

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

In reply to Theodore Ts'o

Quoted 48 lines · Theodore Ts'o

Quoted 9 lines · Darrick J. Wong
Quoted 8 lines · Ritesh Harjani
Test generic/020 fails for ext4 with 64K blocksize. So increase some overhead value to reduce the MAX_ATTRS so that it can accomodate for 64K blocksize.
Signed-off-by Ritesh Harjani <[email protected]>
---
 common/attr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines
common/attr xfs|udf|pvfs2|9p|ceph|nfs) · line 263
Quoted 10 lines
common/attr +1 -1
index d3902346..e8661d80 100644
@@ -260,7 +260,7 @@ xfs|udf|pvfs2|9p|ceph|nfs)
 	# Assume max ~1 block of attrs
 	BLOCK_SIZE=`_get_block_size $TEST_DIR`
 	# user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
-	let MAX_ATTRS=$BLOCK_SIZE/40
+	let MAX_ATTRS=$BLOCK_SIZE/48
50% is quite a lot of overhead; maybe we should special-case this?
The problem is that 32 bytes is an underestimate when i > 99 for user.attribute_$i=value_$i. And with a 4k blocksize, MAX_ATTRS = 4096 / 40 = 102.
The exact calculation for ext4 is:
fixed_block_overhead = 32
fixed_entry_overhead = 16
max_attr = (block_size - fixed_block_overhead) /
	(fixed_entry_overhead + round_up(len(attr_name), 4) +
	 round_up(len(value), 4))
For 4k blocksizes, most of the attributes have an attr_name of "attribute_NN" which is 8, and "value_NN" which is 12.
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 12 instead of 8, and round_up(len(value, 4)) becomes 16 instead of 12. So:
	max_attrs = (block_size - 32) / (16 + 12 + 16)
or
	max_attrs = (block_size - 32) / 44

Quoted 5 lines

instead of:

	max_attrs = (block_size - 32) / (16 + 8 + 12)
or
	max_attrs = (block_size - 32) / 36

Thanks for the indepth details. Yes, in my testing as well it was coming to be around ~44%. But to be safe I chose 50%.
I verified from the code as well. We do have 32 bytes of overhead per block for the the header. And per entry overhead of 16 bytes. The rounding happens for both name (EXT4_XATTR_LEN) and value (EXT4_XATTR_SIZE) of attr.

Perhaps, it will be helpful if we update above info in ext4 documentation as well. In that the rounding off is only mentioned for value and not for name length.

Quoted 4 lines

So special casing things for block sizes > 4k may very well make sense. Perhaps it's even worth it to put in an ext[234] specific, exalc calculation for MAX_ATTRS in common/attr.

yes, will make these changes for ext[234] specific in common/attr.

-ritesh