Skip to contentSkip to conversation

xfstests: 64K blocksize related fixes

PATCH In review Ritesh Harjani 2 participants 17 messages Jul 21, 2021 – Aug 8, 2021

lore
Share & tools
lore

Ritesh Harjani

author
#0 lore raw

v2 cover xfstests: 64K blocksize related fixes

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

author
#1 lore raw

PATCH v2 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 3c9a8486..773bcb03 100755
@@ -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
Signature · 1 line
2.31.1
author
#2 lore raw

PATCH v2 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

Reviewed-by Darrick J. Wong <[email protected]>
Signed-off-by Ritesh Harjani <[email protected]>
---
 tests/ext4/027 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tests/ext4/027 +2 -2
index 84e11a29..5bcb2d55 100755
@@ -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
Signature · 1 line
2.31.1
author
#3 lore raw

PATCH v2 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.

Reviewed-by Theodore Ts'o <[email protected]>
Signed-off-by Ritesh Harjani <[email protected]>
---
 tests/ext4/306 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tests/ext4/306 +4 -1
index 4a339570..2ff88537 100755
@@ -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
Signature · 1 line
2.31.1
author
#4 lore raw

PATCH v2 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.

Reviewed-by Theodore Ts'o <[email protected]>
Signed-off-by Ritesh Harjani <[email protected]>
---
 tests/ext4/022 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
tests/ext4/022 +5 -2
index fdc19d93..321050b3 100755
@@ -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
Signature · 1 line
2.31.1
author
#5 lore raw

PATCH v2 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     | 14 +++++++++-----
 tests/generic/031.out | 16 ++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)
tests/generic/031 +9 -5
index 313ce9ff..11961c54 100755
@@ -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
-
tests/generic/031.out +8 -8
index 194bfa45..7dfcfe41 100644
@@ -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
Signature · 1 line
2.31.1
#6 lore raw

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

Quoted 19 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     | 14 +++++++++-----
 tests/generic/031.out | 16 ++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)
tests/generic/031 +1 -0
index 313ce9ff..11961c54 100755
@@ -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

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

Quoted 10 lines
tests/generic/031 +5 -4
+# 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

Quoted 45 lines
tests/generic/031 +3 -1
+	-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
-
tests/generic/031.out +8 -8
index 194bfa45..7dfcfe41 100644
@@ -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
Signature · 1 line
2.31.1
author
#7 lore raw

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

Quoted 19 lines · Eryu Guan
Quoted 18 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     | 14 +++++++++-----
 tests/generic/031.out | 16 ++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)
tests/generic/031 +0 -0
index 313ce9ff..11961c54 100755
@@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
Quoted 13 lines

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

Quoted 11 lines
tests/generic/031 +6 -4
+# 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.

Quoted 1 line

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

Quoted 4 lines

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.

Quoted 49 lines

Thanks,
Eryu

Quoted 45 lines
tests/generic/031 +3 -1
+	-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
-
tests/generic/031.out +8 -9
index 194bfa45..7dfcfe41 100644
@@ -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

#8 lore raw

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

Quoted 37 lines · Ritesh Harjani
Quoted 19 lines · Eryu Guan
Quoted 18 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     | 14 +++++++++-----
 tests/generic/031.out | 16 ++++++++--------
 2 files changed, 17 insertions(+), 13 deletions(-)
tests/generic/031 +0 -0
index 313ce9ff..11961c54 100755
@@ -26,11 +26,16 @@ testfile=$SCRATCH_MNT/testfile
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
Quoted 13 lines

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

Quoted 11 lines
tests/generic/031 +6 -4
+# 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.

Quoted 8 lines
Quoted 1 line

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

Quoted 9 lines
Quoted 6 lines

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

author
#9 lore raw

PATCH v2 6/9 common/rc: Add _mkfs_dev_blocksized functionality

In reply to Ritesh Harjani

This adds _mkfs_dev_blocksized functionality.

Signed-off-by Ritesh Harjani <[email protected]>
---
 common/rc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
common/rc +47 -0
index d4b1f21f..b5fe5c71 100644
@@ -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()
 {
Signature · 1 line
2.31.1
author
#10 lore raw

PATCH v2 7/9 generic/620: Use _mkfs_dev_blocksized to use 4k bs

In reply to 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 <[email protected]>
---
 tests/generic/620 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tests/generic/620 +3 -1
index b052376f..444e682d 100755
@@ -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
Signature · 1 line
2.31.1
#11 lore raw

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

Quoted 17 lines · 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 <[email protected]>
---
 tests/generic/620 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tests/generic/620 +0 -0
index b052376f..444e682d 100755

tests/generic/620 sectors=$((2*1024*1024*1024*17)) · line 47

Quoted 7 lines
tests/generic/620 +3 -1
@@ -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

Quoted 5 lines
tests/generic/620 +0 -0
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
 testfile=$SCRATCH_MNT/testfile-$seq
Signature · 1 line
2.31.1
author
#12 lore raw

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

Quoted 20 lines · Eryu Guan
Quoted 19 lines · 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 <[email protected]>
---
 tests/generic/620 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tests/generic/620 +0 -0
index b052376f..444e682d 100755
@@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
 chunk_size=128
Quoted 9 lines

tests/generic/620 sectors=$((2*1024*1024*1024*17)) · line 47

Quoted 5 lines
tests/generic/620 +3 -1
 _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

Quoted 8 lines

Thanks,
Eryu

Quoted 5 lines
tests/generic/620 +0 -1
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
 testfile=$SCRATCH_MNT/testfile-$seq
 
--

2.31.1

#13 lore raw

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

Quoted 5 lines · Ritesh Harjani
Quoted 4 lines · Eryu Guan
Quoted 3 lines · 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.

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

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

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

Quoted 23 lines
Quoted 13 lines
Quoted 13 lines
Signed-off-by Ritesh Harjani <[email protected]>
---
 tests/generic/620 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tests/generic/620 +0 -0
index b052376f..444e682d 100755
@@ -42,7 +42,9 @@ sectors=$((2*1024*1024*1024*17))
 chunk_size=128
Quoted 9 lines

tests/generic/620 sectors=$((2*1024*1024*1024*17)) · line 47

Quoted 5 lines
tests/generic/620 +3 -1
 _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.

Quoted 6 lines

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

Quoted 11 lines

-ritesh

Quoted 8 lines

Thanks,
Eryu

Quoted 5 lines
tests/generic/620 +0 -1
 _mount $DMHUGEDISK_DEV $SCRATCH_MNT || _fail "mount failed for $DMHUGEDISK_DEV $SCRATCH_MNT"
 testfile=$SCRATCH_MNT/testfile-$seq
 
--

2.31.1

author
#14 lore raw

PATCH v2 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.

Reviewed-by Theodore Ts'o <[email protected]>
Reviewed-by Darrick J. Wong <[email protected]>
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
author
#15 lore raw

PATCH v2 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. 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 <[email protected]>
Signed-off-by Ritesh Harjani <[email protected]>
---
 common/attr | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
common/attr +39 -0
index d3902346..35682d7c 100644
@@ -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`
Signature · 1 line
2.31.1
#16 lore raw
In reply to Ritesh Harjani
Quoted 8 lines · 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]

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

Thanks,
Eryu

Quoted 27 lines

[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