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;
IronPort-SDR: d6V3jlkX4NKqImZVIfMSeNlT1SrCxnX9JgSytMLjxh+Psbi1zABocr/o6J5Gzb6hmftlFgEzMU
 yjwR6rjuwd/XBoy7EORXVLkiyKEZmeC5TYXSCl9mvJhHYmcpBbzEoVmKZh3rOH4WpALPPA/Jt1
 gaSqP5/wMjGQ87Ds73JxovEX29V8QWF6GObvSsZKV+z9wgpu6GzfN8ndEfs0NhQw8k4v8WG5O3
 gewQ8zCwDbb5rYeMlYgIYk2XU9T3+0L5XpJ5nIocBxIjgLSPsSAEeSrt/EEKaBB+gDfkHh/4x1
 lfs=
From:   Johannes Thumshirn <Johannes.Thumshirn@wdc.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>,
        =?iso-8859-1?Q?Javier_Gonz=E1lez?= <javier@javigon.com>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
Thread-Topic: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
Thread-Index: AQHWUJ1laLSAJw11LUObxsrvIlcW1g==
Date:   Fri, 3 Jul 2020 13:41:13 +0000
Message-ID: <SN4PR0401MB35983A6201A6E2AF2611FECD9B6A0@SN4PR0401MB3598.namprd04.prod.outlook.com>
References: <20200702181922.24190-1-niklas.cassel@wdc.com>
 <20200702181922.24190-2-niklas.cassel@wdc.com>
 <SN4PR0401MB359886C77E3711DAF16D9B9F9B6A0@SN4PR0401MB3598.namprd04.prod.outlook.com>
 <20200703092353.GA33841@localhost.localdomain>
 <SN4PR0401MB359801D165A0B9882B43B3719B6A0@SN4PR0401MB3598.namprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 528c3708-eb81-49c4-7f47-08d81f56c0c5
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jul 2020 13:41:13.3250
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4KUl4QosvNFeXGnoZuJn+nEUKuAs7xzWWfLYXQGelvdX0lJi9UFogxfv5vK0MPAsEhm+BbKPU2PdUSwIAAqfYcS9OW7YGrc7MVfN+NYEdcU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN4PR0401MB3519
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 03/07/2020 14:09, Johannes Thumshirn wrote:=0A=
> On 03/07/2020 11:23, Niklas Cassel wrote:=0A=
>> On Fri, Jul 03, 2020 at 08:22:45AM +0000, Johannes Thumshirn wrote:=0A=
>>> On 02/07/2020 20:20, Niklas Cassel wrote:=0A=
>>>> Documentation/block/queue-sysfs.rst |  7 +++++++=0A=
>>>>  block/blk-sysfs.c                   | 15 +++++++++++++++=0A=
>>>>  drivers/nvme/host/zns.c             |  1 +=0A=
>>>>  drivers/scsi/sd_zbc.c               |  4 ++++=0A=
>>>>  include/linux/blkdev.h              | 16 ++++++++++++++++=0A=
>>>>  5 files changed, 43 insertions(+)=0A=
>>>=0A=
>>> Sorry I haven't noticed before, but you forgot null_blk.=0A=
>>=0A=
>> Hello Johannes,=0A=
>>=0A=
>> Actually, I haven't forgotten about null_blk :)=0A=
>>=0A=
>> The problem with null_blk is that, compared to these simple patches that=
=0A=
>> simply exposes the Max Open Zones/Max Active Zones, null_blk additionall=
y=0A=
>> has to keep track of all the zone accounting, and give an error if any=
=0A=
>> of these limits are exceeded.=0A=
>>=0A=
>> null_blk right now follows neither the ZBC nor the ZNS specification=0A=
>> (even though it is almost compliant with ZBC). However, since null_blk=
=0A=
>> is really great to test with, we want it to support Max Active Zones,=0A=
>> even if that concept does not exist in the ZBC standard.=0A=
>>=0A=
>> To add to the problem, open() does not work exactly the same in ZBC and=
=0A=
>> ZNS. In ZBC, the device always tries to close an implicit zone to make=
=0A=
>> room for an explicit zone. In ZNS, a controller that doesn't do this is=
=0A=
>> fully compliant with the spec.=0A=
>>=0A=
>> So now for null_blk, you have things like zones being implicit closed wh=
en=0A=
>> a new zone is opened. And now we will have an additional limit (Max Acti=
ve=0A=
>> Zones), that we need to consider before we can even try to close a zone.=
=0A=
>>=0A=
>>=0A=
>> I've spent a couple of days trying to implement this already, and I thin=
k=0A=
>> that I have a way forward. However, considering that vacations are comin=
g=0A=
>> up, and that I have a bunch of other stuff that I need to do before then=
,=0A=
>> I'm not 100% sure that I will be able to finish it in time for the comin=
g=0A=
>> merge window.=0A=
>>=0A=
>> Therefore, I was hoping that we could merge this series as is, and I wil=
l=0A=
>> send out the null_blk changes when they are ready, which might or might=
=0A=
>> not make it for this merge window.=0A=
> =0A=
> No problem, I'm just working on MOR support for zonefs and though about h=
ow=0A=
> I'm going to test it. This is where I've noticed null_blk doesn't really =
=0A=
> expose a config knob for MOR. I can do some temporary hacks to test my ch=
anges=0A=
> and wait for your's to materialize. =0A=
> =0A=
> =0A=
>> In the meantime, MAR/MOR properties for null_blk will be exposed as 0,=
=0A=
>> which means "no limit". (Which is the case when a zoned block device dri=
ver=0A=
>> doesn't do an explicit call to blk_queue_max_{open,active}_zones()).=0A=
> =0A=
> =0A=
=0A=
Another thing I've noticed, can you please introduce the bdev_max_open_zone=
s() =0A=
and bdev_max_open_reagions() functions as well?=0A=
Like this (untested):=0A=
=0A=
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h=0A=
index bb9e6eb6a7e6..612f4e36828d 100644=0A=
--- a/include/linux/blkdev.h=0A=
+++ b/include/linux/blkdev.h=0A=
@@ -1522,6 +1522,15 @@ static inline sector_t bdev_zone_sectors(struct bloc=
k_device *bdev)=0A=
        return 0;=0A=
 }=0A=
 =0A=
+static inline unsigned int bdev_max_open_zones(struct block_device *bdev)=
=0A=
+{=0A=
+       struct request_queue *q =3D bdev_get_queue(bdev);=0A=
+=0A=
+       if (q)=0A=
+               return queue_max_open_zones(q);=0A=
+       return 0;=0A=
+}=0A=
+=0A=
 static inline int queue_dma_alignment(const struct request_queue *q)=0A=
 {=0A=
        return q ? q->dma_alignment : 511;=0A=