[PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[Damien Le Moal]

Improves the checks on the zones of a zoned block device done in
blk_revalidate_disk_zones() by making sure that the device report_zones
method did report at least one zone and that the zones reported exactly
cover the entire disk capacity, that is, that there are no missing zones
at the end of the disk sector range.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
Changes from V1:
* Add checks on the number of zones reported
* Check the reported zone coverage only if zones are successfully
  reported
* Reword the commit message

 block/blk-zoned.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 6817a673e5ce..7a68b6e4300c 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -508,15 +508,29 @@ int blk_revalidate_disk_zones(struct gendisk *disk,
 	noio_flag = memalloc_noio_save();
 	ret = disk->fops->report_zones(disk, 0, UINT_MAX,
 				       blk_revalidate_zone_cb, &args);
+	if (!ret) {
+		pr_warn("%s: No zones reported\n", disk->disk_name);
+		ret = -ENODEV;
+	}
 	memalloc_noio_restore(noio_flag);
 
+	/*
+	 * If zones where reported, make sure that the entire disk capacity
+	 * has been checked.
+	 */
+	if (ret > 0 && args.sector != get_capacity(disk)) {
+		pr_warn("%s: Missing zones from sector %llu\n",
+			disk->disk_name, args.sector);
+		ret = -ENODEV;
+	}
+
 	/*
 	 * Install the new bitmaps and update nr_zones only once the queue is
 	 * stopped and all I/Os are completed (i.e. a scheduler is not
 	 * referencing the bitmaps).
 	 */
 	blk_mq_freeze_queue(q);
-	if (ret >= 0) {
+	if (ret > 0) {
 		blk_queue_chunk_sectors(q, args.zone_sectors);
 		q->nr_zones = args.nr_zones;
 		swap(q->seq_zones_wlock, args.seq_zones_wlock);
-- 
2.26.2

Re: [PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[Christoph Hellwig]

On Wed, Nov 11, 2020 at 04:36:06PM +0900, Damien Le Moal wrote:
> Improves the checks on the zones of a zoned block device done in
> blk_revalidate_disk_zones() by making sure that the device report_zones
> method did report at least one zone and that the zones reported exactly
> cover the entire disk capacity, that is, that there are no missing zones
> at the end of the disk sector range.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[Damien Le Moal]

On 2020/11/11 16:36, Damien Le Moal wrote:
> Improves the checks on the zones of a zoned block device done in
> blk_revalidate_disk_zones() by making sure that the device report_zones
> method did report at least one zone and that the zones reported exactly
> cover the entire disk capacity, that is, that there are no missing zones
> at the end of the disk sector range.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> Changes from V1:
> * Add checks on the number of zones reported
> * Check the reported zone coverage only if zones are successfully
>   reported
> * Reword the commit message
> 
>  block/blk-zoned.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 6817a673e5ce..7a68b6e4300c 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -508,15 +508,29 @@ int blk_revalidate_disk_zones(struct gendisk *disk,
>  	noio_flag = memalloc_noio_save();
>  	ret = disk->fops->report_zones(disk, 0, UINT_MAX,
>  				       blk_revalidate_zone_cb, &args);
> +	if (!ret) {
> +		pr_warn("%s: No zones reported\n", disk->disk_name);
> +		ret = -ENODEV;
> +	}
>  	memalloc_noio_restore(noio_flag);
>  
> +	/*
> +	 * If zones where reported, make sure that the entire disk capacity
> +	 * has been checked.
> +	 */
> +	if (ret > 0 && args.sector != get_capacity(disk)) {
> +		pr_warn("%s: Missing zones from sector %llu\n",
> +			disk->disk_name, args.sector);
> +		ret = -ENODEV;
> +	}
> +
>  	/*
>  	 * Install the new bitmaps and update nr_zones only once the queue is
>  	 * stopped and all I/Os are completed (i.e. a scheduler is not
>  	 * referencing the bitmaps).
>  	 */
>  	blk_mq_freeze_queue(q);
> -	if (ret >= 0) {
> +	if (ret > 0) {
>  		blk_queue_chunk_sectors(q, args.zone_sectors);
>  		q->nr_zones = args.nr_zones;
>  		swap(q->seq_zones_wlock, args.seq_zones_wlock);
> 

Jens,

I did not see this patch queued up in your tree.
Could you consider it please ?

Thanks !

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[Damien Le Moal]

On 2020/11/11 16:36, Damien Le Moal wrote:
> Improves the checks on the zones of a zoned block device done in
> blk_revalidate_disk_zones() by making sure that the device report_zones
> method did report at least one zone and that the zones reported exactly
> cover the entire disk capacity, that is, that there are no missing zones
> at the end of the disk sector range.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Jens,

Ping ?


> ---
> Changes from V1:
> * Add checks on the number of zones reported
> * Check the reported zone coverage only if zones are successfully
>   reported
> * Reword the commit message
> 
>  block/blk-zoned.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 6817a673e5ce..7a68b6e4300c 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -508,15 +508,29 @@ int blk_revalidate_disk_zones(struct gendisk *disk,
>  	noio_flag = memalloc_noio_save();
>  	ret = disk->fops->report_zones(disk, 0, UINT_MAX,
>  				       blk_revalidate_zone_cb, &args);
> +	if (!ret) {
> +		pr_warn("%s: No zones reported\n", disk->disk_name);
> +		ret = -ENODEV;
> +	}
>  	memalloc_noio_restore(noio_flag);
>  
> +	/*
> +	 * If zones where reported, make sure that the entire disk capacity
> +	 * has been checked.
> +	 */
> +	if (ret > 0 && args.sector != get_capacity(disk)) {
> +		pr_warn("%s: Missing zones from sector %llu\n",
> +			disk->disk_name, args.sector);
> +		ret = -ENODEV;
> +	}
> +
>  	/*
>  	 * Install the new bitmaps and update nr_zones only once the queue is
>  	 * stopped and all I/Os are completed (i.e. a scheduler is not
>  	 * referencing the bitmaps).
>  	 */
>  	blk_mq_freeze_queue(q);
> -	if (ret >= 0) {
> +	if (ret > 0) {
>  		blk_queue_chunk_sectors(q, args.zone_sectors);
>  		q->nr_zones = args.nr_zones;
>  		swap(q->seq_zones_wlock, args.seq_zones_wlock);
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] block: Improve blk_revalidate_disk_zones() checks

[Jens Axboe]

On 12/7/20 5:29 PM, Damien Le Moal wrote:
> On 2020/11/11 16:36, Damien Le Moal wrote:
>> Improves the checks on the zones of a zoned block device done in
>> blk_revalidate_disk_zones() by making sure that the device report_zones
>> method did report at least one zone and that the zones reported exactly
>> cover the entire disk capacity, that is, that there are no missing zones
>> at the end of the disk sector range.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> 
> Jens,
> 
> Ping ?

Picked up, thanks.

-- 
Jens Axboe