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
>>
>>> 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.
>
>>
>>>
>>> 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.
>>
>>> - # 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
