Re: [PATCH v13 1/9] block: Introduce queue limits for copy-offload support

From: Nitesh Shetty
Date: Thu Jun 29 2023 - 03:48:13 EST


On 23/06/28 03:40PM, Damien Le Moal wrote:
On 6/28/23 03:36, Nitesh Shetty wrote:
Add device limits as sysfs entries,
- copy_offload (RW)
- copy_max_bytes (RW)
- copy_max_bytes_hw (RO)

Above limits help to split the copy payload in block layer.
copy_offload: used for setting copy offload(1) or emulation(0).
copy_max_bytes: maximum total length of copy in single payload.
copy_max_bytes_hw: Reflects the device supported maximum limit.

Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
Signed-off-by: Nitesh Shetty <nj.shetty@xxxxxxxxxxx>
Signed-off-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx>
Signed-off-by: Anuj Gupta <anuj20.g@xxxxxxxxxxx>
---
Documentation/ABI/stable/sysfs-block | 33 +++++++++++++++
block/blk-settings.c | 24 +++++++++++
block/blk-sysfs.c | 63 ++++++++++++++++++++++++++++
include/linux/blkdev.h | 12 ++++++
include/uapi/linux/fs.h | 3 ++
5 files changed, 135 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
index c57e5b7cb532..3c97303f658b 100644
--- a/Documentation/ABI/stable/sysfs-block
+++ b/Documentation/ABI/stable/sysfs-block
@@ -155,6 +155,39 @@ Description:
last zone of the device which may be smaller.


+What: /sys/block/<disk>/queue/copy_offload
+Date: June 2023
+Contact: linux-block@xxxxxxxxxxxxxxx
+Description:
+ [RW] When read, this file shows whether offloading copy to a
+ device is enabled (1) or disabled (0). Writing '0' to this
+ file will disable offloading copies for this device.
+ Writing any '1' value will enable this feature. If the device
+ does not support offloading, then writing 1, will result in an
+ error.

I am still not convinced that this one is really necessary. copy_max_bytes_hw !=
0 indicates that the devices supports copy offload. And setting copy_max_bytes
to 0 can be used to disable copy offload (which probably should be the default
for now).


Agreed, we will do this in next iteration.

+
+
+What: /sys/block/<disk>/queue/copy_max_bytes
+Date: June 2023
+Contact: linux-block@xxxxxxxxxxxxxxx
+Description:
+ [RW] This is the maximum number of bytes that the block layer
+ will allow for a copy request. This will is always smaller or

will is -> is


acked

+ equal to the maximum size allowed by the hardware, indicated by
+ 'copy_max_bytes_hw'. An attempt to set a value higher than
+ 'copy_max_bytes_hw' will truncate this to 'copy_max_bytes_hw'.
+
+
+What: /sys/block/<disk>/queue/copy_max_bytes_hw

Nit: In keeping with the spirit of attributes like
max_hw_sectors_kb/max_sectors_kb, I would call this one copy_max_hw_bytes.


acked, will update in next iteration.

+Date: June 2023
+Contact: linux-block@xxxxxxxxxxxxxxx
+Description:
+ [RO] This is the maximum number of bytes that the hardware
+ will allow for single data copy request.
+ A value of 0 means that the device does not support
+ copy offload.
+
+
What: /sys/block/<disk>/queue/crypto/
Date: February 2022
Contact: linux-block@xxxxxxxxxxxxxxx
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 4dd59059b788..738cd3f21259 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -59,6 +59,8 @@ void blk_set_default_limits(struct queue_limits *lim)
lim->zoned = BLK_ZONED_NONE;
lim->zone_write_granularity = 0;
lim->dma_alignment = 511;
+ lim->max_copy_sectors_hw = 0;
+ lim->max_copy_sectors = 0;
}

/**
@@ -82,6 +84,8 @@ void blk_set_stacking_limits(struct queue_limits *lim)
lim->max_dev_sectors = UINT_MAX;
lim->max_write_zeroes_sectors = UINT_MAX;
lim->max_zone_append_sectors = UINT_MAX;
+ lim->max_copy_sectors_hw = UINT_MAX;
+ lim->max_copy_sectors = UINT_MAX;
}
EXPORT_SYMBOL(blk_set_stacking_limits);

@@ -183,6 +187,22 @@ void blk_queue_max_discard_sectors(struct request_queue *q,
}
EXPORT_SYMBOL(blk_queue_max_discard_sectors);

+/**
+ * blk_queue_max_copy_sectors_hw - set max sectors for a single copy payload
+ * @q: the request queue for the device
+ * @max_copy_sectors: maximum number of sectors to copy
+ **/
+void blk_queue_max_copy_sectors_hw(struct request_queue *q,
+ unsigned int max_copy_sectors)
+{
+ if (max_copy_sectors > (COPY_MAX_BYTES >> SECTOR_SHIFT))
+ max_copy_sectors = COPY_MAX_BYTES >> SECTOR_SHIFT;
+
+ q->limits.max_copy_sectors_hw = max_copy_sectors;
+ q->limits.max_copy_sectors = max_copy_sectors;
+}
+EXPORT_SYMBOL_GPL(blk_queue_max_copy_sectors_hw);
+
/**
* blk_queue_max_secure_erase_sectors - set max sectors for a secure erase
* @q: the request queue for the device
@@ -578,6 +598,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
t->max_segment_size = min_not_zero(t->max_segment_size,
b->max_segment_size);

+ t->max_copy_sectors = min(t->max_copy_sectors, b->max_copy_sectors);
+ t->max_copy_sectors_hw = min(t->max_copy_sectors_hw,
+ b->max_copy_sectors_hw);
+
t->misaligned |= b->misaligned;

alignment = queue_limit_alignment_offset(b, start);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index afc797fb0dfc..43551778d035 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -199,6 +199,62 @@ static ssize_t queue_discard_zeroes_data_show(struct request_queue *q, char *pag
return queue_var_show(0, page);
}

+static ssize_t queue_copy_offload_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(blk_queue_copy(q), page);
+}
+
+static ssize_t queue_copy_offload_store(struct request_queue *q,
+ const char *page, size_t count)
+{
+ unsigned long copy_offload;
+ ssize_t ret = queue_var_store(&copy_offload, page, count);
+
+ if (ret < 0)
+ return ret;
+
+ if (copy_offload && !q->limits.max_copy_sectors_hw)
+ return -EINVAL;
+
+ if (copy_offload)
+ blk_queue_flag_set(QUEUE_FLAG_COPY, q);
+ else
+ blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
+
+ return count;
+}

See above. I think we can drop this attribute.

acked

Thank you, Nitesh Shetty