| Message ID | 20190611221017.10264-1-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | null_blk: remove duplicate check for report zone | expand |
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@wdc.com> 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@wdc.com> > --- > 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); >
On 6/12/19 6:10 AM, 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@wdc.com> Looks good to me. Reviewed-by: Bob Liu <bob.liu@oracle.com> > --- > 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); >
On 6/11/19 4:10 PM, 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; Applied, thanks.
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);
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@wdc.com> --- drivers/block/null_blk_zoned.c | 4 ---- 1 file changed, 4 deletions(-)