diff mbox series

[v3,1/4] block: Add zone flags to queue zone prop.

Message ID 20200702092438.63717-2-javier@javigon.com (mailing list archive)
State New, archived
Headers show
Series ZNS: Extra features for current patches | expand

Commit Message

Javier González July 2, 2020, 9:24 a.m. UTC 
From: Javier González <javier.gonz@samsung.com>

As the zoned block device will have to deal with features that are
optional for the backend device, add a flag field to inform the block
layer about supported features. This builds on top of
blk_zone_report_flags and extendes to the zone report code.

Signed-off-by: Javier González <javier.gonz@samsung.com>
Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
---
 block/blk-zoned.c              | 3 ++-
 drivers/block/null_blk_zoned.c | 2 ++
 drivers/nvme/host/zns.c        | 1 +
 drivers/scsi/sd.c              | 2 ++
 include/linux/blkdev.h         | 3 +++
 5 files changed, 10 insertions(+), 1 deletion(-)

Comments

Damien Le Moal July 2, 2020, 9:46 a.m. UTC | #1 
On 2020/07/02 18:25, Javier González wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> As the zoned block device will have to deal with features that are
> optional for the backend device, add a flag field to inform the block
> layer about supported features. This builds on top of
> blk_zone_report_flags and extendes to the zone report code.
> 
> Signed-off-by: Javier González <javier.gonz@samsung.com>
> Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> ---
>  block/blk-zoned.c              | 3 ++-
>  drivers/block/null_blk_zoned.c | 2 ++
>  drivers/nvme/host/zns.c        | 1 +
>  drivers/scsi/sd.c              | 2 ++
>  include/linux/blkdev.h         | 3 +++
>  5 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 81152a260354..0f156e96e48f 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -312,7 +312,8 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
>  		return ret;
>  
>  	rep.nr_zones = ret;
> -	rep.flags = BLK_ZONE_REP_CAPACITY;
> +	rep.flags = q->zone_flags;

*all* zoned devices support capacity. So why changing this ?
You are only forcing adding setting the queue flags for all device drivers. That
is more code.

> +

White line change

>  	if (copy_to_user(argp, &rep, sizeof(struct blk_zone_report)))
>  		return -EFAULT;
>  	return 0;
> diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
> index b05832eb21b2..957c2103f240 100644
> --- a/drivers/block/null_blk_zoned.c
> +++ b/drivers/block/null_blk_zoned.c
> @@ -78,6 +78,8 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
>  	}
>  
>  	q->limits.zoned = BLK_ZONED_HM;
> +	q->zone_flags = BLK_ZONE_REP_CAPACITY;
> +

White line change

>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
>  	blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
>  
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> index c08f6281b614..afe62dc27ff7 100644
> --- a/drivers/nvme/host/zns.c
> +++ b/drivers/nvme/host/zns.c
> @@ -81,6 +81,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns,
>  	}
>  
>  	q->limits.zoned = BLK_ZONED_HM;
> +	q->zone_flags = BLK_ZONE_REP_CAPACITY;
>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
>  free_data:
>  	kfree(id);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d90fefffe31b..b9c920bace28 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2967,6 +2967,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
>  	if (sdkp->device->type == TYPE_ZBC) {
>  		/* Host-managed */
>  		q->limits.zoned = BLK_ZONED_HM;
> +		q->zone_flags = BLK_ZONE_REP_CAPACITY;
>  	} else {
>  		sdkp->zoned = (buffer[8] >> 4) & 3;
>  		if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
> @@ -2983,6 +2984,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
>  					  "Drive-managed SMR disk\n");
>  		}
>  	}
> +

White line change

>  	if (blk_queue_is_zoned(q) && sdkp->first_scan)
>  		sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n",
>  		      q->limits.zoned == BLK_ZONED_HM ? "managed" : "aware");
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 8fd900998b4e..3f2e3425fa53 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -512,12 +512,15 @@ struct request_queue {
>  	 * Stacking drivers (device mappers) may or may not initialize
>  	 * these fields.
>  	 *
> +	 * Flags represent features as described by blk_zone_report_flags in blkzoned.h
> +	 *
>  	 * Reads of this information must be protected with blk_queue_enter() /
>  	 * blk_queue_exit(). Modifying this information is only allowed while
>  	 * no requests are being processed. See also blk_mq_freeze_queue() and
>  	 * blk_mq_unfreeze_queue().
>  	 */
>  	unsigned int		nr_zones;
> +	unsigned int		zone_flags;
>  	unsigned long		*conv_zones_bitmap;
>  	unsigned long		*seq_zones_wlock;
>  #endif /* CONFIG_BLK_DEV_ZONED */
> 

And you are still breaking device mapper targets report zones with this patch.
Javier González July 2, 2020, 10:18 a.m. UTC | #2 
On 02.07.2020 09:46, Damien Le Moal wrote:
>On 2020/07/02 18:25, Javier González wrote:
>> From: Javier González <javier.gonz@samsung.com>
>>
>> As the zoned block device will have to deal with features that are
>> optional for the backend device, add a flag field to inform the block
>> layer about supported features. This builds on top of
>> blk_zone_report_flags and extendes to the zone report code.
>>
>> Signed-off-by: Javier González <javier.gonz@samsung.com>
>> Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
>> ---
>>  block/blk-zoned.c              | 3 ++-
>>  drivers/block/null_blk_zoned.c | 2 ++
>>  drivers/nvme/host/zns.c        | 1 +
>>  drivers/scsi/sd.c              | 2 ++
>>  include/linux/blkdev.h         | 3 +++
>>  5 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>> index 81152a260354..0f156e96e48f 100644
>> --- a/block/blk-zoned.c
>> +++ b/block/blk-zoned.c
>> @@ -312,7 +312,8 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
>>  		return ret;
>>
>>  	rep.nr_zones = ret;
>> -	rep.flags = BLK_ZONE_REP_CAPACITY;
>> +	rep.flags = q->zone_flags;
>
>*all* zoned devices support capacity. So why changing this ?
>You are only forcing adding setting the queue flags for all device drivers. That
>is more code.
>
>> +
>
>White line change
>
>>  	if (copy_to_user(argp, &rep, sizeof(struct blk_zone_report)))
>>  		return -EFAULT;
>>  	return 0;
>> diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
>> index b05832eb21b2..957c2103f240 100644
>> --- a/drivers/block/null_blk_zoned.c
>> +++ b/drivers/block/null_blk_zoned.c
>> @@ -78,6 +78,8 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
>>  	}
>>
>>  	q->limits.zoned = BLK_ZONED_HM;
>> +	q->zone_flags = BLK_ZONE_REP_CAPACITY;
>> +
>
>White line change
>
>>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
>>  	blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
>>
>> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
>> index c08f6281b614..afe62dc27ff7 100644
>> --- a/drivers/nvme/host/zns.c
>> +++ b/drivers/nvme/host/zns.c
>> @@ -81,6 +81,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns,
>>  	}
>>
>>  	q->limits.zoned = BLK_ZONED_HM;
>> +	q->zone_flags = BLK_ZONE_REP_CAPACITY;
>>  	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
>>  free_data:
>>  	kfree(id);
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index d90fefffe31b..b9c920bace28 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -2967,6 +2967,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
>>  	if (sdkp->device->type == TYPE_ZBC) {
>>  		/* Host-managed */
>>  		q->limits.zoned = BLK_ZONED_HM;
>> +		q->zone_flags = BLK_ZONE_REP_CAPACITY;
>>  	} else {
>>  		sdkp->zoned = (buffer[8] >> 4) & 3;
>>  		if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
>> @@ -2983,6 +2984,7 @@ static void sd_read_block_characteristics(struct scsi_disk *sdkp)
>>  					  "Drive-managed SMR disk\n");
>>  		}
>>  	}
>> +
>
>White line change
>
>>  	if (blk_queue_is_zoned(q) && sdkp->first_scan)
>>  		sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n",
>>  		      q->limits.zoned == BLK_ZONED_HM ? "managed" : "aware");
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index 8fd900998b4e..3f2e3425fa53 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -512,12 +512,15 @@ struct request_queue {
>>  	 * Stacking drivers (device mappers) may or may not initialize
>>  	 * these fields.
>>  	 *
>> +	 * Flags represent features as described by blk_zone_report_flags in blkzoned.h
>> +	 *
>>  	 * Reads of this information must be protected with blk_queue_enter() /
>>  	 * blk_queue_exit(). Modifying this information is only allowed while
>>  	 * no requests are being processed. See also blk_mq_freeze_queue() and
>>  	 * blk_mq_unfreeze_queue().
>>  	 */
>>  	unsigned int		nr_zones;
>> +	unsigned int		zone_flags;
>>  	unsigned long		*conv_zones_bitmap;
>>  	unsigned long		*seq_zones_wlock;
>>  #endif /* CONFIG_BLK_DEV_ZONED */
>>
>
>And you are still breaking device mapper targets report zones with this patch.

Yes. I mentioned in the version changes that I only fixed a bad commit
with debug code. I'll address this in a new version when the use of the
feature bit is clarified (i.e., keep it this way or move to sysfs).

Javier
diff mbox series

Patch

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 81152a260354..0f156e96e48f 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -312,7 +312,8 @@  int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
 		return ret;
 
 	rep.nr_zones = ret;
-	rep.flags = BLK_ZONE_REP_CAPACITY;
+	rep.flags = q->zone_flags;
+
 	if (copy_to_user(argp, &rep, sizeof(struct blk_zone_report)))
 		return -EFAULT;
 	return 0;
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index b05832eb21b2..957c2103f240 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -78,6 +78,8 @@  int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
 	}
 
 	q->limits.zoned = BLK_ZONED_HM;
+	q->zone_flags = BLK_ZONE_REP_CAPACITY;
+
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
 	blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
 
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index c08f6281b614..afe62dc27ff7 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -81,6 +81,7 @@  int nvme_update_zone_info(struct gendisk *disk, struct nvme_ns *ns,
 	}
 
 	q->limits.zoned = BLK_ZONED_HM;
+	q->zone_flags = BLK_ZONE_REP_CAPACITY;
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
 free_data:
 	kfree(id);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d90fefffe31b..b9c920bace28 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2967,6 +2967,7 @@  static void sd_read_block_characteristics(struct scsi_disk *sdkp)
 	if (sdkp->device->type == TYPE_ZBC) {
 		/* Host-managed */
 		q->limits.zoned = BLK_ZONED_HM;
+		q->zone_flags = BLK_ZONE_REP_CAPACITY;
 	} else {
 		sdkp->zoned = (buffer[8] >> 4) & 3;
 		if (sdkp->zoned == 1 && !disk_has_partitions(sdkp->disk)) {
@@ -2983,6 +2984,7 @@  static void sd_read_block_characteristics(struct scsi_disk *sdkp)
 					  "Drive-managed SMR disk\n");
 		}
 	}
+
 	if (blk_queue_is_zoned(q) && sdkp->first_scan)
 		sd_printk(KERN_NOTICE, sdkp, "Host-%s zoned block device\n",
 		      q->limits.zoned == BLK_ZONED_HM ? "managed" : "aware");
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8fd900998b4e..3f2e3425fa53 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -512,12 +512,15 @@  struct request_queue {
 	 * Stacking drivers (device mappers) may or may not initialize
 	 * these fields.
 	 *
+	 * Flags represent features as described by blk_zone_report_flags in blkzoned.h
+	 *
 	 * Reads of this information must be protected with blk_queue_enter() /
 	 * blk_queue_exit(). Modifying this information is only allowed while
 	 * no requests are being processed. See also blk_mq_freeze_queue() and
 	 * blk_mq_unfreeze_queue().
 	 */
 	unsigned int		nr_zones;
+	unsigned int		zone_flags;
 	unsigned long		*conv_zones_bitmap;
 	unsigned long		*seq_zones_wlock;
 #endif /* CONFIG_BLK_DEV_ZONED */