Re: [RFC,2/2] xfstests: common/rc: add cluster size support for ext4

Re: [RFC,2/2] xfstests: common/rc: add cluster size support for ext4

[JeffleXu]

Sorry for digging this really old post [1]. The overall background is
that, @offset and @len need to be aligned with cluster size when doing
fallocate(), or several xfstests cases calling fsx will fail if the
tested filesystem enabling 'bigalloc' feature.

On April 27, 2020, 5:33 p.m. UTC Darrick J. Wong wrote:

> On Fri, Apr 24, 2020 at 05:33:50PM +0800, Jeffle Xu wrote:
>> Inserting and collapsing range on ext4 with 'bigalloc' feature will
>> fail due to the offset and size should be alligned with the cluster
>> size.
>> 
>> The previous patch has add support for cluster size in fsx. Detect and
>> pass the cluster size parameter to fsx if the underlying filesystem
>> is ext4 with bigalloc.
>> 
>> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
>> ---
>>  common/rc | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/common/rc b/common/rc
>> index 2000bd9..71dde5f 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3908,6 +3908,15 @@ run_fsx()
>>  {
>>  	echo fsx $@
>>  	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
>> +
>> +	if [ "$FSTYP" == "ext4" ]; then
>> +		local cluster_size=$(tune2fs -l $TEST_DEV | grep 'Cluster size' | awk '{print $3}')
>> +		if [ -n $cluster_size ]; then
>> +			echo "cluster size: $cluster_size"
>> +			args="$args -u $cluster_size"
>> +		fi
>> +	fi
> 
> Computing the file allocation block size ought to be a separate helper.
> 
> I wonder if there's a standard way to report cluster sizes, seeing as
> fat, ext4, ocfs2, and xfs can all have minimum space allocation units
> that are larger than the base fs block size.

In fact only for insert_range and collapse range of ext4 and xfs (in
realtime mode), @offset and @len need to be aligned with cluster size.

Though fat and ocfs2 also support cluster size, ocfs2 only supports
preallocate and punch_hole, and fat only supports preallocate, in which
case @offset and @len needn't be aligned with cluster size.


So we need to align @offset and @len with cluster size only for ext4 and
xfs (in realtime mode) at a minimum cost, to fix this issue. But the
question is, there's no standard programming interface exporting cluster
size. For both ext4 and xfs, it's stored as a binary data in disk
version superblock, e.g., tune2fs could detect the cluster size of ext4.


Any idea on how to query the cluster size?


[1]
https://patchwork.kernel.org/project/fstests/cover/1587720830-11955-1-git-send-email-jefflexu@linux.alibaba.com/

-- 
Thanks,
Jeffle

Re: [RFC,2/2] xfstests: common/rc: add cluster size support for ext4

[Darrick J. Wong]

On Mon, Jul 05, 2021 at 02:58:51PM +0800, JeffleXu wrote:
> 
> Sorry for digging this really old post [1]. The overall background is
> that, @offset and @len need to be aligned with cluster size when doing
> fallocate(), or several xfstests cases calling fsx will fail if the
> tested filesystem enabling 'bigalloc' feature.
> 
> On April 27, 2020, 5:33 p.m. UTC Darrick J. Wong wrote:
> 
> > On Fri, Apr 24, 2020 at 05:33:50PM +0800, Jeffle Xu wrote:
> >> Inserting and collapsing range on ext4 with 'bigalloc' feature will
> >> fail due to the offset and size should be alligned with the cluster
> >> size.
> >> 
> >> The previous patch has add support for cluster size in fsx. Detect and
> >> pass the cluster size parameter to fsx if the underlying filesystem
> >> is ext4 with bigalloc.
> >> 
> >> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> >> ---
> >>  common/rc | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >> 
> >> diff --git a/common/rc b/common/rc
> >> index 2000bd9..71dde5f 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -3908,6 +3908,15 @@ run_fsx()
> >>  {
> >>  	echo fsx $@
> >>  	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> >> +
> >> +	if [ "$FSTYP" == "ext4" ]; then
> >> +		local cluster_size=$(tune2fs -l $TEST_DEV | grep 'Cluster size' | awk '{print $3}')
> >> +		if [ -n $cluster_size ]; then
> >> +			echo "cluster size: $cluster_size"
> >> +			args="$args -u $cluster_size"
> >> +		fi
> >> +	fi
> > 
> > Computing the file allocation block size ought to be a separate helper.
> > 
> > I wonder if there's a standard way to report cluster sizes, seeing as
> > fat, ext4, ocfs2, and xfs can all have minimum space allocation units
> > that are larger than the base fs block size.
> 
> In fact only for insert_range and collapse range of ext4 and xfs (in
> realtime mode), @offset and @len need to be aligned with cluster size.
> 
> Though fat and ocfs2 also support cluster size, ocfs2 only supports
> preallocate and punch_hole, and fat only supports preallocate, in which
> case @offset and @len needn't be aligned with cluster size.
> 
> 
> So we need to align @offset and @len with cluster size only for ext4 and
> xfs (in realtime mode) at a minimum cost, to fix this issue. But the
> question is, there's no standard programming interface exporting cluster
> size. For both ext4 and xfs, it's stored as a binary data in disk
> version superblock, e.g., tune2fs could detect the cluster size of ext4.
> 
> 
> Any idea on how to query the cluster size?

xfs and ocfs2 return the rt extent size in stat.st_blksize.

In fstestsland you could use _get_file_block_size to figure out the
allocation unit.

Alternately, I've been testing a more permanent fix for the blocksize
issues[1]; perhaps that will fix the problem?

(Note that the series is at the end of my dev tree, so it's likely to
have apply errors for the tests that exist in djwong-dev but aren't
upstream.)

--D

[1] https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfstests-dev.git/log/?h=check-blocksize-congruency

> 
> 
> [1]
> https://patchwork.kernel.org/project/fstests/cover/1587720830-11955-1-git-send-email-jefflexu@linux.alibaba.com/
> 
> -- 
> Thanks,
> Jeffle

[RFC PATCH] misc: skip fsx tests when op length not congruent with file allocation unit

[Jeffle Xu]

Yes it works. For example in this RFC patch, generic/075 could be
skipped once 'bigalloc' is detected for ext4.

I could fix all related test cases in this way, if it is acceptable to
the community.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 common/rc         | 20 ++++++++++++++++++++
 tests/generic/075 |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/common/rc b/common/rc
index f58a542..92245e0 100644
--- a/common/rc
+++ b/common/rc
@@ -4041,6 +4041,23 @@ _sysfs_dev()
 	min=$(echo "ibase=16; $min" | bc)
 	echo /sys/dev/block/$maj:$min
 }
+		
+_ext4_get_file_block_size()
+{
+	local path="$1"
+	path="$(readlink -m "$path")"
+	local dev=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $1}'`
+	
+	# The alloc unit size equals block size for ext4 without 'bigalloc'.
+	if ! (tune2fs -l $dev | grep -q 'Cluster size'); then
+		_get_block_size "$path"
+		return
+	fi
+
+	# Otherwise, call tune2fs acquiring cluster size.
+	tune2fs -l $dev | sed -n -e 's/^.*Cluster size:\s*\([0-9]*\).*$/\1/p'
+
+}
 
 # Get the minimum block size of a file.  Usually this is the
 # minimum fs block size, but some filesystems (ocfs2) do block
@@ -4059,6 +4076,9 @@ _get_file_block_size()
 	"xfs")
 		_xfs_get_file_block_size $1
 		;;
+	"ext4")
+		_ext4_get_file_block_size $1
+		;;
 	*)
 		_get_block_size $1
 		;;
diff --git a/tests/generic/075 b/tests/generic/075
index 7467bb7..d269a7c 100755
--- a/tests/generic/075
+++ b/tests/generic/075
@@ -109,6 +109,9 @@ _process_args()
 _supported_fs generic
 _require_test
 
+blksz=$(_get_block_size $TEST_DIR)
+_require_congruent_file_oplen $TEST_DIR $blksz
+
 size10=`expr 10 \* 1024 \* 1024`	# 10 megabytes
 filelen=$size10
 numops1=1000
-- 
1.8.3.1

Re: [RFC PATCH] misc: skip fsx tests when op length not congruent with file allocation unit

[Eryu Guan]

On Wed, Jul 07, 2021 at 06:47:45PM +0800, Jeffle Xu wrote:
> 
> Yes it works. For example in this RFC patch, generic/075 could be
> skipped once 'bigalloc' is detected for ext4.
> 
> I could fix all related test cases in this way, if it is acceptable to
> the community.

Looks fine to me.

> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>  common/rc         | 20 ++++++++++++++++++++
>  tests/generic/075 |  3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index f58a542..92245e0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4041,6 +4041,23 @@ _sysfs_dev()
>  	min=$(echo "ibase=16; $min" | bc)
>  	echo /sys/dev/block/$maj:$min
>  }
> +		
> +_ext4_get_file_block_size()

Another ext4 specific helper in common/rc, perhaps it's time to create
its own file for ext4, i.e. common/ext4, and move all ext4 specific
helpers in common/rc there.

> +{
> +	local path="$1"
> +	path="$(readlink -m "$path")"
> +	local dev=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $1}'`

$dir is not defined, you mean $path here?

> +	
> +	# The alloc unit size equals block size for ext4 without 'bigalloc'.
> +	if ! (tune2fs -l $dev | grep -q 'Cluster size'); then

$TUNE2FS_PROG

Perhaps you could call tune2fs only once and save its "Cluster size"
line for later use.

Thanks,
Eryu

> +		_get_block_size "$path"
> +		return
> +	fi
> +
> +	# Otherwise, call tune2fs acquiring cluster size.
> +	tune2fs -l $dev | sed -n -e 's/^.*Cluster size:\s*\([0-9]*\).*$/\1/p'
> +
> +}
>  
>  # Get the minimum block size of a file.  Usually this is the
>  # minimum fs block size, but some filesystems (ocfs2) do block
> @@ -4059,6 +4076,9 @@ _get_file_block_size()
>  	"xfs")
>  		_xfs_get_file_block_size $1
>  		;;
> +	"ext4")
> +		_ext4_get_file_block_size $1
> +		;;
>  	*)
>  		_get_block_size $1
>  		;;
> diff --git a/tests/generic/075 b/tests/generic/075
> index 7467bb7..d269a7c 100755
> --- a/tests/generic/075
> +++ b/tests/generic/075
> @@ -109,6 +109,9 @@ _process_args()
>  _supported_fs generic
>  _require_test
>  
> +blksz=$(_get_block_size $TEST_DIR)
> +_require_congruent_file_oplen $TEST_DIR $blksz
> +
>  size10=`expr 10 \* 1024 \* 1024`	# 10 megabytes
>  filelen=$size10
>  numops1=1000
> -- 
> 1.8.3.1