Messages in this thread | | | Date | Wed, 28 Jun 2023 21:05:18 +0530 | From | Nitesh Shetty <> | Subject | Re: [PATCH v13 1/9] block: Introduce queue limits for copy-offload support |
| |
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@suse.de> >> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> >> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> >> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com> >> --- >> 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@vger.kernel.org >> +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@vger.kernel.org >> +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@vger.kernel.org >> +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@vger.kernel.org >> 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(©_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
| |