| Message ID | 20240325044452.3125418-7-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Zone write plugging | expand |
On 3/24/24 21:44, Damien Le Moal wrote: > + /* > + * Remember the capacity of the first sequential zone and check > + * if it is constant for all zones. > + */ > + if (!args->zone_capacity) > + args->zone_capacity = zone->capacity; > + if (zone->capacity != args->zone_capacity) { > + pr_warn("%s: Invalid variable zone capacity\n", > + disk->disk_name); > + return -ENODEV; > + } The above code won't refuse devices for which the first few zones have capacity zero. Shouldn't these be rejected? Thanks, Bart.
On 3/26/24 06:53, Bart Van Assche wrote: > On 3/24/24 21:44, Damien Le Moal wrote: >> + /* >> + * Remember the capacity of the first sequential zone and check >> + * if it is constant for all zones. >> + */ >> + if (!args->zone_capacity) >> + args->zone_capacity = zone->capacity; >> + if (zone->capacity != args->zone_capacity) { >> + pr_warn("%s: Invalid variable zone capacity\n", >> + disk->disk_name); >> + return -ENODEV; >> + } > > The above code won't refuse devices for which the first few zones have > capacity zero. Shouldn't these be rejected? The device driver is supposed to ensure that zone capacity is always set to the device reported value or to the zone size for devices that do not have a zone capacity smaller than the zone size. But sure, I can add a check.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 3/25/24 05:44, Damien Le Moal wrote: > In preparation for adding zone write plugging, modify > blk_revalidate_disk_zones() to get the capacity of zones of a zoned > block device. This capacity value as a number of 512B sectors is stored > in the gendisk zone_capacity field. > > Given that host-managed SMR disks (including zoned UFS drives) and all > known NVMe ZNS devices have the same zone capacity for all zones > blk_revalidate_disk_zones() returns an error if different capacities are > detected for different zones. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > block/blk-zoned.c | 15 +++++++++++++++ > include/linux/blkdev.h | 1 + > 2 files changed, 16 insertions(+) > That again. We should never have allowed zones with different sizes. Anyway. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index da0f4b2a8fa0..88110a1329cd 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -438,6 +438,7 @@ struct blk_revalidate_zone_args { unsigned long *conv_zones_bitmap; unsigned long *seq_zones_wlock; unsigned int nr_zones; + unsigned int zone_capacity; sector_t sector; }; @@ -500,6 +501,18 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, if (!args->seq_zones_wlock) return -ENOMEM; } + + /* + * Remember the capacity of the first sequential zone and check + * if it is constant for all zones. + */ + if (!args->zone_capacity) + args->zone_capacity = zone->capacity; + if (zone->capacity != args->zone_capacity) { + pr_warn("%s: Invalid variable zone capacity\n", + disk->disk_name); + return -ENODEV; + } break; case BLK_ZONE_TYPE_SEQWRITE_PREF: default: @@ -597,6 +610,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk, disk->nr_zones = args.nr_zones; swap(disk->seq_zones_wlock, args.seq_zones_wlock); swap(disk->conv_zones_bitmap, args.conv_zones_bitmap); + disk->zone_capacity = args.zone_capacity; if (update_driver_data) update_driver_data(disk); ret = 0; @@ -608,6 +622,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk, kfree(args.seq_zones_wlock); kfree(args.conv_zones_bitmap); + return ret; } EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 93f93f352b54..b57244ee74f0 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -191,6 +191,7 @@ struct gendisk { * blk_mq_unfreeze_queue(). */ unsigned int nr_zones; + unsigned int zone_capacity; unsigned long *conv_zones_bitmap; unsigned long *seq_zones_wlock; #endif /* CONFIG_BLK_DEV_ZONED */
In preparation for adding zone write plugging, modify blk_revalidate_disk_zones() to get the capacity of zones of a zoned block device. This capacity value as a number of 512B sectors is stored in the gendisk zone_capacity field. Given that host-managed SMR disks (including zoned UFS drives) and all known NVMe ZNS devices have the same zone capacity for all zones blk_revalidate_disk_zones() returns an error if different capacities are detected for different zones. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- block/blk-zoned.c | 15 +++++++++++++++ include/linux/blkdev.h | 1 + 2 files changed, 16 insertions(+)