On 2020/08/28 19:06, Niklas Cassel wrote: > 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 :) Well, I prefer to prioritize code readability over following a not-so-good name that the standard chose. The function description makes it clear that it is zone management a-la-ZBC, so a function name clarifying what is being checked is better in my opinion. Not a blocker though. Feel free to chose what to do here. Cheers. -- Damien Le Moal Western Digital Research