Re: [PATCH] xfs/205,432,516: read sysfs for mkfs block size instead of hardcoded values

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Mar 02, 2022 at 03:00:22AM +0530, Nitesh Shetty wrote:
> At present block size of 1024 is hardcoded for mkfs. This creates
> problem when device block size is more than 1024. So we use sysfs values

What kinds of problems?

> of SCRATCH_DEV, if not found then default to 1024.
> 
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> ---
>  tests/xfs/205 | 3 ++-
>  tests/xfs/432 | 3 ++-
>  tests/xfs/516 | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/xfs/205 b/tests/xfs/205
> index 104f1f45..73e32c4d 100755
> --- a/tests/xfs/205
> +++ b/tests/xfs/205
> @@ -22,7 +22,8 @@ _require_scratch_nocheck
>  # bitmap consuming all the free space in our small data device.
>  unset SCRATCH_RTDEV
>  
> -fsblksz=1024
> +physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)

For a disk with 512 byte sectors, does this set physical=512?

The code within the $() really ought to be turned into a helper
_scratch_dev_phys_block_size or something.

> +fsblksz=${physical:-1024}

Filesystem blocksize isn't supposed to be smaller than the physical
blocksize, but why do you change the fs block size to the physical size?


>  _scratch_mkfs_xfs -d size=$((32768*fsblksz)) -b size=$fsblksz >> $seqres.full 2>&1
>  _scratch_mount
>  
> diff --git a/tests/xfs/432 b/tests/xfs/432
> index 86012f0b..cd6367e2 100755
> --- a/tests/xfs/432
> +++ b/tests/xfs/432
> @@ -49,7 +49,8 @@ echo "Format and mount"
>  # block.  8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
>  # dablock.  33 dirblocks * 64k mean that we can expand a directory by
>  # 2112k before we have to allocate another da btree block.
> -_scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
> +physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
> +_scratch_mkfs -b size=${physical:-1k} -n size=64k > "$seqres.full" 2>&1

This test formats a very specific geometry because it needs precise
calculations to generate a directory with 1000 consecutively mapped
blocks.  Does it still do that if the blocksize isn't 1k?

>  _scratch_mount >> "$seqres.full" 2>&1
>  
>  metadump_file="$TEST_DIR/meta-$seq"
> diff --git a/tests/xfs/516 b/tests/xfs/516
> index 9e1b9931..b9d4f0c9 100755
> --- a/tests/xfs/516
> +++ b/tests/xfs/516
> @@ -84,7 +84,8 @@ test_su_opts()
>  	local mounted=0
>  
>  	echo "Format with 256k stripe unit; 4x stripe width" >> $seqres.full
> -	_scratch_mkfs -b size=1k -d su=256k,sw=4 >> $seqres.full 2>&1
> +	physical=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/physical_block_size)
> +	_scratch_mkfs -b size=${physical:-1k} -d su=256k,sw=4 >> $seqres.full 2>&1

This is a test of sunit/swidth.  Do you need to scale those up as well?

--D

>  
>  	__test_mount_opts "$@"
>  }
> 
> base-commit: 2ea74ba4e70b546279896e2a733c8c7f4b206193
> -- 
> 2.25.1
>