Re: [PATCH V2 2/4] blk-zoned: implement REQ_OP_ZONE_RESET_ALL

[Hannes Reinecke <hare@suse.de>]

On 8/1/19 7:26 PM, Chaitanya Kulkarni wrote:
> This implements REQ_OP_ZONE_RESET_ALL as a special case of the block
> device zone reset operations where we just simply issue bio with the
> newly introduced req op.
> 
> We issue this req op when the number of sectors is equal to the device's
> partition's number of sectors and device has no partitions.
> 
> We also add support so that blk_op_str() can print the new reset-all
> zone operation.
> 
> This patch also adds a generic make request check for newly
> introduced REQ_OP_ZONE_RESET_ALL req_opf. We simply return error
> when queue is zoned and reset-all flag is not set for
> REQ_OP_ZONE_RESET_ALL.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  block/blk-core.c  |  5 +++++
>  block/blk-zoned.c | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index d0cc6e14d2f0..1b53ab56228b 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -129,6 +129,7 @@ static const char *const blk_op_name[] = {
>  	REQ_OP_NAME(DISCARD),
>  	REQ_OP_NAME(SECURE_ERASE),
>  	REQ_OP_NAME(ZONE_RESET),
> +	REQ_OP_NAME(ZONE_RESET_ALL),
>  	REQ_OP_NAME(WRITE_SAME),
>  	REQ_OP_NAME(WRITE_ZEROES),
>  	REQ_OP_NAME(SCSI_IN),

I wonder if this shouldn't be moved to the previous patch ...

> @@ -931,6 +932,10 @@ generic_make_request_checks(struct bio *bio)
>  		if (!blk_queue_is_zoned(q))
>  			goto not_supported;
>  		break;
> +	case REQ_OP_ZONE_RESET_ALL:
> +		if (!blk_queue_is_zoned(q) || !blk_queue_zone_resetall(q))
> +			goto not_supported;
> +		break;
>  	case REQ_OP_WRITE_ZEROES:
>  		if (!q->limits.max_write_zeroes_sectors)
>  			goto not_supported;
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 6c503824ba3f..4bc5f260248a 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -202,6 +202,42 @@ int blkdev_report_zones(struct block_device *bdev, sector_t sector,
>  }
>  EXPORT_SYMBOL_GPL(blkdev_report_zones);
>  
> +/*
> + * Special case of zone reset operation to reset all zones in one command,
> + * useful for applications like mkfs.
> + */
> +static int __blkdev_reset_all_zones(struct block_device *bdev, gfp_t gfp_mask)
> +{
> +	struct bio *bio = bio_alloc(gfp_mask, 0);
> +	int ret;
> +
> +	/* across the zones operations, don't need any sectors */
> +	bio_set_dev(bio, bdev);
> +	bio_set_op_attrs(bio, REQ_OP_ZONE_RESET_ALL, 0);
> +
> +	ret = submit_bio_wait(bio);
> +	bio_put(bio);
> +
> +	return ret;
> +}
> +
> +static inline bool blkdev_allow_reset_all_zones(struct block_device *bdev,
> +						sector_t nr_sectors)
> +{
> +	if (!blk_queue_zone_resetall(bdev_get_queue(bdev)))
> +		return false;
> +
> +	if (nr_sectors != part_nr_sects_read(bdev->bd_part))
> +		return false;
> +	/*
> +	 * REQ_OP_ZONE_RESET_ALL can be executed only if the block device is
> +	 * the entire disk, that is, if the blocks device start offset is 0 and
> +	 * its capacity is the same as the entire disk.
> +	 */
> +	return get_start_sect(bdev) == 0 &&
> +	       part_nr_sects_read(bdev->bd_part) == get_capacity(bdev->bd_disk);
> +}
> +
>  /**
>   * blkdev_reset_zones - Reset zones write pointer
>   * @bdev:	Target block device
> @@ -235,6 +271,9 @@ int blkdev_reset_zones(struct block_device *bdev,
>  		/* Out of range */
>  		return -EINVAL;
>  
> +	if (blkdev_allow_reset_all_zones(bdev, nr_sectors))
> +		return  __blkdev_reset_all_zones(bdev, gfp_mask);
> +
>  	/* Check alignment (handle eventual smaller last zone) */
>  	zone_sectors = blk_queue_zone_sectors(q);
>  	if (sector & (zone_sectors - 1))
> 
But anyway:

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)