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

From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: "Javier González" <javier@javigon.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
Date: Thu, 2 Jul 2020 08:41:05 +0000	[thread overview]
Message-ID: <20200702084104.GA607715@localhost.localdomain> (raw)
In-Reply-To: <20200701111330.3vpivrovh3i46maa@mpHalley.local>

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.

Kind regards,
Niklas