Re: [PATCH v2] null_blk: add support for max open/active zone limit for zoned devices

[Niklas Cassel]

On Fri, Aug 28, 2020 at 07:06:26AM +0000, Damien Le Moal wrote:
> On 2020/08/27 22:50, Niklas Cassel wrote:
> > Add support for user space to set a max open zone and a max active zone
> > limit via configfs. By default, the default values are 0 == no limit.
> > 
> > Call the block layer API functions used for exposing the configured
> > limits to sysfs.
> > 
> > Add accounting in null_blk_zoned so that these new limits are respected.
> > Performing an operating that would exceed these limits results in a
> 
> Performing a write operation that would result in exceeding these...
> 
> > standard I/O error.
> > 

It is not only a write operation, also e.g. open zone operation.
However I will s/Performing an operating/Performing an operation/

> > +/*
> > + * This function matches the manage open zone resources function in the ZBC standard,
> > + * with the addition of max active zones support (added in the ZNS standard).
> > + *
> > + * The function determines if a zone can transition to implicit open or explicit open,
> > + * while maintaining the max open zone (and max active zone) limit(s). It may close an
> > + * implicit open zone in order to make additional zone resources available.
> > + *
> > + * ZBC states that an implicit open zone shall be closed only if there is not
> > + * room within the open limit. However, with the addition of an active limit,
> > + * it is not certain that closing an implicit open zone will allow a new zone
> > + * to be opened, since we might already be at the active limit capacity.
> > + */
> > +static bool null_manage_zone_resources(struct nullb_device *dev, struct blk_zone *zone)
> 
> I still do not like the name. Since this return a bool, what about
> null_has_zone_resources() ?

I also don't like the name :)

However, since the ZBC spec, in the descriptions of "Write operation, Finish
operation, and Open operation", says that the "manage open zone resources"
function must be called before each of these operations are performed,
and that there is a section that defines how the "manage open zone resources"
is defined, I was thinking that having a similar name would be of value.

And I agree that it is weird that it returns a bool, but that is how it is
defined in the standard.

Perhaps it should have exactly the same name as the standard, i.e.
null_manage_open_zone_resources() ?

However, if you don't think that there is any point of trying to have
a similar name to the function in ZBC, then I will happily rename it :)


Kind regards,
Niklas