diff mbox series

[2/2] xfs/157,xfs/547,xfs/548: switch to using _scratch_mkfs_sized

Message ID 20241008105055.11928-3-hans.holmberg@wdc.com (mailing list archive)
State New
Headers show
Series _scratch_mkfs_sized fixes for xfs rt devices | expand

Commit Message

Hans Holmberg Oct. 8, 2024, 10:52 a.m. UTC
From: Hans Holmberg <Hans.Holmberg@wdc.com>

These test cases specify small -d sizes which combined with a rt dev of
unrestricted size and the rtrmap feature can cause mkfs to fail with
error:

mkfs.xfs: cannot handle expansion of realtime rmap btree; need <x> free
blocks, have <y>

This is due to that the -d size is not big enough to support the
metadata space allocation required for the rt groups.

Switch to use _scratch_mkfs_sized that sets up the -r size parameter
to avoid this. If -r size=x and -d size=x we will not risk running
out of space on the ddev as the metadata size is just a fraction of
the rt data size.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/157 | 12 ++++++++----
 tests/xfs/547 |  4 +++-
 tests/xfs/548 |  2 +-
 3 files changed, 12 insertions(+), 6 deletions(-)

Comments

Darrick J. Wong Oct. 8, 2024, 5:11 p.m. UTC | #1
On Tue, Oct 08, 2024 at 10:52:04AM +0000, Hans Holmberg wrote:
> From: Hans Holmberg <Hans.Holmberg@wdc.com>
> 
> These test cases specify small -d sizes which combined with a rt dev of
> unrestricted size and the rtrmap feature can cause mkfs to fail with
> error:
> 
> mkfs.xfs: cannot handle expansion of realtime rmap btree; need <x> free
> blocks, have <y>
> 
> This is due to that the -d size is not big enough to support the
> metadata space allocation required for the rt groups.
> 
> Switch to use _scratch_mkfs_sized that sets up the -r size parameter
> to avoid this. If -r size=x and -d size=x we will not risk running
> out of space on the ddev as the metadata size is just a fraction of
> the rt data size.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/xfs/157 | 12 ++++++++----
>  tests/xfs/547 |  4 +++-
>  tests/xfs/548 |  2 +-
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/xfs/157 b/tests/xfs/157
> index 79d45ac2bb34..9b5badbaeb3c 100755
> --- a/tests/xfs/157
> +++ b/tests/xfs/157
> @@ -34,18 +34,21 @@ _require_test
>  _require_scratch_nocheck
>  _require_command "$XFS_ADMIN_PROG" "xfs_admin"
>  
> +
>  # Create some fake sparse files for testing external devices and whatnot
> +fs_size=$((500 * 1024 * 1024))
> +
>  fake_datafile=$TEST_DIR/$seq.scratch.data
>  rm -f $fake_datafile
> -truncate -s 500m $fake_datafile
> +truncate -s $fs_size $fake_datafile
>  
>  fake_logfile=$TEST_DIR/$seq.scratch.log
>  rm -f $fake_logfile
> -truncate -s 500m $fake_logfile
> +truncate -s $fs_size $fake_logfile
>  
>  fake_rtfile=$TEST_DIR/$seq.scratch.rt
>  rm -f $fake_rtfile
> -truncate -s 500m $fake_rtfile
> +truncate -s $fs_size $fake_rtfile
>  
>  # Save the original variables
>  orig_ddev=$SCRATCH_DEV
> @@ -63,7 +66,8 @@ scenario() {
>  }
>  
>  check_label() {
> -	_scratch_mkfs -L oldlabel >> $seqres.full
> +	MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
> +		>> $seqres.full

I was surprised that this was necessary until I remembered that this
test checks various *combinations* of block devices and 500M files.
The block device can be quite large, so that's why you want
_scratch_mkfs_sized to force the size of both sections to 500M.
Heh, oops.  My bad, I should have caught that. :(

>  	_scratch_xfs_db -c label
>  	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
>  	_scratch_xfs_db -c label
> diff --git a/tests/xfs/547 b/tests/xfs/547
> index eada4aadc27f..ffac546be4cd 100755
> --- a/tests/xfs/547
> +++ b/tests/xfs/547
> @@ -24,10 +24,12 @@ _require_xfs_db_command path
>  _require_test_program "punch-alternating"
>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>  
> +fs_size=$((512 * 1024 * 1024))
> +
>  for nrext64 in 0 1; do
>  	echo "* Verify extent counter fields with nrext64=${nrext64} option"
>  
> -	_scratch_mkfs -i nrext64=${nrext64} -d size=$((512 * 1024 * 1024)) \
> +	MKFS_OPTIONS="-i nrext64=${nrext64} $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
>  		      >> $seqres.full
>  	_scratch_mount >> $seqres.full
>  
> diff --git a/tests/xfs/548 b/tests/xfs/548
> index f0b58563e64d..af72885a9c6e 100755
> --- a/tests/xfs/548
> +++ b/tests/xfs/548
> @@ -24,7 +24,7 @@ _require_xfs_db_command path
>  _require_test_program "punch-alternating"
>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>  
> -_scratch_mkfs -d size=$((512 * 1024 * 1024)) >> $seqres.full
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full

These other two are pretty self evident so
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  _scratch_mount >> $seqres.full
>  
>  bsize=$(_get_file_block_size $SCRATCH_MNT)
> -- 
> 2.34.1
>
Hans Holmberg Oct. 9, 2024, 6:45 a.m. UTC | #2
On 08/10/2024 19:11, Darrick J. Wong wrote:
> On Tue, Oct 08, 2024 at 10:52:04AM +0000, Hans Holmberg wrote:
>> From: Hans Holmberg <Hans.Holmberg@wdc.com>
>>
>> These test cases specify small -d sizes which combined with a rt dev of
>> unrestricted size and the rtrmap feature can cause mkfs to fail with
>> error:
>>
>> mkfs.xfs: cannot handle expansion of realtime rmap btree; need <x> free
>> blocks, have <y>
>>
>> This is due to that the -d size is not big enough to support the
>> metadata space allocation required for the rt groups.
>>
>> Switch to use _scratch_mkfs_sized that sets up the -r size parameter
>> to avoid this. If -r size=x and -d size=x we will not risk running
>> out of space on the ddev as the metadata size is just a fraction of
>> the rt data size.
>>
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  tests/xfs/157 | 12 ++++++++----
>>  tests/xfs/547 |  4 +++-
>>  tests/xfs/548 |  2 +-
>>  3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/xfs/157 b/tests/xfs/157
>> index 79d45ac2bb34..9b5badbaeb3c 100755
>> --- a/tests/xfs/157
>> +++ b/tests/xfs/157
>> @@ -34,18 +34,21 @@ _require_test
>>  _require_scratch_nocheck
>>  _require_command "$XFS_ADMIN_PROG" "xfs_admin"
>>  
>> +
>>  # Create some fake sparse files for testing external devices and whatnot
>> +fs_size=$((500 * 1024 * 1024))
>> +
>>  fake_datafile=$TEST_DIR/$seq.scratch.data
>>  rm -f $fake_datafile
>> -truncate -s 500m $fake_datafile
>> +truncate -s $fs_size $fake_datafile
>>  
>>  fake_logfile=$TEST_DIR/$seq.scratch.log
>>  rm -f $fake_logfile
>> -truncate -s 500m $fake_logfile
>> +truncate -s $fs_size $fake_logfile
>>  
>>  fake_rtfile=$TEST_DIR/$seq.scratch.rt
>>  rm -f $fake_rtfile
>> -truncate -s 500m $fake_rtfile
>> +truncate -s $fs_size $fake_rtfile
>>  
>>  # Save the original variables
>>  orig_ddev=$SCRATCH_DEV
>> @@ -63,7 +66,8 @@ scenario() {
>>  }
>>  
>>  check_label() {
>> -	_scratch_mkfs -L oldlabel >> $seqres.full
>> +	MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
>> +		>> $seqres.full
> 
> I was surprised that this was necessary until I remembered that this
> test checks various *combinations* of block devices and 500M files.
> The block device can be quite large, so that's why you want
> _scratch_mkfs_sized to force the size of both sections to 500M.

Yeah, exactly.

> Heh, oops.  My bad, I should have caught that. :(

This was a tricky one, but the fix straight-forward :)

Thanks,
Hans


> 
>>  	_scratch_xfs_db -c label
>>  	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
>>  	_scratch_xfs_db -c label
>> diff --git a/tests/xfs/547 b/tests/xfs/547
>> index eada4aadc27f..ffac546be4cd 100755
>> --- a/tests/xfs/547
>> +++ b/tests/xfs/547
>> @@ -24,10 +24,12 @@ _require_xfs_db_command path
>>  _require_test_program "punch-alternating"
>>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>>  
>> +fs_size=$((512 * 1024 * 1024))
>> +
>>  for nrext64 in 0 1; do
>>  	echo "* Verify extent counter fields with nrext64=${nrext64} option"
>>  
>> -	_scratch_mkfs -i nrext64=${nrext64} -d size=$((512 * 1024 * 1024)) \
>> +	MKFS_OPTIONS="-i nrext64=${nrext64} $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
>>  		      >> $seqres.full
>>  	_scratch_mount >> $seqres.full
>>  
>> diff --git a/tests/xfs/548 b/tests/xfs/548
>> index f0b58563e64d..af72885a9c6e 100755
>> --- a/tests/xfs/548
>> +++ b/tests/xfs/548
>> @@ -24,7 +24,7 @@ _require_xfs_db_command path
>>  _require_test_program "punch-alternating"
>>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>>  
>> -_scratch_mkfs -d size=$((512 * 1024 * 1024)) >> $seqres.full
>> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
> 
> These other two are pretty self evident so
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> --D
> 
>>  _scratch_mount >> $seqres.full
>>  
>>  bsize=$(_get_file_block_size $SCRATCH_MNT)
>> -- 
>> 2.34.1
>>
>
Zorro Lang Oct. 10, 2024, 6:57 a.m. UTC | #3
On Tue, Oct 08, 2024 at 10:52:04AM +0000, Hans Holmberg wrote:
> From: Hans Holmberg <Hans.Holmberg@wdc.com>
> 
> These test cases specify small -d sizes which combined with a rt dev of
> unrestricted size and the rtrmap feature can cause mkfs to fail with
> error:
> 
> mkfs.xfs: cannot handle expansion of realtime rmap btree; need <x> free
> blocks, have <y>
> 
> This is due to that the -d size is not big enough to support the
> metadata space allocation required for the rt groups.
> 
> Switch to use _scratch_mkfs_sized that sets up the -r size parameter
> to avoid this. If -r size=x and -d size=x we will not risk running
> out of space on the ddev as the metadata size is just a fraction of
> the rt data size.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/xfs/157 | 12 ++++++++----
>  tests/xfs/547 |  4 +++-
>  tests/xfs/548 |  2 +-
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/xfs/157 b/tests/xfs/157
> index 79d45ac2bb34..9b5badbaeb3c 100755
> --- a/tests/xfs/157
> +++ b/tests/xfs/157
> @@ -34,18 +34,21 @@ _require_test
>  _require_scratch_nocheck
>  _require_command "$XFS_ADMIN_PROG" "xfs_admin"
>  
> +
>  # Create some fake sparse files for testing external devices and whatnot
> +fs_size=$((500 * 1024 * 1024))
> +
>  fake_datafile=$TEST_DIR/$seq.scratch.data
>  rm -f $fake_datafile
> -truncate -s 500m $fake_datafile
> +truncate -s $fs_size $fake_datafile
>  
>  fake_logfile=$TEST_DIR/$seq.scratch.log
>  rm -f $fake_logfile
> -truncate -s 500m $fake_logfile
> +truncate -s $fs_size $fake_logfile
>  
>  fake_rtfile=$TEST_DIR/$seq.scratch.rt
>  rm -f $fake_rtfile
> -truncate -s 500m $fake_rtfile
> +truncate -s $fs_size $fake_rtfile
>  
>  # Save the original variables
>  orig_ddev=$SCRATCH_DEV
> @@ -63,7 +66,8 @@ scenario() {
>  }
>  
>  check_label() {
> -	_scratch_mkfs -L oldlabel >> $seqres.full
> +	MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
> +		>> $seqres.full

Currently _scratch_mkfs_sized will _notrun if fails, but _scratch_mkfs doesn't.
So _scratch_mkfs might be similar with _try_scratch_mkfs_sized (returns fail).
But for this patch, both make sense to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  	_scratch_xfs_db -c label
>  	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
>  	_scratch_xfs_db -c label
> diff --git a/tests/xfs/547 b/tests/xfs/547
> index eada4aadc27f..ffac546be4cd 100755
> --- a/tests/xfs/547
> +++ b/tests/xfs/547
> @@ -24,10 +24,12 @@ _require_xfs_db_command path
>  _require_test_program "punch-alternating"
>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>  
> +fs_size=$((512 * 1024 * 1024))
> +
>  for nrext64 in 0 1; do
>  	echo "* Verify extent counter fields with nrext64=${nrext64} option"
>  
> -	_scratch_mkfs -i nrext64=${nrext64} -d size=$((512 * 1024 * 1024)) \
> +	MKFS_OPTIONS="-i nrext64=${nrext64} $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
>  		      >> $seqres.full
>  	_scratch_mount >> $seqres.full
>  
> diff --git a/tests/xfs/548 b/tests/xfs/548
> index f0b58563e64d..af72885a9c6e 100755
> --- a/tests/xfs/548
> +++ b/tests/xfs/548
> @@ -24,7 +24,7 @@ _require_xfs_db_command path
>  _require_test_program "punch-alternating"
>  _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
>  
> -_scratch_mkfs -d size=$((512 * 1024 * 1024)) >> $seqres.full
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
>  _scratch_mount >> $seqres.full
>  
>  bsize=$(_get_file_block_size $SCRATCH_MNT)
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/tests/xfs/157 b/tests/xfs/157
index 79d45ac2bb34..9b5badbaeb3c 100755
--- a/tests/xfs/157
+++ b/tests/xfs/157
@@ -34,18 +34,21 @@  _require_test
 _require_scratch_nocheck
 _require_command "$XFS_ADMIN_PROG" "xfs_admin"
 
+
 # Create some fake sparse files for testing external devices and whatnot
+fs_size=$((500 * 1024 * 1024))
+
 fake_datafile=$TEST_DIR/$seq.scratch.data
 rm -f $fake_datafile
-truncate -s 500m $fake_datafile
+truncate -s $fs_size $fake_datafile
 
 fake_logfile=$TEST_DIR/$seq.scratch.log
 rm -f $fake_logfile
-truncate -s 500m $fake_logfile
+truncate -s $fs_size $fake_logfile
 
 fake_rtfile=$TEST_DIR/$seq.scratch.rt
 rm -f $fake_rtfile
-truncate -s 500m $fake_rtfile
+truncate -s $fs_size $fake_rtfile
 
 # Save the original variables
 orig_ddev=$SCRATCH_DEV
@@ -63,7 +66,8 @@  scenario() {
 }
 
 check_label() {
-	_scratch_mkfs -L oldlabel >> $seqres.full
+	MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
+		>> $seqres.full
 	_scratch_xfs_db -c label
 	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
 	_scratch_xfs_db -c label
diff --git a/tests/xfs/547 b/tests/xfs/547
index eada4aadc27f..ffac546be4cd 100755
--- a/tests/xfs/547
+++ b/tests/xfs/547
@@ -24,10 +24,12 @@  _require_xfs_db_command path
 _require_test_program "punch-alternating"
 _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
 
+fs_size=$((512 * 1024 * 1024))
+
 for nrext64 in 0 1; do
 	echo "* Verify extent counter fields with nrext64=${nrext64} option"
 
-	_scratch_mkfs -i nrext64=${nrext64} -d size=$((512 * 1024 * 1024)) \
+	MKFS_OPTIONS="-i nrext64=${nrext64} $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
 		      >> $seqres.full
 	_scratch_mount >> $seqres.full
 
diff --git a/tests/xfs/548 b/tests/xfs/548
index f0b58563e64d..af72885a9c6e 100755
--- a/tests/xfs/548
+++ b/tests/xfs/548
@@ -24,7 +24,7 @@  _require_xfs_db_command path
 _require_test_program "punch-alternating"
 _require_xfs_io_error_injection "bmap_alloc_minlen_extent"
 
-_scratch_mkfs -d size=$((512 * 1024 * 1024)) >> $seqres.full
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
 _scratch_mount >> $seqres.full
 
 bsize=$(_get_file_block_size $SCRATCH_MNT)