Re: [dm-devel] [PATCH v13 1/9] block: Introduce queue limits for copy-offload support
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > index b7b56871029c..dff56813f95a 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -64,6 +64,9 @@ struct fstrim_range { > __u64 minlen; > }; > > +/* maximum copy offload length, this is set to 128MB based on current > testing */ > +#define COPY_MAX_BYTES (1 << 27) This should not be in the UAPI. It is a totally arbitrary internal limit. -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v13 1/9] block: Introduce queue limits for copy-offload support
Just a little heads up: I think we need to properly solve the hw vs user limit as the various ad-hoc mechanisms tend to be broken and don't scale. I plan to start looking into that the next days. On Wed, Jun 28, 2023 at 12:06:15AM +0530, 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. Why do we need the separate copy_offload boolean vs just looking at the max_bytes value? -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v13 1/9] block: Introduce queue limits for copy-offload support
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 > Signed-off-by: Nitesh Shetty > Signed-off-by: Kanchan Joshi > Signed-off-by: Anuj Gupta > --- > 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//queue/copy_offload > +Date:June 2023 > +Contact: linux-bl...@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). > + > + > +What:/sys/block//queue/copy_max_bytes > +Date:June 2023 > +Contact: linux-bl...@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 > + 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//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. > +Date:June 2023 > +Contact: linux-bl...@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//queue/crypto/ > Date:February 2022 > Contact: linux-bl...@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
Re: [dm-devel] [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 Signed-off-by: Nitesh Shetty Signed-off-by: Kanchan Joshi Signed-off-by: Anuj Gupta --- 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//queue/copy_offload +Date: June 2023 +Contact: linux-bl...@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//queue/copy_max_bytes +Date: June 2023 +Contact: linux-bl...@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//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-bl...@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//queue/crypto/ Date: February 2022 Contact: linux-bl...@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,
[dm-devel] [PATCH v13 1/9] block: Introduce queue limits for copy-offload support
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 Signed-off-by: Nitesh Shetty Signed-off-by: Kanchan Joshi Signed-off-by: Anuj Gupta --- 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//queue/copy_offload +Date: June 2023 +Contact: linux-bl...@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. + + +What: /sys/block//queue/copy_max_bytes +Date: June 2023 +Contact: linux-bl...@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 + 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//queue/copy_max_bytes_hw +Date: June 2023 +Contact: linux-bl...@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//queue/crypto/ Date: February 2022 Contact: linux-bl...@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