| Message ID | 20200409165352.2126-5-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce Zone Append for writing to zoned block devices | expand |
On 2020/04/10 1:54, Johannes Thumshirn wrote: > From: Damien Le Moal <damien.lemoal@wdc.com> > > Modify the interface of blk_revalidate_disk_zones() to add an optional > revalidation callback function that a driver can use to extend checks and > processing done during zone revalidation. The callback, if defined, is > executed time after all zones are inspected and with the queue frozen. > blk_revalidate_disk_zones() is renamed as __blk_revalidate_disk_zones() > and blk_revalidate_disk_zones() implemented as an inline function calling > __blk_revalidate_disk_zones() without no revalidation callback specified, > resulting in an unchanged behavior for all callers of > blk_revalidate_disk_zones(). > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > block/blk-zoned.c | 19 ++++++++++++++----- > include/linux/blkdev.h | 10 +++++++++- > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 00b025b8b7c0..6c37fec6859b 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -437,20 +437,27 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, > } > > /** > - * blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps > - * @disk: Target disk > + * __blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps > + * @disk: Target disk > + * @revalidate_cb: LLD callback > + * @revalidate_data: LLD callback argument > * > * Helper function for low-level device drivers to (re) allocate and initialize > * a disk request queue zone bitmaps. This functions should normally be called > * within the disk ->revalidate method for blk-mq based drivers. For BIO based > * drivers only q->nr_zones needs to be updated so that the sysfs exposed value > * is correct. > + * If the @revalidate_cb callback function is not NULL, the callback will be > + * executed with the device request queue frozen after all zones have been > + * checked. > */ > -int blk_revalidate_disk_zones(struct gendisk *disk) > +int __blk_revalidate_disk_zones(struct gendisk *disk, > + revalidate_zones_cb revalidate_cb, > + void *revalidate_data) > { > struct request_queue *q = disk->queue; > struct blk_revalidate_zone_args args = { > - .disk = disk, > + .disk = disk, Ooops... whitespace change here. > }; > unsigned int noio_flag; > int ret; > @@ -480,6 +487,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk) > q->nr_zones = args.nr_zones; > swap(q->seq_zones_wlock, args.seq_zones_wlock); > swap(q->conv_zones_bitmap, args.conv_zones_bitmap); > + if (revalidate_cb) > + revalidate_cb(disk, revalidate_data); > ret = 0; > } else { > pr_warn("%s: failed to revalidate zones\n", disk->disk_name); > @@ -491,4 +500,4 @@ int blk_revalidate_disk_zones(struct gendisk *disk) > kfree(args.conv_zones_bitmap); > return ret; > } > -EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones); > +EXPORT_SYMBOL_GPL(__blk_revalidate_disk_zones); > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index e591b22ace03..a730cacda0f7 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -353,6 +353,8 @@ struct queue_limits { > typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx, > void *data); > > +typedef void (*revalidate_zones_cb)(struct gendisk *disk, void *data); > + > #ifdef CONFIG_BLK_DEV_ZONED > > #define BLK_ALL_ZONES ((unsigned int)-1) > @@ -362,7 +364,13 @@ unsigned int blkdev_nr_zones(struct gendisk *disk); > extern int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op, > sector_t sectors, sector_t nr_sectors, > gfp_t gfp_mask); > -extern int blk_revalidate_disk_zones(struct gendisk *disk); > +int __blk_revalidate_disk_zones(struct gendisk *disk, > + revalidate_zones_cb revalidate_cb, > + void *revalidate_data); > +static inline int blk_revalidate_disk_zones(struct gendisk *disk) > +{ > + return __blk_revalidate_disk_zones(disk, NULL, NULL); > +} > > extern int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, > unsigned int cmd, unsigned long arg); >
On Fri, Apr 10, 2020 at 01:53:46AM +0900, Johannes Thumshirn wrote: > From: Damien Le Moal <damien.lemoal@wdc.com> > > Modify the interface of blk_revalidate_disk_zones() to add an optional > revalidation callback function that a driver can use to extend checks and > processing done during zone revalidation. The callback, if defined, is > executed time after all zones are inspected and with the queue frozen. > blk_revalidate_disk_zones() is renamed as __blk_revalidate_disk_zones() > and blk_revalidate_disk_zones() implemented as an inline function calling > __blk_revalidate_disk_zones() without no revalidation callback specified, > resulting in an unchanged behavior for all callers of > blk_revalidate_disk_zones(). The data argument to __blk_revalidate_disk_zones and the cllback is now unused. I also think we now merge __blk_revalidate_disk_zones and blk_revalidate_disk_zones instead of having two versions for a grand total of two callers. Something like this on top of your whole branch: diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 6c37fec6859b..0e7763a590e5 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -437,23 +437,21 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, } /** - * __blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps + * blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps * @disk: Target disk - * @revalidate_cb: LLD callback - * @revalidate_data: LLD callback argument + * @update_driver_data: Callback to update driver data on the frozen disk * * Helper function for low-level device drivers to (re) allocate and initialize * a disk request queue zone bitmaps. This functions should normally be called * within the disk ->revalidate method for blk-mq based drivers. For BIO based * drivers only q->nr_zones needs to be updated so that the sysfs exposed value * is correct. - * If the @revalidate_cb callback function is not NULL, the callback will be - * executed with the device request queue frozen after all zones have been + * If the @update_driver_data callback function is not NULL, the callback will + * be executed with the device request queue frozen after all zones have been * checked. */ -int __blk_revalidate_disk_zones(struct gendisk *disk, - revalidate_zones_cb revalidate_cb, - void *revalidate_data) +int blk_revalidate_disk_zones(struct gendisk *disk, + void (*update_driver_data)(struct gendisk *disk)) { struct request_queue *q = disk->queue; struct blk_revalidate_zone_args args = { @@ -487,8 +485,8 @@ int __blk_revalidate_disk_zones(struct gendisk *disk, q->nr_zones = args.nr_zones; swap(q->seq_zones_wlock, args.seq_zones_wlock); swap(q->conv_zones_bitmap, args.conv_zones_bitmap); - if (revalidate_cb) - revalidate_cb(disk, revalidate_data); + if (update_driver_data) + update_driver_data(disk); ret = 0; } else { pr_warn("%s: failed to revalidate zones\n", disk->disk_name); @@ -500,4 +498,4 @@ int __blk_revalidate_disk_zones(struct gendisk *disk, kfree(args.conv_zones_bitmap); return ret; } -EXPORT_SYMBOL_GPL(__blk_revalidate_disk_zones); +EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones); diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c index b664be0bbb5e..f7beb72a321a 100644 --- a/drivers/block/null_blk_zoned.c +++ b/drivers/block/null_blk_zoned.c @@ -71,7 +71,7 @@ int null_register_zoned_dev(struct nullb *nullb) struct request_queue *q = nullb->q; if (queue_is_mq(q)) { - int ret = blk_revalidate_disk_zones(nullb->disk); + int ret = blk_revalidate_disk_zones(nullb->disk, NULL); if (ret) return ret; diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c index 53cfe998a3f6..893d2e0da255 100644 --- a/drivers/scsi/sd_zbc.c +++ b/drivers/scsi/sd_zbc.c @@ -656,7 +656,7 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf, return 0; } -static void sd_zbc_revalidate_zones_cb(struct gendisk *disk, void *data) +static void sd_zbc_update_zone_data(struct gendisk *disk) { struct scsi_disk *sdkp = scsi_disk(disk); @@ -680,8 +680,7 @@ static int sd_zbc_revalidate_zones(struct scsi_disk *sdkp, goto unlock; } - ret = __blk_revalidate_disk_zones(sdkp->disk, - sd_zbc_revalidate_zones_cb, NULL); + ret = blk_revalidate_disk_zones(sdkp->disk, sd_zbc_update_zone_data); kvfree(sdkp->rev_wp_ofst); sdkp->rev_wp_ofst = NULL; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index a730cacda0f7..d970c36cb8d3 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -353,8 +353,6 @@ struct queue_limits { typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx, void *data); -typedef void (*revalidate_zones_cb)(struct gendisk *disk, void *data); - #ifdef CONFIG_BLK_DEV_ZONED #define BLK_ALL_ZONES ((unsigned int)-1) @@ -364,14 +362,8 @@ unsigned int blkdev_nr_zones(struct gendisk *disk); extern int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op, sector_t sectors, sector_t nr_sectors, gfp_t gfp_mask); -int __blk_revalidate_disk_zones(struct gendisk *disk, - revalidate_zones_cb revalidate_cb, - void *revalidate_data); -static inline int blk_revalidate_disk_zones(struct gendisk *disk) -{ - return __blk_revalidate_disk_zones(disk, NULL, NULL); -} - +int blk_revalidate_disk_zones(struct gendisk *disk, + void (*update_driver_data)(struct gendisk *disk)); extern int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg); extern int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode,
On 2020/04/10 15:40, Christoph Hellwig wrote: > On Fri, Apr 10, 2020 at 01:53:46AM +0900, Johannes Thumshirn wrote: >> From: Damien Le Moal <damien.lemoal@wdc.com> >> >> Modify the interface of blk_revalidate_disk_zones() to add an optional >> revalidation callback function that a driver can use to extend checks and >> processing done during zone revalidation. The callback, if defined, is >> executed time after all zones are inspected and with the queue frozen. >> blk_revalidate_disk_zones() is renamed as __blk_revalidate_disk_zones() >> and blk_revalidate_disk_zones() implemented as an inline function calling >> __blk_revalidate_disk_zones() without no revalidation callback specified, >> resulting in an unchanged behavior for all callers of >> blk_revalidate_disk_zones(). > > The data argument to __blk_revalidate_disk_zones and the cllback is now > unused. I also think we now merge __blk_revalidate_disk_zones and > blk_revalidate_disk_zones instead of having two versions for a grand > total of two callers. Something like this on top of your whole branch: Yes, indeed. Even simpler. Will add this change to v6. > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 6c37fec6859b..0e7763a590e5 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -437,23 +437,21 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, > } > > /** > - * __blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps > + * blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps > * @disk: Target disk > - * @revalidate_cb: LLD callback > - * @revalidate_data: LLD callback argument > + * @update_driver_data: Callback to update driver data on the frozen disk > * > * Helper function for low-level device drivers to (re) allocate and initialize > * a disk request queue zone bitmaps. This functions should normally be called > * within the disk ->revalidate method for blk-mq based drivers. For BIO based > * drivers only q->nr_zones needs to be updated so that the sysfs exposed value > * is correct. > - * If the @revalidate_cb callback function is not NULL, the callback will be > - * executed with the device request queue frozen after all zones have been > + * If the @update_driver_data callback function is not NULL, the callback will > + * be executed with the device request queue frozen after all zones have been > * checked. > */ > -int __blk_revalidate_disk_zones(struct gendisk *disk, > - revalidate_zones_cb revalidate_cb, > - void *revalidate_data) > +int blk_revalidate_disk_zones(struct gendisk *disk, > + void (*update_driver_data)(struct gendisk *disk)) > { > struct request_queue *q = disk->queue; > struct blk_revalidate_zone_args args = { > @@ -487,8 +485,8 @@ int __blk_revalidate_disk_zones(struct gendisk *disk, > q->nr_zones = args.nr_zones; > swap(q->seq_zones_wlock, args.seq_zones_wlock); > swap(q->conv_zones_bitmap, args.conv_zones_bitmap); > - if (revalidate_cb) > - revalidate_cb(disk, revalidate_data); > + if (update_driver_data) > + update_driver_data(disk); > ret = 0; > } else { > pr_warn("%s: failed to revalidate zones\n", disk->disk_name); > @@ -500,4 +498,4 @@ int __blk_revalidate_disk_zones(struct gendisk *disk, > kfree(args.conv_zones_bitmap); > return ret; > } > -EXPORT_SYMBOL_GPL(__blk_revalidate_disk_zones); > +EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones); > diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c > index b664be0bbb5e..f7beb72a321a 100644 > --- a/drivers/block/null_blk_zoned.c > +++ b/drivers/block/null_blk_zoned.c > @@ -71,7 +71,7 @@ int null_register_zoned_dev(struct nullb *nullb) > struct request_queue *q = nullb->q; > > if (queue_is_mq(q)) { > - int ret = blk_revalidate_disk_zones(nullb->disk); > + int ret = blk_revalidate_disk_zones(nullb->disk, NULL); > > if (ret) > return ret; > diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c > index 53cfe998a3f6..893d2e0da255 100644 > --- a/drivers/scsi/sd_zbc.c > +++ b/drivers/scsi/sd_zbc.c > @@ -656,7 +656,7 @@ static int sd_zbc_check_capacity(struct scsi_disk *sdkp, unsigned char *buf, > return 0; > } > > -static void sd_zbc_revalidate_zones_cb(struct gendisk *disk, void *data) > +static void sd_zbc_update_zone_data(struct gendisk *disk) > { > struct scsi_disk *sdkp = scsi_disk(disk); > > @@ -680,8 +680,7 @@ static int sd_zbc_revalidate_zones(struct scsi_disk *sdkp, > goto unlock; > } > > - ret = __blk_revalidate_disk_zones(sdkp->disk, > - sd_zbc_revalidate_zones_cb, NULL); > + ret = blk_revalidate_disk_zones(sdkp->disk, sd_zbc_update_zone_data); > kvfree(sdkp->rev_wp_ofst); > sdkp->rev_wp_ofst = NULL; > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index a730cacda0f7..d970c36cb8d3 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -353,8 +353,6 @@ struct queue_limits { > typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx, > void *data); > > -typedef void (*revalidate_zones_cb)(struct gendisk *disk, void *data); > - > #ifdef CONFIG_BLK_DEV_ZONED > > #define BLK_ALL_ZONES ((unsigned int)-1) > @@ -364,14 +362,8 @@ unsigned int blkdev_nr_zones(struct gendisk *disk); > extern int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op, > sector_t sectors, sector_t nr_sectors, > gfp_t gfp_mask); > -int __blk_revalidate_disk_zones(struct gendisk *disk, > - revalidate_zones_cb revalidate_cb, > - void *revalidate_data); > -static inline int blk_revalidate_disk_zones(struct gendisk *disk) > -{ > - return __blk_revalidate_disk_zones(disk, NULL, NULL); > -} > - > +int blk_revalidate_disk_zones(struct gendisk *disk, > + void (*update_driver_data)(struct gendisk *disk)); > extern int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, > unsigned int cmd, unsigned long arg); > extern int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, >
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 00b025b8b7c0..6c37fec6859b 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -437,20 +437,27 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx, } /** - * blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps - * @disk: Target disk + * __blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps + * @disk: Target disk + * @revalidate_cb: LLD callback + * @revalidate_data: LLD callback argument * * Helper function for low-level device drivers to (re) allocate and initialize * a disk request queue zone bitmaps. This functions should normally be called * within the disk ->revalidate method for blk-mq based drivers. For BIO based * drivers only q->nr_zones needs to be updated so that the sysfs exposed value * is correct. + * If the @revalidate_cb callback function is not NULL, the callback will be + * executed with the device request queue frozen after all zones have been + * checked. */ -int blk_revalidate_disk_zones(struct gendisk *disk) +int __blk_revalidate_disk_zones(struct gendisk *disk, + revalidate_zones_cb revalidate_cb, + void *revalidate_data) { struct request_queue *q = disk->queue; struct blk_revalidate_zone_args args = { - .disk = disk, + .disk = disk, }; unsigned int noio_flag; int ret; @@ -480,6 +487,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk) q->nr_zones = args.nr_zones; swap(q->seq_zones_wlock, args.seq_zones_wlock); swap(q->conv_zones_bitmap, args.conv_zones_bitmap); + if (revalidate_cb) + revalidate_cb(disk, revalidate_data); ret = 0; } else { pr_warn("%s: failed to revalidate zones\n", disk->disk_name); @@ -491,4 +500,4 @@ int blk_revalidate_disk_zones(struct gendisk *disk) kfree(args.conv_zones_bitmap); return ret; } -EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones); +EXPORT_SYMBOL_GPL(__blk_revalidate_disk_zones); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index e591b22ace03..a730cacda0f7 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -353,6 +353,8 @@ struct queue_limits { typedef int (*report_zones_cb)(struct blk_zone *zone, unsigned int idx, void *data); +typedef void (*revalidate_zones_cb)(struct gendisk *disk, void *data); + #ifdef CONFIG_BLK_DEV_ZONED #define BLK_ALL_ZONES ((unsigned int)-1) @@ -362,7 +364,13 @@ unsigned int blkdev_nr_zones(struct gendisk *disk); extern int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op, sector_t sectors, sector_t nr_sectors, gfp_t gfp_mask); -extern int blk_revalidate_disk_zones(struct gendisk *disk); +int __blk_revalidate_disk_zones(struct gendisk *disk, + revalidate_zones_cb revalidate_cb, + void *revalidate_data); +static inline int blk_revalidate_disk_zones(struct gendisk *disk) +{ + return __blk_revalidate_disk_zones(disk, NULL, NULL); +} extern int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode, unsigned int cmd, unsigned long arg);