[PATCH] block: fix revalidate performance regression

[PATCH] block: fix revalidate performance regression

[Damien Le Moal]

The scsi driver function sd_read_block_characteristics() always calls
disk_set_zoned() to a disk zoned model correctly, in case the device
model changed. This is done even for regular disks to set the zoned
model to BLK_ZONED_NONE and free any zone related resources if the drive
previously was zoned.

This behavior significantly impact the time it takes to revalidate disks
on a large system as the call to disk_clear_zone_settings() done from
disk_set_zoned() for the BLK_ZONED_NONE case results in the device
request queued to be quiesced, even if there is no zone resource to
free.

Avoid this overhead for non zoned devices by not calling
disk_clear_zone_settings() in disk_set_zoned() if the device model
already was set to BLK_ZONED_NONE.

Reported by: Brian Bunker <brian@purestorage.com>
Fixes: 508aebb80527 ("block: introduce blk_queue_clear_zone_settings()")
Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 block/blk-settings.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 896b4654ab00..4dd59059b788 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -915,6 +915,7 @@ static bool disk_has_partitions(struct gendisk *disk)
 void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
 {
 	struct request_queue *q = disk->queue;
+	unsigned int old_model = q->limits.zoned;
 
 	switch (model) {
 	case BLK_ZONED_HM:
@@ -952,7 +953,7 @@ void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
 		 */
 		blk_queue_zone_write_granularity(q,
 						queue_logical_block_size(q));
-	} else {
+	} else if (old_model != BLK_ZONED_NONE) {
 		disk_clear_zone_settings(disk);
 	}
 }
-- 
2.40.1

Re: [PATCH] block: fix revalidate performance regression

[Ming Lei]

On Mon, May 29, 2023 at 10:40 AM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> The scsi driver function sd_read_block_characteristics() always calls
> disk_set_zoned() to a disk zoned model correctly, in case the device
> model changed. This is done even for regular disks to set the zoned
> model to BLK_ZONED_NONE and free any zone related resources if the drive
> previously was zoned.
>
> This behavior significantly impact the time it takes to revalidate disks
> on a large system as the call to disk_clear_zone_settings() done from
> disk_set_zoned() for the BLK_ZONED_NONE case results in the device
> request queued to be quiesced, even if there is no zone resource to

s/quiesced/frozen

> free.
>
> Avoid this overhead for non zoned devices by not calling
> disk_clear_zone_settings() in disk_set_zoned() if the device model
> already was set to BLK_ZONED_NONE.
>
> Reported by: Brian Bunker <brian@purestorage.com>
> Fixes: 508aebb80527 ("block: introduce blk_queue_clear_zone_settings()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Re: [PATCH] block: fix revalidate performance regression

[Hannes Reinecke]

On 5/29/23 04:38, Damien Le Moal wrote:
> The scsi driver function sd_read_block_characteristics() always calls
> disk_set_zoned() to a disk zoned model correctly, in case the device
> model changed. This is done even for regular disks to set the zoned
> model to BLK_ZONED_NONE and free any zone related resources if the drive
> previously was zoned.
> 
> This behavior significantly impact the time it takes to revalidate disks
> on a large system as the call to disk_clear_zone_settings() done from
> disk_set_zoned() for the BLK_ZONED_NONE case results in the device
> request queued to be quiesced, even if there is no zone resource to
> free.
> 
> Avoid this overhead for non zoned devices by not calling
> disk_clear_zone_settings() in disk_set_zoned() if the device model
> already was set to BLK_ZONED_NONE.
> 
> Reported by: Brian Bunker <brian@purestorage.com>
> Fixes: 508aebb80527 ("block: introduce blk_queue_clear_zone_settings()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   block/blk-settings.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 896b4654ab00..4dd59059b788 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -915,6 +915,7 @@ static bool disk_has_partitions(struct gendisk *disk)
>   void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
>   {
>   	struct request_queue *q = disk->queue;
> +	unsigned int old_model = q->limits.zoned;
>   
>   	switch (model) {
>   	case BLK_ZONED_HM:
> @@ -952,7 +953,7 @@ void disk_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
>   		 */
>   		blk_queue_zone_write_granularity(q,
>   						queue_logical_block_size(q));
> -	} else {
> +	} else if (old_model != BLK_ZONED_NONE) {
>   		disk_clear_zone_settings(disk);
>   	}
>   }
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman