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

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

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