Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 2 Jul 2020 12:20:41 +0200
From:   Javier =?utf-8?B?R29uesOhbGV6?= <javier@javigon.com>
To:     Niklas Cassel <Niklas.Cassel@wdc.com>
Cc:     Jonathan Corbet <corbet@lwn.net>, Jens Axboe <axboe@kernel.dk>,
        Keith Busch <kbusch@kernel.org>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [PATCH 2/2] block: add max_active_zones to blk-sysfs
Message-ID: <20200702102041.qlehlokxel5ed6sf@mpHalley.local>
References: <20200616102546.491961-1-niklas.cassel@wdc.com>
 <20200616102546.491961-3-niklas.cassel@wdc.com>
 <20200701111330.3vpivrovh3i46maa@mpHalley.local>
 <20200702084104.GA607715@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200702084104.GA607715@localhost.localdomain>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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