On 2019/06/12 7:10, Chaitanya Kulkarni wrote:
> This patch removes the check in the null_blk_zoned for report zone
> command, where it checks for the dev-,>zoned before executing the report
> zone.
>
> The null_zone_report() function is a block_device operation callback
> which is initialized in the null_blk_main.c and gets called as a part
> of blkdev for report zone IOCTL (BLKREPORTZONE).
>
> blkdev_ioctl()
> blkdev_report_zones_ioctl()
> blkdev_report_zones()
> blk_report_zones()
> disk->fops->report_zones()
> nullb_zone_report();
>
> The null_zone_report() will never get executed on the non-zoned block
> device, in the non zoned block device blk_queue_is_zoned() will always
> be false which is first check the blkdev_report_zones_ioctl()
> before actual low level driver report zone callback is executed.
>
> Here is the detailed scenario:-
>
> 1. modprobe null_blk
> null_init
> null_alloc_dev
> dev->zoned = 0
> null_add_dev
> dev->zoned == 0
> so we don't set the q->limits.zoned = BLK_ZONED_HR
>
> 2. blkzone report /dev/nullb0
>
> blkdev_ioctl()
> blkdev_report_zones_ioctl()
> blk_queue_is_zoned()
> blk_queue_is_zoned
> q->limits.zoned == 0
> return false
> if (!blk_queue_is_zoned(q)) <--- true
> return -ENOTTY;
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
Looks good. The SCSI layer has a similar check too. But since there are scsi
layer internal execution of the report zones driver function (e.g. device scan),
we need to keep that check there. But I think it is safe to remove in nullblk
since revalidate is the only pass other than the user ioctl that would call
report zones.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
> ---
> drivers/block/null_blk_zoned.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
> index 5d1c261a2cfd..fca0c97ff1aa 100644
> --- a/drivers/block/null_blk_zoned.c
> +++ b/drivers/block/null_blk_zoned.c
> @@ -74,10 +74,6 @@ int null_zone_report(struct gendisk *disk, sector_t sector,
> struct nullb_device *dev = nullb->dev;
> unsigned int zno, nrz = 0;
>
> - if (!dev->zoned)
> - /* Not a zoned null device */
> - return -EOPNOTSUPP;
> -
> zno = null_zone_no(dev, sector);
> if (zno < dev->nr_zones) {
> nrz = min_t(unsigned int, *nr_zones, dev->nr_zones - zno);
>
--
Damien Le Moal
Western Digital Research
