From: Damien Le Moal <Damien.LeMoal@xxxxxxx>
Sent: Wednesday, July 31, 2019 5:55 PM
To: Chaitanya Kulkarni <Chaitanya.Kulkarni@xxxxxxx>; linux-block@xxxxxxxxxxxxxxx <linux-block@xxxxxxxxxxxxxxx>; linux-scsi@xxxxxxxxxxxxxxx <linux-scsi@xxxxxxxxxxxxxxx>
Cc: axboe@xxxxxxxxx <axboe@xxxxxxxxx>; jejb@xxxxxxxxxxxxx <jejb@xxxxxxxxxxxxx>; dennis@xxxxxxxxxx <dennis@xxxxxxxxxx>; hare@xxxxxxxx <hare@xxxxxxxx>; sagi@xxxxxxxxxxx <sagi@xxxxxxxxxxx>; dennisszhou@xxxxxxxxx <dennisszhou@xxxxxxxxx>; jthumshirn@xxxxxxx
<jthumshirn@xxxxxxx>; osandov@xxxxxx <osandov@xxxxxx>; ming.lei@xxxxxxxxxx <ming.lei@xxxxxxxxxx>; tj@xxxxxxxxxx <tj@xxxxxxxxxx>; bvanassche@xxxxxxx <bvanassche@xxxxxxx>; martin.petersen@xxxxxxxxxx <martin.petersen@xxxxxxxxxx>
Subject: Re: [PATCH 2/4] blk-zoned: implement REQ_OP_ZONE_RESET_ALL
On 2019/08/01 6:01, 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@xxxxxxx>
> ---
> block/blk-core.c | 5 +++++
> block/blk-zoned.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 45 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),
> @@ -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..d1ed728b7464 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -202,6 +202,43 @@ 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 = NULL;
There is no need to initialize the bio to NULL here.
[CK] Would you prefer something like following that declares and allocate
in one line which is similar in blk-lib.c:-blk_next_bio() function ? or we should keep
declaration and allocation on the different line :-
/*
* 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;
}
> + int ret;
> +
> + /* across the zones operations, don't need any sectors */
> + bio = bio_alloc(gfp_mask, 0);
> + 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 +272,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))
>
--
Damien Le Moal
Western Digital Research
