On Jun 01, 2021 / 07:19, Damien Le Moal wrote:
> On 2021/06/01 16:15, Niklas Cassel wrote:
> > On Tue, Jun 01, 2021 at 07:01:18AM +0000, Damien Le Moal wrote:
> >> On 2021/06/01 15:48, Shin'ichiro Kawasaki wrote:
> >>> Recent commit d2f442bc0bd5 ("ioengines: add get_max_open_zones zoned
> >>> block device operation") modified fio to compare --max_open_zoned option
> >>
> >> s/max_open_zoned/max_open_zones
Thanks. Will reflect.
> >>
> >>> value and max_open_zones reported by the device. The device limit is
> >>> fetched through sysfs or through an ioengine specific implementation.
> >>>
> >>> The test script currently try to fetch the max open zones limit using
> >>> libzbc tools or sg_inq. If either of these fail, default value 128 is
> >>> supplied. This default value can be too high when the test script is
> >>> run for certain zoned block devices, and can therefore result in fio
> >>> error and test case failure.
> >>
> >> If sysfs has the max_open_zones attribute file present, that value should be
> >> retrieved first. libzbc and sg_inq should be used only if the sysfs attribute is
> >> not present, that is, when running on older kernels.
> >
> > Since we can assume that the test script and fio versions are updated in sync,
> > I don't see any point of duplicating the sysfs parsing in the test script.
> >
> > If the test script either specifies --max_open_zones=0 or omits the parameter,
> > fio itself will either parse sysfs, or use libzbc to fetch the max limit.
>
> True. Unless the user is running the tests with some special max_open_zones, we
> should not try to grab any value then.
>
> >
> > So I think that the test script should only use those "methods" that fio
> > itself can't do.
> > Right now fio can parse sysfs and use libzbc.
> > The only remaining method is sg_inq. This only I assume that we want to
> > keep in the test. (I guess the alternative is to implement the
> > .get_max_open_zones() fio ioengine callback in engines/sg.c, that does
> > the same. But we probably don't want to do that, right?)
>
> No, we certainly do not. That would need a lot more code than that, e.g. to also
> detect zone model. That would mean basically re-implementing what libzbc does
> inside the sg engine. And since we now have the libzbc engine, I do not see the
> point.
>
> >
> >>
> >>>
> >>> To avoid the failure, modify the default value used in the test script
> >>> from 128 to 0. With this, --max_open_zoned=0 is passed to fio, and it
> >>> makes fio use the max_open_zones reported by the device.
> >>
> >> Why pass anything at all since now fio will use the device reported limit by
> >> default if the user does not specify anything ?
> >
> > Sure, we can just omit the --max_open_zones parameter completely.
> > That might be more clear.
I think still we need the --max_open_zones option in case the latest fio is
run on older kernel which does not have max_open_zones in sysfs. In such case,
fio is not able to obtain the correct max_open_zones of the device (unless it
uses libzbc I/O engine).
There are only two test cases which refers the max_open_zones that the test
script obtained: #31 and #32. The test case #31 has a check if the found
max_open_zones is zero or not to decide write target zone count. The
max_open_zones value is not used for --max_open_zones option (can not omit it).
As for the test case #32, it specifies "--max_open_zones=$max_open_zones" to the
fio command. I assume this option is still valid and required for the older
kernels. We could add a check to the test script to specify this option only
when max_open_zones is zero, but I think it's the better not to add such check
to keep the test script simpler.
> >
> >>
> >>>
> >>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> >>> ---
> >>> t/zbd/functions | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/t/zbd/functions b/t/zbd/functions
> >>> index 40ffe1de..1a7ea970 100644
> >>> --- a/t/zbd/functions
> >>> +++ b/t/zbd/functions
> >>> @@ -180,8 +180,8 @@ max_open_zones() {
> >>> if ! ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" \
> >>> > /dev/null 2>&1; then
> >>> # Non scsi device such as null_blk can not return max open zones.
> >>
> >> This comment is a little outdated since even nullblk now has a limit exposed
> >> through sysfs (the limit may be 0 == no limit). See above comment. sysfs should
> >> be the first thing to look at.
Right. I will repharse as follows.
# Sg_inq can not get max_open_zones of non scsi devices.
> >>
> >>> - # Use default value.
> >>> - echo 128
> >>> + # Specify 0 to indicate fio to get max open zones from the device.
> >>> + echo 0
> >>> else
> >>> ${sg_inq} -e --page=0xB6 --len=20 --hex "$dev" | tail -1 |
> >>> {
> >>>
> >>
> >>
> >> --
> >> Damien Le Moal
> >> Western Digital Research
> >>
>
>
> --
> Damien Le Moal
> Western Digital Research
>
--
Best Regards,
Shin'ichiro Kawasaki
