| Message ID | 20240325044452.3125418-13-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Zone write plugging | expand |
> + /* > + * For devices using a BIO-based driver, we need zone resources only > + * if zone append emulation is required. > + */ > + if (!queue_is_mq(disk->queue) && > + !queue_emulates_zone_append(disk->queue)) > + return 0; > + This code and the comment is duplicated in two places. It also fails to capture why bio drivers don't need zoned resources - that is that we can't enforce QD=1 per zone for them. Maybe add a little helper to encapsulate this check and expand the comment a little? Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 3/26/24 16:08, Christoph Hellwig wrote: >> + /* >> + * For devices using a BIO-based driver, we need zone resources only >> + * if zone append emulation is required. >> + */ >> + if (!queue_is_mq(disk->queue) && >> + !queue_emulates_zone_append(disk->queue)) >> + return 0; >> + > > This code and the comment is duplicated in two places. It also fails > to capture why bio drivers don't need zoned resources - that is that > we can't enforce QD=1 per zone for them. Maybe add a little helper > to encapsulate this check and expand the comment a little? That is not the reason. Because we can actually enforce QD=1 write per zone for DM devices with zone write plugging. E.g. dm-crypt requires that. The reason is more that most DM devices don't need to have that enforcement as they are responsible for handling the sequential writing of zones themselves. So no hash table and no conv bitmap needed for them, unless like dm-crypt, they request zone append emulation, in which case, zone write plugging is turned on for the DM device. But I agree about the helper. Will make that change. > > Otherwise looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> >
On 3/25/24 05:44, Damien Le Moal wrote: > In preparation for allowing BIO based device drivers to use zone write > plugging and its zone append emulation, allow these drivers to call > blk_revalidate_disk_zones() so that all zone resources necessary to zone > write plugging can be initialized. > > To do so, remove the check in blk_revalidate_disk_zones() restricting > the use of this function to mq request-based drivers to allow also > BIO-based drivers to use it. This is safe to do as long as the > BIO-based block device queue is already setup and usable, as it should, > and can be safely frozen. The zone write plug hash table and > conventional zone bitmap are allocated and initialized only for mq > devices and for BIO-based devices that require zone append emulation. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > block/blk-zoned.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 5b86f1aa80f0..8dc7cdc539c6 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -1481,6 +1481,14 @@ static int disk_revalidate_zone_resources(struct gendisk *disk, unsigned int hash_size; int ret; + /* + * For devices using a BIO-based driver, we need zone resources only + * if zone append emulation is required. + */ + if (!queue_is_mq(disk->queue) && + !queue_emulates_zone_append(disk->queue)) + return 0; + /* * If the device has no limit on the maximum number of open and active * zones, set the max_open_zones queue limit to indicate the size of @@ -1571,6 +1579,13 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, /* Check zone type */ switch (zone->type) { case BLK_ZONE_TYPE_CONVENTIONAL: + /* + * For devices using a BIO-based driver, we need zone resources + * only if zone append emulation is required. + */ + if (!queue_is_mq(disk->queue) && + !queue_emulates_zone_append(disk->queue)) + break; if (!args->conv_zones_bitmap) { args->conv_zones_bitmap = blk_alloc_zone_bitmap(q->node, args->nr_zones); @@ -1602,10 +1617,11 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, /* * We need to track the write pointer of all zones that are not * empty nor full. So make sure we have a zone write plug for - * such zone. + * such zone if the device has a zone write plug hash table. */ wp_offset = blk_zone_wp_offset(zone); - if (wp_offset && wp_offset < zone_sectors) { + if (disk->zone_wplugs_hash && + wp_offset && wp_offset < zone_sectors) { zwplug = disk_get_zone_wplug_locked(disk, zone->start, GFP_NOIO, &flags); if (!zwplug) @@ -1636,8 +1652,8 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, * be called within the disk ->revalidate method for blk-mq based drivers. * Before calling this function, the device driver must already have set the * device zone size (chunk_sector limit) and the max zone append limit. - * For BIO based drivers, this function cannot be used. BIO based device drivers - * only need to set disk->nr_zones so that the sysfs exposed value is correct. + * BIO based drivers can also use this function as long as the device queue + * can be safely frozen. * If the @update_driver_data callback function is not NULL, the callback is * executed with the device request queue frozen after all zones have been * checked. @@ -1654,8 +1670,6 @@ int blk_revalidate_disk_zones(struct gendisk *disk, if (WARN_ON_ONCE(!blk_queue_is_zoned(q))) return -EIO; - if (WARN_ON_ONCE(!queue_is_mq(q))) - return -EIO; if (!capacity) return -ENODEV;
In preparation for allowing BIO based device drivers to use zone write plugging and its zone append emulation, allow these drivers to call blk_revalidate_disk_zones() so that all zone resources necessary to zone write plugging can be initialized. To do so, remove the check in blk_revalidate_disk_zones() restricting the use of this function to mq request-based drivers to allow also BIO-based drivers to use it. This is safe to do as long as the BIO-based block device queue is already setup and usable, as it should, and can be safely frozen. The zone write plug hash table and conventional zone bitmap are allocated and initialized only for mq devices and for BIO-based devices that require zone append emulation. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- block/blk-zoned.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)