* [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation
[not found] <CGME20220317122843eucas1p1d01a061e25bd7bfaaf912780c011ffdd@eucas1p1.samsung.com>
@ 2022-03-17 12:28 ` Pankaj Raghav
2022-03-18 7:54 ` Naohiro Aota
0 siblings, 1 reply; 4+ messages in thread
From: Pankaj Raghav @ 2022-03-17 12:28 UTC (permalink / raw)
To: Naohiro Aota, Damien Le Moal, Johannes Thumshirn, fstests
Cc: Luis Chamberlain, Javier González, Pankaj Raghav,
Kanchan Joshi, Adam Manzanares, 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.
As a zoned device can have variable capacity, use the capacity from zone 5
as it is the first data block group.
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>
---
Changes since v1:
- Add a filter that can append a 'cap' column for blkzone version <2.37
(Naohiro)
- Extract the capacity from the zone on which the test is performed as a
zoned device can have variable zone capacity(Naohiro)
common/filter | 12 ++++++++++++
tests/btrfs/237 | 13 ++++++++-----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/common/filter b/common/filter
index 257227c2..5acd57dd 100644
--- a/common/filter
+++ b/common/filter
@@ -634,5 +634,17 @@ _filter_bash()
sed -e "s/^bash: line 1: /bash: /"
}
+#
+#blkzone report added zone capacity to be printed from v2.37.
+#This filter will add an extra column 'cap' with the same value of
+#'len'(zone size) for blkzone version < 2.37
+#
+#Before: start: 0x000100000, len 0x040000, wptr 0x000000 ..
+#After: start: 0x000100000, len 0x040000, cap 0x040000, wptr 0x000000 ..
+_filter_blkzone_report()
+{
+ $AWK_PROG -F "," 'BEGIN{OFS=",";} $3 !~ /cap/ {$2=$2","$2;} {print;}' | sed -e 's/len/cap/2'
+}
+
# make sure this script returns success
/bin/true
diff --git a/tests/btrfs/237 b/tests/btrfs/237
index 96940549..33a0d81d 100755
--- a/tests/btrfs/237
+++ b/tests/btrfs/237
@@ -35,9 +35,6 @@ get_data_bg()
grep -Eo "CHUNK_ITEM [[:digit:]]+" | cut -d ' ' -f 2
}
-zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
-zonesize=$((zonesize << 9))
-
_scratch_mkfs >/dev/null 2>&1
_scratch_mount -o commit=1 # 1s commit time to speed up test
@@ -49,12 +46,18 @@ if [[ "$uuid" == "" ]]; then
exit 1
fi
+data_bg_zone_idx=5
+size=$($BLKZONE_PROG report -c $data_bg_zone_idx $SCRATCH_DEV | tail -1 |\
+ _filter_blkzone_report |\
+ grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
+size=$((size << 9))
+
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation
2022-03-17 12:28 ` [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation Pankaj Raghav
@ 2022-03-18 7:54 ` Naohiro Aota
2022-03-18 9:28 ` Pankaj Raghav
0 siblings, 1 reply; 4+ messages in thread
From: Naohiro Aota @ 2022-03-18 7:54 UTC (permalink / raw)
To: Pankaj Raghav
Cc: Damien Le Moal, Johannes Thumshirn, fstests, Luis Chamberlain,
Javier González, Pankaj Raghav, Kanchan Joshi,
Adam Manzanares
On Thu, Mar 17, 2022 at 01:28:39PM +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.
>
> As a zoned device can have variable capacity, use the capacity from zone 5
> as it is the first data block group.
>
> 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>
> ---
> Changes since v1:
> - Add a filter that can append a 'cap' column for blkzone version <2.37
> (Naohiro)
> - Extract the capacity from the zone on which the test is performed as a
> zoned device can have variable zone capacity(Naohiro)
>
> common/filter | 12 ++++++++++++
> tests/btrfs/237 | 13 ++++++++-----
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/common/filter b/common/filter
> index 257227c2..5acd57dd 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -634,5 +634,17 @@ _filter_bash()
> sed -e "s/^bash: line 1: /bash: /"
> }
>
> +#
> +#blkzone report added zone capacity to be printed from v2.37.
> +#This filter will add an extra column 'cap' with the same value of
> +#'len'(zone size) for blkzone version < 2.37
> +#
> +#Before: start: 0x000100000, len 0x040000, wptr 0x000000 ..
> +#After: start: 0x000100000, len 0x040000, cap 0x040000, wptr 0x000000 ..
^- nit: need a space after '#'
> +_filter_blkzone_report()
> +{
> + $AWK_PROG -F "," 'BEGIN{OFS=",";} $3 !~ /cap/ {$2=$2","$2;} {print;}' | sed -e 's/len/cap/2'
> +}
> +
> # make sure this script returns success
> /bin/true
> diff --git a/tests/btrfs/237 b/tests/btrfs/237
> index 96940549..33a0d81d 100755
> --- a/tests/btrfs/237
> +++ b/tests/btrfs/237
> @@ -35,9 +35,6 @@ get_data_bg()
> grep -Eo "CHUNK_ITEM [[:digit:]]+" | cut -d ' ' -f 2
> }
>
> -zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
> -zonesize=$((zonesize << 9))
> -
> _scratch_mkfs >/dev/null 2>&1
> _scratch_mount -o commit=1 # 1s commit time to speed up test
>
> @@ -49,12 +46,18 @@ if [[ "$uuid" == "" ]]; then
> exit 1
> fi
>
> +data_bg_zone_idx=5
Hmm. Yeah, it's currently there. But, I'd hesitate to use that like a
constant number.
Instead, how about querying "btrfs inspect-internal" like in
get_data_bg() do? E.g, like this:
# Assumes SINGLE data profile
btrfs inspect-internal dump-tree -t CHUNK $SCRATCH_DEV | grep -A 4 CHUNK_ITEM |\
grep -A 3 'type DATA\|SINGLE' |\
grep -Eo 'offset [[:digit:]]+'| cut -d ' ' -f 2
This command dumps the location of data BG in a byte unit.
Then, we can calculate the sector number and can supply it to "blkzone
report -o".
> +size=$($BLKZONE_PROG report -c $data_bg_zone_idx $SCRATCH_DEV | tail -1 |\
> + _filter_blkzone_report |\
> + grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
> +size=$((size << 9))
> +
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation
2022-03-18 7:54 ` Naohiro Aota
@ 2022-03-18 9:28 ` Pankaj Raghav
2022-03-19 3:41 ` Naohiro Aota
0 siblings, 1 reply; 4+ messages in thread
From: Pankaj Raghav @ 2022-03-18 9:28 UTC (permalink / raw)
To: Naohiro Aota
Cc: Damien Le Moal, Johannes Thumshirn, fstests, Luis Chamberlain,
Javier González, Pankaj Raghav, Kanchan Joshi,
Adam Manzanares
On 2022-03-18 08:54, Naohiro Aota wrote:
>>
>> -zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
>> -zonesize=$((zonesize << 9))
>> -
>> _scratch_mkfs >/dev/null 2>&1
>> _scratch_mount -o commit=1 # 1s commit time to speed up test
>>
>> @@ -49,12 +46,18 @@ if [[ "$uuid" == "" ]]; then
>> exit 1
>> fi
>>
>> +data_bg_zone_idx=5
>
> Hmm. Yeah, it's currently there. But, I'd hesitate to use that like a
> constant number.
>
> Instead, how about querying "btrfs inspect-internal" like in
> get_data_bg() do? E.g, like this:
>
> # Assumes SINGLE data profile
> btrfs inspect-internal dump-tree -t CHUNK $SCRATCH_DEV | grep -A 4 CHUNK_ITEM |\
> grep -A 3 'type DATA\|SINGLE' |\
> grep -Eo 'offset [[:digit:]]+'| cut -d ' ' -f 2
>
> This command dumps the location of data BG in a byte unit.
>
> Then, we can calculate the sector number and can supply it to "blkzone
> report -o".
>
Got it. I also prefer this over hardcoding.
I think we can reuse the get_data_bg function as it also outputs the
same data BG offset.
Like this:
start_data_bg_zone=$(get_data_bg)
start_data_bg_zone=$((data_bg_zone >> 9))
size=$($BLKZONE_PROG report -o $start_data_bg_zone -l 1 $SCRATCH_DEV |\
...
Let me know your thoughts.
>> +size=$($BLKZONE_PROG report -c $data_bg_zone_idx $SCRATCH_DEV | tail -1 |\
>> + _filter_blkzone_report |\
>> + grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
>> +size=$((size << 9))
>> +
>> 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
--
Regards,
Pankaj
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation
2022-03-18 9:28 ` Pankaj Raghav
@ 2022-03-19 3:41 ` Naohiro Aota
0 siblings, 0 replies; 4+ messages in thread
From: Naohiro Aota @ 2022-03-19 3:41 UTC (permalink / raw)
To: Pankaj Raghav
Cc: Damien Le Moal, Johannes Thumshirn, fstests, Luis Chamberlain,
Javier González, Pankaj Raghav, Kanchan Joshi,
Adam Manzanares
On Fri, Mar 18, 2022 at 10:28:44AM +0100, Pankaj Raghav wrote:
>
>
> On 2022-03-18 08:54, Naohiro Aota wrote:
> >>
> >> -zonesize=$(cat /sys/block/$(_short_dev $SCRATCH_DEV)/queue/chunk_sectors)
> >> -zonesize=$((zonesize << 9))
> >> -
> >> _scratch_mkfs >/dev/null 2>&1
> >> _scratch_mount -o commit=1 # 1s commit time to speed up test
> >>
> >> @@ -49,12 +46,18 @@ if [[ "$uuid" == "" ]]; then
> >> exit 1
> >> fi
> >>
> >> +data_bg_zone_idx=5
> >
> > Hmm. Yeah, it's currently there. But, I'd hesitate to use that like a
> > constant number.
> >
> > Instead, how about querying "btrfs inspect-internal" like in
> > get_data_bg() do? E.g, like this:
> >
> > # Assumes SINGLE data profile
> > btrfs inspect-internal dump-tree -t CHUNK $SCRATCH_DEV | grep -A 4 CHUNK_ITEM |\
> > grep -A 3 'type DATA\|SINGLE' |\
> > grep -Eo 'offset [[:digit:]]+'| cut -d ' ' -f 2
> >
> > This command dumps the location of data BG in a byte unit.
> >
> > Then, we can calculate the sector number and can supply it to "blkzone
> > report -o".
> >
> Got it. I also prefer this over hardcoding.
> I think we can reuse the get_data_bg function as it also outputs the
> same data BG offset.
No, we cannot.
They look the same in number, but they are different in type. The
get_data_bg returns the logical address (virtual btrfs's internal
address) of the BG. And, my command above returns the physical address
(on-disk address) of the BG.
> Like this:
>
> start_data_bg_zone=$(get_data_bg)
> start_data_bg_zone=$((data_bg_zone >> 9))
>
> size=$($BLKZONE_PROG report -o $start_data_bg_zone -l 1 $SCRATCH_DEV |\
> ...
> Let me know your thoughts.
> >> +size=$($BLKZONE_PROG report -c $data_bg_zone_idx $SCRATCH_DEV | tail -1 |\
> >> + _filter_blkzone_report |\
> >> + grep -Po "cap ([0x\d]+)+" | cut -d ' ' -f 2)
> >> +size=$((size << 9))
> >> +
> >> 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
>
> --
> Regards,
> Pankaj
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-19 3:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20220317122843eucas1p1d01a061e25bd7bfaaf912780c011ffdd@eucas1p1.samsung.com>
2022-03-17 12:28 ` [PATCH v2] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation Pankaj Raghav
2022-03-18 7:54 ` Naohiro Aota
2022-03-18 9:28 ` Pankaj Raghav
2022-03-19 3:41 ` Naohiro Aota
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.