Re: [PATCH] null_blk: remove duplicate check for report zone

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux