Re: [RFC PATCH v2 3/4] block: remove queue_is_mq restriction from blk_revalidate_disk_zones()

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2020/05/16 22:14, Coly Li wrote:
> On 2020/5/16 20:40, Christoph Hellwig wrote:
>> On Sat, May 16, 2020 at 11:54:33AM +0800, Coly Li wrote:
>>> The bcache driver is bio based and NOT request based multiqueued driver,
>>> if a zoned SMR hard drive is used as backing device of a bcache device,
>>> calling blk_revalidate_disk_zones() for the bcache device will fail due
>>> to the following check in blk_revalidate_disk_zones(),
>>> 478       if (WARN_ON_ONCE(!queue_is_mq(q)))
>>> 479             return -EIO;
>>>
>>> Now bcache is able to export the zoned information from the underlying
>>> zoned SMR drives and format zonefs on top of a bcache device, the
>>> resitriction that a zoned device should be multiqueued is unnecessary
>>> for now.
>>>
>>> Although in commit ae58954d8734c ("block: don't handle bio based drivers
>>> in blk_revalidate_disk_zones") it is said that bio based drivers should
>>> not call blk_revalidate_disk_zones() and just manually update their own
>>> q->nr_zones, but this is inaccurate. The bio based drivers also need to
>>> set their zone size and initialize bitmaps for cnv and seq zones, it is
>>> necessary to call blk_revalidate_disk_zones() for bio based drivers.
>>
>> Why would you need these bitmaps for bcache?  There is no reason to
>> serialize requests for stacking drivers, and you can already derive
>> if a zone is sequential or not from whatever internal information
>> you use.
>>
>> So without a user that actually makes sense: NAK.
>>
> 
> It is OK for me to set the zone_nr and zone size without calling
> blk_revalidate_disk_zones().

Yes, no problem with that.

This is how device mapper BIO based targets handle zoned devices. See
dm_set_device_limits() which uses bdev_stack_limits() for handling chunk_sectors
(zone size) and directly set q->limits.zoned to blk_queue_zoned_model(q) of the
the underlying device. For the number of zones, see dm_table_set_restrictions()
which uses blkdev_nr_zones(t->md->disk) to set q->nr_zones. Note that
blkdev_nr_zones() uses the gendisk capacity of the logical device (the bcache
device in your case) and the zone size, so both must be set first before calling
blkdev_nr_zones().


> 
> Coly Li
> 


-- 
Damien Le Moal
Western Digital Research