Re: [PATCH 2/2] block: add max_active_zones to blk-sysfs

[Javier GonzÃlez]

On 02.07.2020 08:41, Niklas Cassel wrote:
> On Wed, Jul 01, 2020 at 01:16:52PM +0200, Javier GonzÃlez wrote:
> > On 16.06.2020 12:25, Niklas Cassel wrote:
> > > Add a new max_active zones definition in the sysfs documentation.
> > > This definition will be common for all devices utilizing the zoned block
> > > device support in the kernel.
> > >
> > > Export max_active_zones according to this new definition for NVMe Zoned
> > > Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
> > > the kernel), and ZBC SCSI devices.
> > >
> > > Add the new max_active_zones struct member to the request_queue, rather
> > > than as a queue limit, since this property cannot be split across stacking
> > > drivers.
> > >
> > > For SCSI devices, even though max active zones is not part of the ZBC/ZAC
> > > spec, export max_active_zones as 0, signifying "no limit".
> > >
> > > Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
> > > ---
>
> (snip)
>
> > Looking a second time at these patches, wouldn't it make sense to move
> > this to queue_limits?
>
> Hello Javier,
>
> The problem with having MAR/MOR as queue_limits, is that they
> then would be split across stacking drivers/device-mapper targets.
> However, MAR/MOR are not splittable, at least not the way the
> block layer works today.
>
> If the block layer and drivers ever change so that they do
> accounting of zone conditions, then we could divide the MAR/MOR to
> be split over stacking drivers, but because of performance reasons,
> this will probably never happen.
> In the unlikely event that it did happen, we would still use the
> same sysfs-path for these properties, the only thing that would
> change would be that these would be moved into queue_limits.
>
>
> So the way the code looks right now, these properties cannot
> be split, therefore I chose to put them inside request_queue
> (just like nr_zones), rather than request_queue->limits
> (which is of type struct queue_limits).
>
> nr_zones is also exposed as a sysfs property, even though it
> is part of request_queue, so I don't see why MAR/MOR can't do
> the same. Also see Damien's replies to PATCH 1/2 of this series,
> which reaches the same conclusion.

Thanks for explaining Niklas - makes sense. I just looked at your patch
again while adding other attributes and thought it would be worth asking
the reason behind it.

You can keep the reviewed-by on the 2 patches.

Javier