* [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation [not found] <CGME20220315201834eucas1p14717bdbd666dd59b2ef4c86f42bfeb90@eucas1p1.samsung.com> @ 2022-03-15 20:17 ` Pankaj Raghav 2022-03-16 4:07 ` Naohiro Aota 0 siblings, 1 reply; 5+ messages in thread From: Pankaj Raghav @ 2022-03-15 20:17 UTC (permalink / raw) To: Johannes Thumshirn, fstests Cc: Luis Chamberlain, Javier González, Damien Le Moal, 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. 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation 2022-03-15 20:17 ` [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation Pankaj Raghav @ 2022-03-16 4:07 ` Naohiro Aota 2022-03-16 15:43 ` Pankaj Raghav 0 siblings, 1 reply; 5+ messages in thread From: Naohiro Aota @ 2022-03-16 4:07 UTC (permalink / raw) To: Pankaj Raghav Cc: Johannes Thumshirn, fstests, Luis Chamberlain, Javier González, Damien Le Moal, Pankaj Raghav, Kanchan Joshi, Adam Manzanares 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation 2022-03-16 4:07 ` Naohiro Aota @ 2022-03-16 15:43 ` Pankaj Raghav 2022-03-16 23:20 ` Damien Le Moal 0 siblings, 1 reply; 5+ messages in thread From: Pankaj Raghav @ 2022-03-16 15:43 UTC (permalink / raw) To: Naohiro Aota Cc: Johannes Thumshirn, fstests, Luis Chamberlain, Javier González, Damien Le Moal, Pankaj Raghav, Kanchan Joshi, Adam Manzanares 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation 2022-03-16 15:43 ` Pankaj Raghav @ 2022-03-16 23:20 ` Damien Le Moal 2022-03-17 9:53 ` Pankaj Raghav 0 siblings, 1 reply; 5+ messages in thread From: Damien Le Moal @ 2022-03-16 23:20 UTC (permalink / raw) To: Pankaj Raghav, Naohiro Aota Cc: Johannes Thumshirn, fstests, Luis Chamberlain, Javier González, Pankaj Raghav, Kanchan Joshi, Adam Manzanares 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation 2022-03-16 23:20 ` Damien Le Moal @ 2022-03-17 9:53 ` Pankaj Raghav 0 siblings, 0 replies; 5+ messages in thread From: Pankaj Raghav @ 2022-03-17 9:53 UTC (permalink / raw) To: Damien Le Moal, Naohiro Aota Cc: Johannes Thumshirn, fstests, Luis Chamberlain, Javier González, Pankaj Raghav, Kanchan Joshi, Adam Manzanares 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-17 9:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20220315201834eucas1p14717bdbd666dd59b2ef4c86f42bfeb90@eucas1p1.samsung.com> 2022-03-15 20:17 ` [PATCH] btrfs/237: Use zone cap instead of zone size in fill_size and rest calculation Pankaj Raghav 2022-03-16 4:07 ` Naohiro Aota 2022-03-16 15:43 ` Pankaj Raghav 2022-03-16 23:20 ` Damien Le Moal 2022-03-17 9:53 ` Pankaj Raghav
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.