[2/4] blk-zoned: implement REQ_OP_ZONE_RESET_ALL
diff mbox series

Message ID 20190731210102.3472-3-chaitanya.kulkarni@wdc.com
State New
Headers show
Series
  • block: introduce REQ_OP_ZONE_RESET_ALL
Related show

Commit Message

Chaitanya Kulkarni July 31, 2019, 9:01 p.m. UTC
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 | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

Comments

Damien Le Moal Aug. 1, 2019, 12:55 a.m. UTC | #1
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@wdc.com>
> ---
>  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.

> +	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))
>
Chaitanya Kulkarni Aug. 1, 2019, 4:51 a.m. UTC | #2
From: Damien Le Moal <Damien.LeMoal@wdc.com>

Sent: Wednesday, July 31, 2019 5:55 PM

To: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>; linux-block@vger.kernel.org <linux-block@vger.kernel.org>; linux-scsi@vger.kernel.org <linux-scsi@vger.kernel.org>

Cc: axboe@kernel.dk <axboe@kernel.dk>; jejb@linux.ibm.com <jejb@linux.ibm.com>; dennis@kernel.org <dennis@kernel.org>; hare@suse.com <hare@suse.com>; sagi@grimberg.me <sagi@grimberg.me>; dennisszhou@gmail.com <dennisszhou@gmail.com>; jthumshirn@suse.de
 <jthumshirn@suse.de>; osandov@fb.com <osandov@fb.com>; ming.lei@redhat.com <ming.lei@redhat.com>; tj@kernel.org <tj@kernel.org>; bvanassche@acm.org <bvanassche@acm.org>; martin.petersen@oracle.com <martin.petersen@oracle.com>

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@wdc.com>

> ---

>  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 Aug. 1, 2019, 8:40 a.m. UTC | #3
On 2019/08/01 13:51, Chaitanya Kulkarni wrote:>>> +/*
>>> + * 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 :-

Whichever is fine with me.

> /*
>  * 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 */

May be change this comment to something less cryptic, e.g.:

/* For REQ_OP_ZONE_RESET_ALL, BIO sector and size are not needed  */

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

Patch
diff mbox series

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