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

["Javier González" <javier@javigon.com>]

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@wdc.com>
>> > ---
>
>(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