On 2020/07/02 15:55, Javier González wrote:
> From: Javier González <javier.gonz@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: SelvaKumar S <selvakuma.s1@xxxxxxxxxxx>
> Signed-off-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx>
> Signed-off-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx>
> ---
> 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;
zone_flags seem to be a fairly generic flags field while rep.flags is only about
the reported descriptors structure. So you may want to define a
BLK_ZONE_REP_FLAGS_MASK and have:
rep.flags = q->zone_flags & BLK_ZONE_REP_FLAGS_MASK;
to avoid flags meaningless for the user being set.
In any case, since *all* zoned block devices now report capacity, I do not
really see the point to add BLK_ZONE_REP_FLAGS_MASK to q->zone_flags, especially
considering that you set the flag for all zoned device types, including scsi
which does not have zone capacity. This makes q->zone_flags rather confusing
instead of clearly defining the device features as you mentioned in the commit
message.
I think it may be better to just drop this patch, and if needed, introduce the
zone_flags field where it may be needed (e.g. OFFLINE zone ioctl support).
> +
> 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 0642d3c54e8f..888264261ba3 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 */
>
And you are missing device-mapper support. DM target devices have a request
queue that would need to set the zone_flags too.
--
Damien Le Moal
Western Digital Research
