linux-kernel - Re: [PATCH] block: ublk: enable zoned storage support

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: <87edqec453.fsf@metaspace.dk>
Date:   Fri, 24 Feb 2023 19:20:03 +0100
From:   Andreas Hindborg <nmi@...aspace.dk>
To:     Niklas Cassel <Niklas.Cassel@....com>
Cc:     "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Hans Holmberg <Hans.Holmberg@....com>,
        Matias Bjørling <Matias.Bjorling@....com>,
        Ming Lei <ming.lei@...hat.com>, Jens Axboe <axboe@...nel.dk>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] block: ublk: enable zoned storage support


Niklas Cassel <Niklas.Cassel@....com> writes:

> On Fri, Feb 24, 2023 at 04:53:52PM +0100, Niklas Cassel wrote:
>> Hello Andreas,
>> 
>> On Fri, Feb 24, 2023 at 01:59:50PM +0100, Andreas Hindborg wrote:
>> > Add zoned storage support to ublk: report_zones and operations:
>> >  - REQ_OP_ZONE_OPEN
>> >  - REQ_OP_ZONE_CLOSE
>> >  - REQ_OP_ZONE_FINISH
>> >  - REQ_OP_ZONE_RESET
>> > 
>> > This allows implementation of zoned storage devices in user space. An
>> > example user space implementation based on ubdsrv is available [1].
>> > 
>> > [1] https://github.com/metaspace/ubdsrv/commit/14a2b708f74f70cfecb076d92e680dc718cc1f6d
>> > 
>> > Signed-off-by: Andreas Hindborg <a.hindborg@...sung.com>
>> > ---
>> 
>> Did you try to build this patch with CONFIG_BLK_DEV_ZONED disabled?
>> 
>> I got the following build errors when building it on top of vanilla v6.2
>> when CONFIG_BLK_DEV_ZONED is disabled:
>> 
>> drivers/block/ublk_drv.c: In function ‘ublk_dev_param_basic_apply’:
>> drivers/block/ublk_drv.c:221:28: error: ‘struct gendisk’ has no member named ‘nr_zones’
>>   221 |                 ub->ub_disk->nr_zones = p->dev_sectors / p->chunk_sectors;
>>       |                            ^~
>> drivers/block/ublk_drv.c: In function ‘ublk_dev_param_zoned_apply’:
>> drivers/block/ublk_drv.c:244:17: error: implicit declaration of function ‘disk_set_max_active_zones’; did you mean ‘bdev_max_active_zones’? [-Werror=implicit-function-declaration]
>>   244 |                 disk_set_max_active_zones(ub->ub_disk, p->max_active_zones);
>>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~
>>       |                 bdev_max_active_zones
>> drivers/block/ublk_drv.c:245:17: error: implicit declaration of function ‘disk_set_max_open_zones’; did you mean ‘bdev_max_open_zones’? [-Werror=implicit-function-declaration]
>>   245 |                 disk_set_max_open_zones(ub->ub_disk, p->max_open_zones);
>> 
>
> The problem here is probably that blkdev.h does not have dummy functions for
> disk_set_max_active_zones() and disk_set_max_open_zones()
>
> in the:
> #else /* CONFIG_BLK_DEV_ZONED */
> case.
>
> I do see dummy functions in blkdev.h for disk_nr_zones(), disk_zone_is_seq()
> and disk_zone_no() when CONFIG_BLK_DEV_ZONED is not set.

Thanks for the comments Niklas :)

I completely forgot to test without CONFIG_BLK_DEV_ZONED 🤦 What is the
preferred resolution here? Conditionally remove the lines with #ifdef
rather than use if(IS_ENABLED(...))? Dummy functions would resolve lines
244 and 245, but would not help with assignment of nr_zones at 221 as
that will not exist when CONFIG_BLK_DEV_ZONED is disabled.

BR Andreas