[PATCH] btrfs/122: adjust nodesize to match pagesize

[PATCH] btrfs/122: adjust nodesize to match pagesize

[Anand Jain]

btrf/122 is failing on a system with 64k page size:

     QA output created by 122
    +ERROR: illegal nodesize 16384 (smaller than 65536)
    +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
    +mount /dev/vdb2 /mnt/scratch failed
    +(see /xfstests-dev/results//btrfs/122.full for details)

This test case requires the use of a 16k node size, however, it is not
possible on a system with a 64k page size. The smallest possible node size
is the page size. So, set nodesize to the system page size instead.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/btrfs/122 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/122 b/tests/btrfs/122
index 345317536f40..e7694173cc24 100755
--- a/tests/btrfs/122
+++ b/tests/btrfs/122
@@ -18,9 +18,10 @@ _supported_fs btrfs
 _require_scratch
 _require_btrfs_qgroup_report
 
-# Force a small leaf size to make it easier to blow out our root
+# Force a smallest possible leaf size to make it easier to blow out our root
 # subvolume tree
-_scratch_mkfs "--nodesize 16384" >/dev/null
+pagesize=$(get_page_size)
+_scratch_mkfs "--nodesize $pagesize" >> $seqres.full || _fail "mkfs failed"
 _scratch_mount
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 
-- 
2.38.1

Re: [PATCH] btrfs/122: adjust nodesize to match pagesize

[Zorro Lang]

On Mon, May 29, 2023 at 09:13:20PM +0800, Anand Jain wrote:
> btrf/122 is failing on a system with 64k page size:
> 
>      QA output created by 122
>     +ERROR: illegal nodesize 16384 (smaller than 65536)
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>     +mount /dev/vdb2 /mnt/scratch failed
>     +(see /xfstests-dev/results//btrfs/122.full for details)
> 
> This test case requires the use of a 16k node size, however, it is not
> possible on a system with a 64k page size. The smallest possible node size
> is the page size. So, set nodesize to the system page size instead.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  tests/btrfs/122 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/122 b/tests/btrfs/122
> index 345317536f40..e7694173cc24 100755
> --- a/tests/btrfs/122
> +++ b/tests/btrfs/122
> @@ -18,9 +18,10 @@ _supported_fs btrfs
>  _require_scratch
>  _require_btrfs_qgroup_report
>  
> -# Force a small leaf size to make it easier to blow out our root
> +# Force a smallest possible leaf size to make it easier to blow out our root
>  # subvolume tree
> -_scratch_mkfs "--nodesize 16384" >/dev/null
> +pagesize=$(get_page_size)
> +_scratch_mkfs "--nodesize $pagesize" >> $seqres.full || _fail "mkfs failed"

Will this patch change the original test target? Due to it hopes to test
nodesize=16k in 4k pagesize machine, but now it tests 4k nodesize as this
change.

How about:
  nodesize=16384
  pagesize=$(get_page_size)
  if [ $pagesize -gt $nodesize ];then
  	nodesize=$pagesize
  fi
  _scratch_mkfs "--nodesize $nodesize" ...

Or
  pagesize=$(get_page_size)
  nodesize=$((4 * pagesize))
  if [ $nodesize -gt 65536 ];then
      nodesize=65536
  fi
  _scratch_mkfs "--nodesize $nodesize" ...

Thanks,
Zorro


>  _scratch_mount
>  _run_btrfs_util_prog quota enable $SCRATCH_MNT
>  
> -- 
> 2.38.1
> 

Re: [PATCH] btrfs/122: adjust nodesize to match pagesize

[Anand Jain]

On 01/06/2023 17:32, Zorro Lang wrote:
> On Mon, May 29, 2023 at 09:13:20PM +0800, Anand Jain wrote:
>> btrf/122 is failing on a system with 64k page size:
>>
>>       QA output created by 122
>>      +ERROR: illegal nodesize 16384 (smaller than 65536)
>>      +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>>      +mount /dev/vdb2 /mnt/scratch failed
>>      +(see /xfstests-dev/results//btrfs/122.full for details)
>>
>> This test case requires the use of a 16k node size, however, it is not
>> possible on a system with a 64k page size. The smallest possible node size
>> is the page size. So, set nodesize to the system page size instead.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   tests/btrfs/122 | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/btrfs/122 b/tests/btrfs/122
>> index 345317536f40..e7694173cc24 100755
>> --- a/tests/btrfs/122
>> +++ b/tests/btrfs/122
>> @@ -18,9 +18,10 @@ _supported_fs btrfs
>>   _require_scratch
>>   _require_btrfs_qgroup_report
>>   
>> -# Force a small leaf size to make it easier to blow out our root
>> +# Force a smallest possible leaf size to make it easier to blow out our root
>>   # subvolume tree
>> -_scratch_mkfs "--nodesize 16384" >/dev/null
>> +pagesize=$(get_page_size)
>> +_scratch_mkfs "--nodesize $pagesize" >> $seqres.full || _fail "mkfs failed"
> 
> Will this patch change the original test target? Due to it hopes to test
> nodesize=16k in 4k pagesize machine, but now it tests 4k nodesize as this
> change.
> 
> How about:
>    nodesize=16384
>    pagesize=$(get_page_size)
>    if [ $pagesize -gt $nodesize ];then
>    	nodesize=$pagesize
>    fi
>    _scratch_mkfs "--nodesize $nodesize" ...
> 
> Or
>    pagesize=$(get_page_size)
>    nodesize=$((4 * pagesize))
>    if [ $nodesize -gt 65536 ];then
>        nodesize=65536
>    fi
>    _scratch_mkfs "--nodesize $nodesize" ...
> 

Thanks for the review. Originally, the test case sets a 16K node size,
which is also the default node size. In fact, it would be better to
remove the nodesize option altogether. I'll send v2.

- Anand


> Thanks,
> Zorro
> 
> 
>>   _scratch_mount
>>   _run_btrfs_util_prog quota enable $SCRATCH_MNT
>>   
>> -- 
>> 2.38.1
>>
> 

[PATCH v2] btrfs/122: fix nodesize option in mfks.btrfs

[Anand Jain]

btrf/122 is failing on a system with 64k page size:

     QA output created by 122
    +ERROR: illegal nodesize 16384 (smaller than 65536)
    +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
    +mount /dev/vdb2 /mnt/scratch failed
    +(see /xfstests-dev/results//btrfs/122.full for details)

Mkfs.btrfs sets the default node size to 16K when the sector size is less
than 16K, and it matches the sector size when it's greater than 16K.
So, there's no need to explicitly set it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Remove the redundant explicit nodesize option from mkfs.btrfs.
    Changed: Title from "btrfs/122: adjust nodesize to match pagesize"
    

 tests/btrfs/122 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/btrfs/122 b/tests/btrfs/122
index 345317536f40..9d5e9efccec7 100755
--- a/tests/btrfs/122
+++ b/tests/btrfs/122
@@ -18,9 +18,7 @@ _supported_fs btrfs
 _require_scratch
 _require_btrfs_qgroup_report
 
-# Force a small leaf size to make it easier to blow out our root
-# subvolume tree
-_scratch_mkfs "--nodesize 16384" >/dev/null
+_scratch_mkfs >> $seqres.full || _fail "mkfs failed"
 _scratch_mount
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 
-- 
2.38.1

Re: [PATCH v2] btrfs/122: fix nodesize option in mfks.btrfs

[Zorro Lang]

On Fri, Jun 02, 2023 at 07:38:54PM +0800, Anand Jain wrote:
> btrf/122 is failing on a system with 64k page size:
> 
>      QA output created by 122
>     +ERROR: illegal nodesize 16384 (smaller than 65536)
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>     +mount /dev/vdb2 /mnt/scratch failed
>     +(see /xfstests-dev/results//btrfs/122.full for details)
> 
> Mkfs.btrfs sets the default node size to 16K when the sector size is less
> than 16K, and it matches the sector size when it's greater than 16K.
> So, there's no need to explicitly set it.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2: Remove the redundant explicit nodesize option from mkfs.btrfs.
>     Changed: Title from "btrfs/122: adjust nodesize to match pagesize"

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

>     
> 
>  tests/btrfs/122 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/btrfs/122 b/tests/btrfs/122
> index 345317536f40..9d5e9efccec7 100755
> --- a/tests/btrfs/122
> +++ b/tests/btrfs/122
> @@ -18,9 +18,7 @@ _supported_fs btrfs
>  _require_scratch
>  _require_btrfs_qgroup_report
>  
> -# Force a small leaf size to make it easier to blow out our root
> -# subvolume tree
> -_scratch_mkfs "--nodesize 16384" >/dev/null
> +_scratch_mkfs >> $seqres.full || _fail "mkfs failed"
>  _scratch_mount
>  _run_btrfs_util_prog quota enable $SCRATCH_MNT
>  
> -- 
> 2.38.1
> 

Re: [PATCH v2] btrfs/122: fix nodesize option in mfks.btrfs

[Zorro Lang]

On Fri, Jun 02, 2023 at 07:38:54PM +0800, Anand Jain wrote:
> btrf/122 is failing on a system with 64k page size:
> 
>      QA output created by 122
>     +ERROR: illegal nodesize 16384 (smaller than 65536)
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>     +mount /dev/vdb2 /mnt/scratch failed
>     +(see /xfstests-dev/results//btrfs/122.full for details)
> 
> Mkfs.btrfs sets the default node size to 16K when the sector size is less
> than 16K, and it matches the sector size when it's greater than 16K.
> So, there's no need to explicitly set it.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2: Remove the redundant explicit nodesize option from mkfs.btrfs.
>     Changed: Title from "btrfs/122: adjust nodesize to match pagesize"
>     
> 
>  tests/btrfs/122 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/btrfs/122 b/tests/btrfs/122
> index 345317536f40..9d5e9efccec7 100755
> --- a/tests/btrfs/122
> +++ b/tests/btrfs/122
> @@ -18,9 +18,7 @@ _supported_fs btrfs
>  _require_scratch
>  _require_btrfs_qgroup_report
>  
> -# Force a small leaf size to make it easier to blow out our root
> -# subvolume tree
> -_scratch_mkfs "--nodesize 16384" >/dev/null
> +_scratch_mkfs >> $seqres.full || _fail "mkfs failed"

Oh, generally we don't check the return status of default _scratch_mkfs, except
there're specific arguments for _scratch_mkfs. Or we need to add "_fail" to each
mkfs lines. So I'd like to remove that "_fail" when I merge it.

Thanks,
Zorro

>  _scratch_mount
>  _run_btrfs_util_prog quota enable $SCRATCH_MNT
>  
> -- 
> 2.38.1
> 

Re: [PATCH v2] btrfs/122: fix nodesize option in mfks.btrfs

[Anand Jain]

On 10/06/2023 15:03, Zorro Lang wrote:
> On Fri, Jun 02, 2023 at 07:38:54PM +0800, Anand Jain wrote:
>> btrf/122 is failing on a system with 64k page size:
>>
>>       QA output created by 122
>>      +ERROR: illegal nodesize 16384 (smaller than 65536)
>>      +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>>      +mount /dev/vdb2 /mnt/scratch failed
>>      +(see /xfstests-dev/results//btrfs/122.full for details)
>>
>> Mkfs.btrfs sets the default node size to 16K when the sector size is less
>> than 16K, and it matches the sector size when it's greater than 16K.
>> So, there's no need to explicitly set it.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> v2: Remove the redundant explicit nodesize option from mkfs.btrfs.
>>      Changed: Title from "btrfs/122: adjust nodesize to match pagesize"
>>      
>>
>>   tests/btrfs/122 | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/tests/btrfs/122 b/tests/btrfs/122
>> index 345317536f40..9d5e9efccec7 100755
>> --- a/tests/btrfs/122
>> +++ b/tests/btrfs/122
>> @@ -18,9 +18,7 @@ _supported_fs btrfs
>>   _require_scratch
>>   _require_btrfs_qgroup_report
>>   
>> -# Force a small leaf size to make it easier to blow out our root
>> -# subvolume tree
>> -_scratch_mkfs "--nodesize 16384" >/dev/null
>> +_scratch_mkfs >> $seqres.full || _fail "mkfs failed"
> 
> Oh, generally we don't check the return status of default _scratch_mkfs, except
> there're specific arguments for _scratch_mkfs. Or we need to add "_fail" to each
> mkfs lines. So I'd like to remove that "_fail" when I merge it.

  Ah. Ok that should be fine.

Thanks, Anand

> 
> Thanks,
> Zorro
> 
>>   _scratch_mount
>>   _run_btrfs_util_prog quota enable $SCRATCH_MNT
>>   
>> -- 
>> 2.38.1
>>
> 

Re: [PATCH v2] btrfs/122: fix nodesize option in mfks.btrfs

[Ritesh Harjani]

Anand Jain <anand.jain@oracle.com> writes:

> btrf/122 is failing on a system with 64k page size:
>
>      QA output created by 122
>     +ERROR: illegal nodesize 16384 (smaller than 65536)
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/vdb2, missing codepage or helper program, or other error.
>     +mount /dev/vdb2 /mnt/scratch failed
>     +(see /xfstests-dev/results//btrfs/122.full for details)
>
> Mkfs.btrfs sets the default node size to 16K when the sector size is less
> than 16K, and it matches the sector size when it's greater than 16K.
> So, there's no need to explicitly set it.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v2: Remove the redundant explicit nodesize option from mkfs.btrfs.
>     Changed: Title from "btrfs/122: adjust nodesize to match pagesize"
>
>
>  tests/btrfs/122 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Thanks for fixing this. I have tested this on Power with 64k pagesize and x86
with 4k pagesize.

Please feel free to add -
Tested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

-ritesh