linux-kernel - Re: [PATCH v2 1/2] block: add max_open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SN4PR0401MB35983A6201A6E2AF2611FECD9B6A0@SN4PR0401MB3598.namprd04.prod.outlook.com>
Date:   Fri, 3 Jul 2020 13:41:13 +0000
From:   Johannes Thumshirn <Johannes.Thumshirn@....com>
To:     Niklas Cassel <Niklas.Cassel@....com>
CC:     Jonathan Corbet <corbet@....net>, Jens Axboe <axboe@...nel.dk>,
        Keith Busch <kbusch@...nel.org>,
        Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Javier González <javier@...igon.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs

On 03/07/2020 14:09, Johannes Thumshirn wrote:
> On 03/07/2020 11:23, Niklas Cassel wrote:
>> On Fri, Jul 03, 2020 at 08:22:45AM +0000, Johannes Thumshirn wrote:
>>> On 02/07/2020 20:20, Niklas Cassel wrote:
>>>> Documentation/block/queue-sysfs.rst |  7 +++++++
>>>>  block/blk-sysfs.c                   | 15 +++++++++++++++
>>>>  drivers/nvme/host/zns.c             |  1 +
>>>>  drivers/scsi/sd_zbc.c               |  4 ++++
>>>>  include/linux/blkdev.h              | 16 ++++++++++++++++
>>>>  5 files changed, 43 insertions(+)
>>>
>>> Sorry I haven't noticed before, but you forgot null_blk.
>>
>> Hello Johannes,
>>
>> Actually, I haven't forgotten about null_blk :)
>>
>> The problem with null_blk is that, compared to these simple patches that
>> simply exposes the Max Open Zones/Max Active Zones, null_blk additionally
>> has to keep track of all the zone accounting, and give an error if any
>> of these limits are exceeded.
>>
>> null_blk right now follows neither the ZBC nor the ZNS specification
>> (even though it is almost compliant with ZBC). However, since null_blk
>> is really great to test with, we want it to support Max Active Zones,
>> even if that concept does not exist in the ZBC standard.
>>
>> To add to the problem, open() does not work exactly the same in ZBC and
>> ZNS. In ZBC, the device always tries to close an implicit zone to make
>> room for an explicit zone. In ZNS, a controller that doesn't do this is
>> fully compliant with the spec.
>>
>> So now for null_blk, you have things like zones being implicit closed when
>> a new zone is opened. And now we will have an additional limit (Max Active
>> Zones), that we need to consider before we can even try to close a zone.
>>
>>
>> I've spent a couple of days trying to implement this already, and I think
>> that I have a way forward. However, considering that vacations are coming
>> up, and that I have a bunch of other stuff that I need to do before then,
>> I'm not 100% sure that I will be able to finish it in time for the coming
>> merge window.
>>
>> Therefore, I was hoping that we could merge this series as is, and I will
>> send out the null_blk changes when they are ready, which might or might
>> not make it for this merge window.
> 
> No problem, I'm just working on MOR support for zonefs and though about how
> I'm going to test it. This is where I've noticed null_blk doesn't really 
> expose a config knob for MOR. I can do some temporary hacks to test my changes
> and wait for your's to materialize. 
> 
> 
>> In the meantime, MAR/MOR properties for null_blk will be exposed as 0,
>> which means "no limit". (Which is the case when a zoned block device driver
>> doesn't do an explicit call to blk_queue_max_{open,active}_zones()).
> 
> 

Another thing I've noticed, can you please introduce the bdev_max_open_zones() 
and bdev_max_open_reagions() functions as well?
Like this (untested):

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bb9e6eb6a7e6..612f4e36828d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1522,6 +1522,15 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
        return 0;
 }
 
+static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
+{
+       struct request_queue *q = bdev_get_queue(bdev);
+
+       if (q)
+               return queue_max_open_zones(q);
+       return 0;
+}
+
 static inline int queue_dma_alignment(const struct request_queue *q)
 {
        return q ? q->dma_alignment : 511;