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@xxxxxxx>
>>
>> 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,
>
--
Damien Le Moal
Western Digital Research
