LinuxLists.cc - [PATCH 0/9] 64K blocksize related fixes

[permalink] [raw]

Subject: [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(-)

diff --git a/tests/ext4/003 b/tests/ext4/003
index 00ea9150..1ddb3063 100755
--- a/tests/ext4/003
+++ b/tests/ext4/003
@@ -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

--
2.31.1

2021-06-14 06:29:37

[permalink] [raw]

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

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(-)

diff --git a/tests/ext4/027 b/tests/ext4/027
index 97c14cf5..83d5a413 100755
--- a/tests/ext4/027
+++ b/tests/ext4/027
@@ -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
--
2.31.1

2021-06-14 06:29:57

[permalink] [raw]

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

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(-)

diff --git a/tests/ext4/306 b/tests/ext4/306
index 146fdb39..1d45a9d0 100755
--- a/tests/ext4/306
+++ b/tests/ext4/306
@@ -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
--
2.31.1

2021-06-14 06:30:05

[permalink] [raw]

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

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(-)

diff --git a/tests/ext4/022 b/tests/ext4/022
index 3de7619a..ca58f34e 100755
--- a/tests/ext4/022
+++ b/tests/ext4/022
@@ -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
--
2.31.1

2021-06-14 06:30:14

[permalink] [raw]

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

Add 031.out file to .gitignore

Signed-off-by: Ritesh Harjani <[email protected]>
---
.gitignore | 1 +
1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index d3194e76..7a3be5e3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -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
--
2.31.1

2021-06-14 06:30:16

[permalink] [raw]

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

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(+)

diff --git a/tests/generic/620 b/tests/generic/620
index 60559441..3ccda5e4 100755
--- a/tests/generic/620
+++ b/tests/generic/620
@@ -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"
--
2.31.1

2021-06-14 06:30:30

[permalink] [raw]

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

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(-)

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

2021-06-14 06:30:38

[permalink] [raw]

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

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(-)

diff --git a/common/attr b/common/attr
index d3902346..e8661d80 100644
--- a/common/attr
+++ b/common/attr
@@ -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
--
2.31.1

2021-06-14 06:31:40

[permalink] [raw]

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

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%)

diff --git a/tests/generic/031 b/tests/generic/031
index db84031b..40cb23af 100755
--- a/tests/generic/031
+++ b/tests/generic/031
@@ -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
-
diff --git a/tests/generic/031.out.64k b/tests/generic/031.out.64k
new file mode 100644
index 00000000..7dfcfe41
--- /dev/null
+++ b/tests/generic/031.out.64k
@@ -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
diff --git a/tests/generic/031.out b/tests/generic/031.out.default
similarity index 100%
rename from tests/generic/031.out
rename to tests/generic/031.out.default
--
2.31.1

2021-06-14 16:42:45

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:06AM +0530, Ritesh Harjani wrote:
> 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(-)
>
> diff --git a/tests/ext4/027 b/tests/ext4/027
> index 97c14cf5..83d5a413 100755
> --- a/tests/ext4/027
> +++ b/tests/ext4/027
> @@ -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

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

2021-06-30 13:31:27

[permalink] [raw]

Subject: Re: [PATCH 0/9] 64K blocksize related fixes

On 21/06/14 11:58AM, Ritesh Harjani wrote:
> 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

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

2021-06-30 15:50:45

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:09AM +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 <[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%)
>
> diff --git a/tests/generic/031 b/tests/generic/031
> index db84031b..40cb23af 100755
> --- a/tests/generic/031
> +++ b/tests/generic/031
> @@ -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

What if the blocksize is 32k?

--D

> + $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
> -
> diff --git a/tests/generic/031.out.64k b/tests/generic/031.out.64k
> new file mode 100644
> index 00000000..7dfcfe41
> --- /dev/null
> +++ b/tests/generic/031.out.64k
> @@ -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
> diff --git a/tests/generic/031.out b/tests/generic/031.out.default
> similarity index 100%
> rename from tests/generic/031.out
> rename to tests/generic/031.out.default
> --
> 2.31.1
>

2021-06-30 15:51:25

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:12AM +0530, Ritesh Harjani wrote:
> 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

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

2021-06-30 15:52:32

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:13AM +0530, Ritesh Harjani wrote:
> 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(-)
>
> diff --git a/common/attr b/common/attr
> index d3902346..e8661d80 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -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

> esac
>
> export MAX_ATTRS
> --
> 2.31.1
>

2021-06-30 16:27:58

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:05AM +0530, Ritesh Harjani wrote:
> 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(-)
>
> diff --git a/tests/ext4/003 b/tests/ext4/003
> index 00ea9150..1ddb3063 100755
> --- a/tests/ext4/003
> +++ b/tests/ext4/003
> @@ -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

2021-06-30 16:30:18

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:07AM +0530, Ritesh Harjani wrote:
> 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

2021-06-30 16:37:02

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:08AM +0530, Ritesh Harjani wrote:
> 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]>

2021-06-30 16:37:22

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:10AM +0530, Ritesh Harjani wrote:
> Add 031.out file to .gitignore
>
> Signed-off-by: Ritesh Harjani <[email protected]>

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

2021-06-30 17:08:18

[permalink] [raw]

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

On Mon, Jun 14, 2021 at 11:58:11AM +0530, Ritesh Harjani wrote:
> 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

2021-06-30 17:20:31

[permalink] [raw]

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

On Wed, Jun 30, 2021 at 08:50:01AM -0700, Darrick J. Wong wrote:
> > +# 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

2021-06-30 17:21:04

[permalink] [raw]

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

2021-06-30 19:21:37

[permalink] [raw]

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

On Wed, Jun 30, 2021 at 08:51:50AM -0700, Darrick J. Wong wrote:
> On Mon, Jun 14, 2021 at 11:58:13AM +0530, Ritesh Harjani wrote:
> > 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(-)
> >
> > diff --git a/common/attr b/common/attr
> > index d3902346..e8661d80 100644
> > --- a/common/attr
> > +++ b/common/attr
> > @@ -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

2021-07-08 06:27:02

[permalink] [raw]

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

On 21/06/30 12:27PM, Theodore Ts'o wrote:
> On Mon, Jun 14, 2021 at 11:58:05AM +0530, Ritesh Harjani wrote:
> > 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(-)
> >
> > diff --git a/tests/ext4/003 b/tests/ext4/003
> > index 00ea9150..1ddb3063 100755
> > --- a/tests/ext4/003
> > +++ b/tests/ext4/003
> > @@ -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!

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

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

>
> - Ted

2021-07-08 09:51:03

[permalink] [raw]

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

On 21/06/30 01:18PM, Theodore Ts'o wrote:
> On Wed, Jun 30, 2021 at 08:50:01AM -0700, Darrick J. Wong wrote:
> > > +# 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.
>

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

2021-07-08 10:01:58

[permalink] [raw]

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

On 21/06/30 01:07PM, Theodore Ts'o wrote:
> On Mon, Jun 14, 2021 at 11:58:11AM +0530, Ritesh Harjani wrote:
> > 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.

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

2021-07-08 12:52:03

[permalink] [raw]

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

On Thu, Jul 08, 2021 at 11:54:45AM +0530, Ritesh Harjani wrote:
> 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

2021-07-09 05:13:03