Re: [PATCH 2/6] block: Add npo2_zone_setup callback to block device fops

Damien Le Moal
2022-03-09T03:46:29Z

On 3/9/22 01:53, Pankaj Raghav wrote:
> A new fops is added to block device which will be used to setup the
> necessary hooks when a non-power_of_2 zone size is detected in a zoned
> device.
>
> This fops will be called as a part of blk_revalidate_disk_zones.

And what does this new hook do ? You are actually not explaining it, nor
why it should be called from blk_revalidate_disk_zones().

Also, blk_revalidate_zone_cb() uses bit shift but this patch, nor the
previous one fix that.

>
> The primary use case for this callback is to deal with zoned devices
> that does not have a power_of_2 zone size such as ZNS drives.For e.g,
> the current NVMe ZNS specification does not require zone size to be
> power_of_2 but the linux block layer still expects a all zoned device to
> have a power_of_2 zone size.
>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> block/blk-zoned.c | 3 +++
> include/linux/blkdev.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 602bef54c813..d3d821797559 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -575,6 +575,9 @@ int blk_revalidate_disk_zones(struct gendisk *disk,
> if (!get_capacity(disk))
> return -EIO;
>
> + if (disk->fops->npo2_zone_setup)
> + disk->fops->npo2_zone_setup(disk);
> +
> /*
> * Ensure that all memory allocations in this context are done as if
> * GFP_NOIO was specified.
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index a12c031af887..08cf039c1622 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1472,6 +1472,7 @@ struct block_device_operations {
> void (*swap_slot_free_notify) (struct block_device *, unsigned long);
> int (*report_zones)(struct gendisk *, sector_t sector,
> unsigned int nr_zones, report_zones_cb cb, void *data);
> + void (*npo2_zone_setup)(struct gendisk *disk);
> char *(*devnode)(struct gendisk *disk, umode_t *mode);
> /* returns the length of the identifier or a negative errno: */
> int (*get_unique_id)(struct gendisk *disk, u8 id[16],

--
Damien Le Moal
Western Digital Research