On Fri, Jul 14, 2023 at 09:25:10AM +0200, Andreas Hindborg wrote: > From: Andreas Hindborg <a.hindborg@xxxxxxxxxxx> > > Add zoned storage support to ublk: report_zones and operations: > - REQ_OP_ZONE_OPEN > - REQ_OP_ZONE_CLOSE > - REQ_OP_ZONE_FINISH > - REQ_OP_ZONE_RESET > - REQ_OP_ZONE_APPEND > > The zone append feature uses the `addr` field of `struct ublksrv_io_cmd` to > communicate ALBA back to the kernel. Therefore ublk must be used with the > user copy feature (UBLK_F_USER_COPY) for zoned storage support to be > available. Without this feature, ublk will not allow zoned storage support. > > Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxxx> > --- ... > +/* > + * Construct a zone report. The report request is carried in `struct > + * ublksrv_io_desc`. The `start_sector` field must be the first sector of a zone > + * and shall indicate the first zone of the report. The `nr_sectors` shall > + * indicate how many zones should be reported (divide by zone size to get number > + * of zones in the report) and must be an integer multiple of the zone size. The > + * report shall be delivered as a `struct blk_zone` array. To report fewer zones > + * than requested, zero the last entry of the returned array. > + */ > +#define UBLK_IO_OP_REPORT_ZONES 18 Actually, I meant the following delta change in V8 comment, then the UAPI looks more clean & readable wrt. reporting how many zones in UBLK_IO_OP_REPORT_ZONES and reusing ublksrv_io_cmd->addr. Otherwise, this patchset looks fine. diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index 5698f4575e05..454c852ed328 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -70,7 +70,7 @@ struct ublk_rq_data { struct kref ref; __u64 sector; __u32 operation; - __u32 nr_sectors; + __u32 nr_zones; }; struct ublk_uring_cmd_pdu { @@ -335,7 +335,7 @@ static int ublk_report_zones(struct gendisk *disk, sector_t sector, pdu = blk_mq_rq_to_pdu(req); pdu->operation = UBLK_IO_OP_REPORT_ZONES; pdu->sector = sector; - pdu->nr_sectors = zones_in_request * zone_size_sectors; + pdu->nr_zones = zones_in_request; ret = blk_rq_map_kern(disk->queue, req, buffer, buffer_length, GFP_KERNEL); @@ -404,7 +404,7 @@ static blk_status_t ublk_setup_iod_zoned(struct ublk_queue *ubq, switch (ublk_op) { case UBLK_IO_OP_REPORT_ZONES: iod->op_flags = ublk_op | ublk_req_build_flags(req); - iod->nr_sectors = pdu->nr_sectors; + iod->nr_zones = pdu->nr_zones; iod->start_sector = pdu->sector; return BLK_STS_OK; default: diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h index 4d97eb0f7d13..602a788a650e 100644 --- a/include/uapi/linux/ublk_cmd.h +++ b/include/uapi/linux/ublk_cmd.h @@ -249,11 +249,13 @@ struct ublksrv_ctrl_dev_info { /* * Construct a zone report. The report request is carried in `struct * ublksrv_io_desc`. The `start_sector` field must be the first sector of a zone - * and shall indicate the first zone of the report. The `nr_sectors` shall - * indicate how many zones should be reported (divide by zone size to get number - * of zones in the report) and must be an integer multiple of the zone size. The - * report shall be delivered as a `struct blk_zone` array. To report fewer zones - * than requested, zero the last entry of the returned array. + * and shall indicate the first zone of the report. The `nr_zones` shall + * indicate how many zones should be reported at most. The report shall be + * delivered as a `struct blk_zone` array. To report fewer zones than + * requested, zero the last entry of the returned array. + * + * So related definitions(blk_zone, blk_zone_cond, blk_zone_type, ...) in + * include/uapi/linux/blkzoned.h are part of ublk UAPI. */ #define UBLK_IO_OP_REPORT_ZONES 18 @@ -276,7 +278,10 @@ struct ublksrv_io_desc { /* op: bit 0-7, flags: bit 8-31 */ __u32 op_flags; - __u32 nr_sectors; + union { + __u32 nr_sectors; + __u32 nr_zones; /* for UBLK_IO_OP_REPORT_ZONES only */ + }; /* start sector for this io */ __u64 start_sector; @@ -308,6 +313,12 @@ struct ublksrv_io_cmd { /* * userspace buffer address in ublksrv daemon process, valid for * FETCH* command only + * + * This field shouldn't be used if UBLK_F_USER_COPY is enabled, + * because userspace deals with data copy by pread()/pwrite() over + * /dev/ublkcN. But in case of UBLK_F_ZONED, 'addr' is re-used to + * pass back the allocated LBA for UBLK_IO_OP_ZONE_APPEND which + * actually depends on UBLK_F_USER_COPY */ __u64 addr; }; Thanks, Ming