diff mbox series

[v5,04/10] block: Modify revalidate zones

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

Commit Message

Johannes Thumshirn April 9, 2020, 4:53 p.m. UTC
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(-)

Comments

Damien Le Moal April 10, 2020, 12:27 a.m. UTC | #1
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);
>
Christoph Hellwig April 10, 2020, 6:40 a.m. UTC | #2
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,
Damien Le Moal April 10, 2020, 6:55 a.m. UTC | #3
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 mbox series

Patch

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);