[PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[Pankaj Raghav]

This test will break when zone capacity != zone size because the
calculation of the size to be filled is done using zone_size instead of
the actual capacity available per zone. Fix it by using zone capacity.

The support to extract zone capacity was added to blkzone only from
version 2.37. So zcap will be used only when the blkzone version is
greater or equal to 2.37.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 tests/btrfs/237 | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/btrfs/237 b/tests/btrfs/237
index 96940549..6d3fe2f2 100755
--- a/tests/btrfs/237
+++ b/tests/btrfs/237
@@ -36,7 +36,21 @@ get_data_bg()
 }
 
 zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
-zonesize=$((zonesize << 9))
+size=$((zonesize << 9))
+
+# blkzone supports printing zone cap only after version 2.37
+_blkzone_req_major_ver=2
+_blkzone_req_minor_ver=37
+blkzone_ver=$($BLKZONE_PROG --version | cut -d " " -f 4 | cut -d "-" -f 1)
+blkzone_ver=( ${blkzone_ver//./ } )
+blkzone_major_ver=${blkzone_ver[0]}
+blkzone_minor_ver=${blkzone_ver[1]}
+if [[ $blkzone_major_ver -ge $_blkzone_req_major_ver &&\
+	$blkzone_minor_ver -ge $_blkzone_req_minor_ver ]]; then
+	zonecap=$($BLKZONE_PROG report -c 1 $SCRATCH_DEV |\
+		grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
+	size=$((zonecap << 9))
+fi
 
 _scratch_mkfs >/dev/null 2>&1
 _scratch_mount -o commit=1 # 1s commit time to speed up test
@@ -53,8 +67,8 @@ reclaim_threshold=75
 echo $reclaim_threshold > /sys/fs/btrfs/"$uuid"/bg_reclaim_threshold
 fill_percent=$((reclaim_threshold + 2))
 rest_percent=$((90 - fill_percent)) # make sure we're not creating a new BG
-fill_size=$((zonesize * fill_percent / 100))
-rest=$((zonesize * rest_percent / 100))
+fill_size=$((size * fill_percent / 100))
+rest=$((size * rest_percent / 100))
 
 # step 1, fill FS over $fillsize
 $XFS_IO_PROG -fc "pwrite 0 $fill_size" $SCRATCH_MNT/$seq.test1 >> $seqres.full
-- 
2.25.1

Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[Naohiro Aota]

On Tue, Mar 15, 2022 at 09:17:56PM +0100, Pankaj Raghav wrote:
> This test will break when zone capacity != zone size because the
> calculation of the size to be filled is done using zone_size instead of
> the actual capacity available per zone. Fix it by using zone capacity.
> 
> The support to extract zone capacity was added to blkzone only from
> version 2.37. So zcap will be used only when the blkzone version is
> greater or equal to 2.37.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  tests/btrfs/237 | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/btrfs/237 b/tests/btrfs/237
> index 96940549..6d3fe2f2 100755
> --- a/tests/btrfs/237
> +++ b/tests/btrfs/237
> @@ -36,7 +36,21 @@ get_data_bg()
>  }
>  
>  zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
> -zonesize=$((zonesize << 9))
> +size=$((zonesize << 9))
> +
> +# blkzone supports printing zone cap only after version 2.37
> +_blkzone_req_major_ver=2
> +_blkzone_req_minor_ver=37
> +blkzone_ver=$($BLKZONE_PROG --version | cut -d " " -f 4 | cut -d "-" -f 1)
> +blkzone_ver=( ${blkzone_ver//./ } )
> +blkzone_major_ver=${blkzone_ver[0]}
> +blkzone_minor_ver=${blkzone_ver[1]}
> +if [[ $blkzone_major_ver -ge $_blkzone_req_major_ver &&\
> +	$blkzone_minor_ver -ge $_blkzone_req_minor_ver ]]; then

I'd like to have it as a helper.

Or, how about adding a filter and a wrapper function to add a "cap"
filed whose value equal to the zone size for the old version (like
with _getcap() and _filter_getcap())? Then, we can just call:

  zonecap=$(_blkzone_report ... | grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
  size=$((zonecap << 9))

> +	zonecap=$($BLKZONE_PROG report -c 1 $SCRATCH_DEV |\
> +		grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)

But still, zone capacity can vary for each zone. So, we need to read
the capacity of a zone where the data BG resides on.

> +	size=$((zonecap << 9))
> +fi
>  
>  _scratch_mkfs >/dev/null 2>&1
>  _scratch_mount -o commit=1 # 1s commit time to speed up test
> @@ -53,8 +67,8 @@ reclaim_threshold=75
>  echo $reclaim_threshold > /sys/fs/btrfs/"$uuid"/bg_reclaim_threshold
>  fill_percent=$((reclaim_threshold + 2))
>  rest_percent=$((90 - fill_percent)) # make sure we're not creating a new BG
> -fill_size=$((zonesize * fill_percent / 100))
> -rest=$((zonesize * rest_percent / 100))
> +fill_size=$((size * fill_percent / 100))
> +rest=$((size * rest_percent / 100))
>  
>  # step 1, fill FS over $fillsize
>  $XFS_IO_PROG -fc "pwrite 0 $fill_size" $SCRATCH_MNT/$seq.test1 >> $seqres.full
> -- 
> 2.25.1
> 

Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[Pankaj Raghav]

Hi Naohiro,

On 2022-03-16 05:07, Naohiro Aota wrote:
>> +# blkzone supports printing zone cap only after version 2.37
>> +_blkzone_req_major_ver=2
>> +_blkzone_req_minor_ver=37
>> +blkzone_ver=$($BLKZONE_PROG --version | cut -d " " -f 4 | cut -d "-" -f 1)
>> +blkzone_ver=( ${blkzone_ver//./ } )
>> +blkzone_major_ver=${blkzone_ver[0]}
>> +blkzone_minor_ver=${blkzone_ver[1]}
>> +if [[ $blkzone_major_ver -ge $_blkzone_req_major_ver &&\
>> +	$blkzone_minor_ver -ge $_blkzone_req_minor_ver ]]; then
> 
> I'd like to have it as a helper.
> 
> Or, how about adding a filter and a wrapper function to add a "cap"
> filed whose value equal to the zone size for the old version (like
> with _getcap() and _filter_getcap())? Then, we can just call:
>   zonecap=$(_blkzone_report ... | grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
>   size=$((zonecap << 9))
> 
That is a good idea. I will give it a shot. Thanks.

>> +	zonecap=$($BLKZONE_PROG report -c 1 $SCRATCH_DEV |\
>> +		grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
> 
> But still, zone capacity can vary for each zone. So, we need to read
> the capacity of a zone where the data BG resides on.
> 
Do we support variable zone capacity in Linux? IIRC variable zone sizes
are definitely not supported but I am not sure about variable zone capacity.

But even if we do support, I see that the zone 5 (old_data_zone) and
zone 6 (new_data_zone) during the test and what if the new_data_zone
(zone 6) has a smaller cap than old_data_zone (zone 5)?
The main question: Is there a way to deterministically tell where the
data BG will reside and where it will relocate before we start the test
with variable capacity?

My first look indicates that adding variable zone capacity will make the
test a bit more complex and I am not sure if it is worth the effort if
there are no use cases for it.
Let me know your thoughts.

-- 
Regards,
Pankaj

Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[Damien Le Moal]

On 3/17/22 00:43, Pankaj Raghav wrote:
> Hi Naohiro,
> 
> On 2022-03-16 05:07, Naohiro Aota wrote:
>>> +# blkzone supports printing zone cap only after version 2.37
>>> +_blkzone_req_major_ver=2
>>> +_blkzone_req_minor_ver=37
>>> +blkzone_ver=$($BLKZONE_PROG --version | cut -d " " -f 4 | cut -d "-" -f 1)
>>> +blkzone_ver=( ${blkzone_ver//./ } )
>>> +blkzone_major_ver=${blkzone_ver[0]}
>>> +blkzone_minor_ver=${blkzone_ver[1]}
>>> +if [[ $blkzone_major_ver -ge $_blkzone_req_major_ver &&\
>>> +	$blkzone_minor_ver -ge $_blkzone_req_minor_ver ]]; then
>>
>> I'd like to have it as a helper.
>>
>> Or, how about adding a filter and a wrapper function to add a "cap"
>> filed whose value equal to the zone size for the old version (like
>> with _getcap() and _filter_getcap())? Then, we can just call:
>>   zonecap=$(_blkzone_report ... | grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
>>   size=$((zonecap << 9))
>>
> That is a good idea. I will give it a shot. Thanks.
> 
>>> +	zonecap=$($BLKZONE_PROG report -c 1 $SCRATCH_DEV |\
>>> +		grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
>>
>> But still, zone capacity can vary for each zone. So, we need to read
>> the capacity of a zone where the data BG resides on.
>>
> Do we support variable zone capacity in Linux? IIRC variable zone sizes
> are definitely not supported but I am not sure about variable zone capacity.

No, variable zone capacity is not supported in Linux. By that, I mean that
drive that may change the capacity of a zone after a reset are not
supported. So a zone capacity in Linux is always fixed throughout the life
time of the NS it belongs to.

But nothing mandates that all zones have the same capacity. A drive could
expose zones with different capacities. That is the point Naohiro was making.

> 
> But even if we do support, I see that the zone 5 (old_data_zone) and
> zone 6 (new_data_zone) during the test and what if the new_data_zone
> (zone 6) has a smaller cap than old_data_zone (zone 5)?
> The main question: Is there a way to deterministically tell where the
> data BG will reside and where it will relocate before we start the test
> with variable capacity?
> 
> My first look indicates that adding variable zone capacity will make the
> test a bit more complex and I am not sure if it is worth the effort if
> there are no use cases for it.
> Let me know your thoughts.
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation

[Pankaj Raghav]

On 2022-03-17 00:20, Damien Le Moal wrote:

>>> But still, zone capacity can vary for each zone. So, we need to read
>>> the capacity of a zone where the data BG resides on.
>>>
>> Do we support variable zone capacity in Linux? IIRC variable zone sizes
>> are definitely not supported but I am not sure about variable zone capacity.
> 
> No, variable zone capacity is not supported in Linux. By that, I mean that
> drive that may change the capacity of a zone after a reset are not
> supported. So a zone capacity in Linux is always fixed throughout the life
> time of the NS it belongs to.
> 
> But nothing mandates that all zones have the same capacity. A drive could
> expose zones with different capacities. That is the point Naohiro was making.
> 
Got it. Thanks for the clarification.
>>
>> But even if we do support, I see that the zone 5 (old_data_zone) and
>> zone 6 (new_data_zone) during the test and what if the new_data_zone
>> (zone 6) has a smaller cap than old_data_zone (zone 5)?
>> The main question: Is there a way to deterministically tell where the
>> data BG will reside and where it will relocate before we start the test
>> with variable capacity?
>>
@noahiro: So the data BG for btrfs starts from Zone 5 if I understand it
correctly. Can I then hard code the test to read the cap from zone 5? I
think that should fix your concern.
>> My first look indicates that adding variable zone capacity will make the
>> test a bit more complex and I am not sure if it is worth the effort if
>> there are no use cases for it.
>> Let me know your thoughts.
>>
> 
> 

-- 
Regards,
Pankaj