* [PATCH v8 1/9] block: Introduce queue limits for copy-offload support
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 8:40 ` Damien Le Moal
2023-03-27 8:40 ` [PATCH v8 2/9] block: Add copy offload support infrastructure Anuj Gupta
` (7 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
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 | 36 ++++++++++++++++
block/blk-settings.c | 24 +++++++++++
block/blk-sysfs.c | 64 ++++++++++++++++++++++++++++
include/linux/blkdev.h | 12 ++++++
include/uapi/linux/fs.h | 3 ++
5 files changed, 139 insertions(+)
diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
index c57e5b7cb532..f5c56ad91ad6 100644
--- a/Documentation/ABI/stable/sysfs-block
+++ b/Documentation/ABI/stable/sysfs-block
@@ -155,6 +155,42 @@ Description:
last zone of the device which may be smaller.
+What: /sys/block/<disk>/queue/copy_offload
+Date: November 2022
+Contact: linux-block@vger.kernel.org
+Description:
+ [RW] When read, this file shows whether offloading copy to
+ 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 device
+ does not support offloading, then writing 1, will result in
+ error.
+
+
+What: /sys/block/<disk>/queue/copy_max_bytes
+Date: November 2022
+Contact: linux-block@vger.kernel.org
+Description:
+ [RW] While 'copy_max_bytes_hw' is the hardware limit for the
+ device, 'copy_max_bytes' setting is the software limit.
+ Setting this value lower will make Linux issue smaller size
+ copies from block layer.
+
+
+What: /sys/block/<disk>/queue/copy_max_bytes_hw
+Date: November 2022
+Contact: linux-block@vger.kernel.org
+Description:
+ [RO] Devices that support offloading copy functionality may have
+ internal limits on the number of bytes that can be offloaded
+ in a single operation. The `copy_max_bytes_hw`
+ parameter is set by the device driver to the maximum number of
+ bytes that can be copied in a single operation. Copy
+ requests issued to the device must not exceed this limit.
+ 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 896b4654ab00..350f3584f691 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 = ULONG_MAX;
+ lim->max_copy_sectors = ULONG_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 >= MAX_COPY_TOTAL_LENGTH)
+ max_copy_sectors = MAX_COPY_TOTAL_LENGTH;
+
+ 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 1a743b4f2958..dccb162cf318 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -213,6 +213,63 @@ 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)
+{
+ s64 copy_offload;
+ ssize_t ret = queue_var_store64(©_offload, page);
+
+ 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;
+}
+
+static ssize_t queue_copy_max_hw_show(struct request_queue *q, char *page)
+{
+ return sprintf(page, "%llu\n", (unsigned long long)
+ q->limits.max_copy_sectors_hw << SECTOR_SHIFT);
+}
+
+static ssize_t queue_copy_max_show(struct request_queue *q, char *page)
+{
+ return sprintf(page, "%llu\n", (unsigned long long)
+ q->limits.max_copy_sectors << SECTOR_SHIFT);
+}
+
+static ssize_t queue_copy_max_store(struct request_queue *q,
+ const char *page, size_t count)
+{
+ s64 max_copy;
+ ssize_t ret = queue_var_store64(&max_copy, page);
+
+ if (ret < 0)
+ return ret;
+
+ if (max_copy & (queue_logical_block_size(q) - 1))
+ return -EINVAL;
+
+ max_copy >>= SECTOR_SHIFT;
+ if (max_copy > q->limits.max_copy_sectors_hw)
+ max_copy = q->limits.max_copy_sectors_hw;
+
+ q->limits.max_copy_sectors = max_copy;
+ return count;
+}
+
static ssize_t queue_write_same_max_show(struct request_queue *q, char *page)
{
return queue_var_show(0, page);
@@ -591,6 +648,10 @@ QUEUE_RO_ENTRY(queue_nr_zones, "nr_zones");
QUEUE_RO_ENTRY(queue_max_open_zones, "max_open_zones");
QUEUE_RO_ENTRY(queue_max_active_zones, "max_active_zones");
+QUEUE_RW_ENTRY(queue_copy_offload, "copy_offload");
+QUEUE_RO_ENTRY(queue_copy_max_hw, "copy_max_bytes_hw");
+QUEUE_RW_ENTRY(queue_copy_max, "copy_max_bytes");
+
QUEUE_RW_ENTRY(queue_nomerges, "nomerges");
QUEUE_RW_ENTRY(queue_rq_affinity, "rq_affinity");
QUEUE_RW_ENTRY(queue_poll, "io_poll");
@@ -638,6 +699,9 @@ static struct attribute *queue_attrs[] = {
&queue_discard_max_entry.attr,
&queue_discard_max_hw_entry.attr,
&queue_discard_zeroes_data_entry.attr,
+ &queue_copy_offload_entry.attr,
+ &queue_copy_max_hw_entry.attr,
+ &queue_copy_max_entry.attr,
&queue_write_same_max_entry.attr,
&queue_write_zeroes_max_entry.attr,
&queue_zone_append_max_entry.attr,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e3242e67a8e3..200338f2ec2e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -298,6 +298,9 @@ struct queue_limits {
unsigned int discard_alignment;
unsigned int zone_write_granularity;
+ unsigned long max_copy_sectors_hw;
+ unsigned long max_copy_sectors;
+
unsigned short max_segments;
unsigned short max_integrity_segments;
unsigned short max_discard_segments;
@@ -564,6 +567,7 @@ struct request_queue {
#define QUEUE_FLAG_NOWAIT 29 /* device supports NOWAIT */
#define QUEUE_FLAG_SQ_SCHED 30 /* single queue style io dispatch */
#define QUEUE_FLAG_SKIP_TAGSET_QUIESCE 31 /* quiesce_tagset skip the queue*/
+#define QUEUE_FLAG_COPY 32 /* supports copy offload */
#define QUEUE_FLAG_MQ_DEFAULT ((1UL << QUEUE_FLAG_IO_STAT) | \
(1UL << QUEUE_FLAG_SAME_COMP) | \
@@ -584,6 +588,7 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q);
test_bit(QUEUE_FLAG_STABLE_WRITES, &(q)->queue_flags)
#define blk_queue_io_stat(q) test_bit(QUEUE_FLAG_IO_STAT, &(q)->queue_flags)
#define blk_queue_add_random(q) test_bit(QUEUE_FLAG_ADD_RANDOM, &(q)->queue_flags)
+#define blk_queue_copy(q) test_bit(QUEUE_FLAG_COPY, &(q)->queue_flags)
#define blk_queue_zone_resetall(q) \
test_bit(QUEUE_FLAG_ZONE_RESETALL, &(q)->queue_flags)
#define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
@@ -902,6 +907,8 @@ extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int);
extern void blk_queue_max_segments(struct request_queue *, unsigned short);
extern void blk_queue_max_discard_segments(struct request_queue *,
unsigned short);
+extern void blk_queue_max_copy_sectors_hw(struct request_queue *q,
+ unsigned int max_copy_sectors);
void blk_queue_max_secure_erase_sectors(struct request_queue *q,
unsigned int max_sectors);
extern void blk_queue_max_segment_size(struct request_queue *, unsigned int);
@@ -1221,6 +1228,11 @@ static inline unsigned int bdev_discard_granularity(struct block_device *bdev)
return bdev_get_queue(bdev)->limits.discard_granularity;
}
+static inline unsigned int bdev_max_copy_sectors(struct block_device *bdev)
+{
+ return bdev_get_queue(bdev)->limits.max_copy_sectors;
+}
+
static inline unsigned int
bdev_max_secure_erase_sectors(struct block_device *bdev)
{
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index b7b56871029c..b3ad173f619c 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -64,6 +64,9 @@ struct fstrim_range {
__u64 minlen;
};
+/* maximum total copy length */
+#define MAX_COPY_TOTAL_LENGTH (1 << 27)
+
/* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
#define FILE_DEDUPE_RANGE_SAME 0
#define FILE_DEDUPE_RANGE_DIFFERS 1
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 1/9] block: Introduce queue limits for copy-offload support
2023-03-27 8:40 ` [PATCH v8 1/9] block: Introduce queue limits for copy-offload support Anuj Gupta
@ 2023-03-29 8:40 ` Damien Le Moal
2023-03-29 10:41 ` Nitesh Shetty
0 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2023-03-29 8:40 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, joshi.k, nitheshshetty, gost.dev,
Nitesh Shetty, linux-block, linux-kernel, linux-nvme,
linux-fsdevel
On 3/27/23 17:40, Anuj Gupta wrote:
> From: Nitesh Shetty <nj.shetty@samsung.com>
>
> 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 | 36 ++++++++++++++++
> block/blk-settings.c | 24 +++++++++++
> block/blk-sysfs.c | 64 ++++++++++++++++++++++++++++
> include/linux/blkdev.h | 12 ++++++
> include/uapi/linux/fs.h | 3 ++
> 5 files changed, 139 insertions(+)
>
> diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
> index c57e5b7cb532..f5c56ad91ad6 100644
> --- a/Documentation/ABI/stable/sysfs-block
> +++ b/Documentation/ABI/stable/sysfs-block
> @@ -155,6 +155,42 @@ Description:
> last zone of the device which may be smaller.
>
>
> +What: /sys/block/<disk>/queue/copy_offload
> +Date: November 2022
> +Contact: linux-block@vger.kernel.org
> +Description:
> + [RW] When read, this file shows whether offloading copy to
> + device is enabled (1) or disabled (0). Writing '0' to this
...to a device...
> + file will disable offloading copies for this device.
> + Writing any '1' value will enable this feature. If device
If the device does...
> + does not support offloading, then writing 1, will result in
> + error.
> +
> +
> +What: /sys/block/<disk>/queue/copy_max_bytes
> +Date: November 2022
> +Contact: linux-block@vger.kernel.org
> +Description:
> + [RW] While 'copy_max_bytes_hw' is the hardware limit for the
> + device, 'copy_max_bytes' setting is the software limit.
> + Setting this value lower will make Linux issue smaller size
> + copies from block layer.
This is the maximum number of bytes that the block
layer will allow for a copy request. Must be smaller than
or equal to the maximum size allowed by the hardware indicated
by copy_max_bytes_hw. Write 0 to use the default kernel
settings.
> +
> +
> +What: /sys/block/<disk>/queue/copy_max_bytes_hw
> +Date: November 2022
> +Contact: linux-block@vger.kernel.org
> +Description:
> + [RO] Devices that support offloading copy functionality may have
> + internal limits on the number of bytes that can be offloaded
> + in a single operation. The `copy_max_bytes_hw`
> + parameter is set by the device driver to the maximum number of
> + bytes that can be copied in a single operation. Copy
> + requests issued to the device must not exceed this limit.
> + A value of 0 means that the device does not
> + support copy offload.
[RO] This is the maximum number of kilobytes supported in a
single data copy offload operation. 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 896b4654ab00..350f3584f691 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 = ULONG_MAX;
> + lim->max_copy_sectors = ULONG_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 >= MAX_COPY_TOTAL_LENGTH)
Confusing name as LENGTH may be interpreted as bytes. MAX_COPY_SECTORS would be
better.
> + max_copy_sectors = MAX_COPY_TOTAL_LENGTH;
> +
> + 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);
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 1/9] block: Introduce queue limits for copy-offload support
2023-03-29 8:40 ` Damien Le Moal
@ 2023-03-29 10:41 ` Nitesh Shetty
2023-03-29 12:24 ` Damien Le Moal
0 siblings, 1 reply; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-29 10:41 UTC (permalink / raw)
To: Damien Le Moal
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner, bvanassche,
hare, ming.lei, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 5796 bytes --]
On Wed, Mar 29, 2023 at 05:40:09PM +0900, Damien Le Moal wrote:
> On 3/27/23 17:40, Anuj Gupta wrote:
> > From: Nitesh Shetty <nj.shetty@samsung.com>
> >
> > 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 | 36 ++++++++++++++++
> > block/blk-settings.c | 24 +++++++++++
> > block/blk-sysfs.c | 64 ++++++++++++++++++++++++++++
> > include/linux/blkdev.h | 12 ++++++
> > include/uapi/linux/fs.h | 3 ++
> > 5 files changed, 139 insertions(+)
> >
> > diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
> > index c57e5b7cb532..f5c56ad91ad6 100644
> > --- a/Documentation/ABI/stable/sysfs-block
> > +++ b/Documentation/ABI/stable/sysfs-block
> > @@ -155,6 +155,42 @@ Description:
> > last zone of the device which may be smaller.
> >
> >
> > +What: /sys/block/<disk>/queue/copy_offload
> > +Date: November 2022
> > +Contact: linux-block@vger.kernel.org
> > +Description:
> > + [RW] When read, this file shows whether offloading copy to
> > + device is enabled (1) or disabled (0). Writing '0' to this
>
> ...to a device...
>
acked
> > + file will disable offloading copies for this device.
> > + Writing any '1' value will enable this feature. If device
>
> If the device does...
>
acked
> > + does not support offloading, then writing 1, will result in
> > + error.
> > +
> > +
> > +What: /sys/block/<disk>/queue/copy_max_bytes
> > +Date: November 2022
> > +Contact: linux-block@vger.kernel.org
> > +Description:
> > + [RW] While 'copy_max_bytes_hw' is the hardware limit for the
> > + device, 'copy_max_bytes' setting is the software limit.
> > + Setting this value lower will make Linux issue smaller size
> > + copies from block layer.
>
> This is the maximum number of bytes that the block
> layer will allow for a copy request. Must be smaller than
> or equal to the maximum size allowed by the hardware indicated
Looks good. Will update in next version. We took reference from discard.
> by copy_max_bytes_hw. Write 0 to use the default kernel
> settings.
>
Nack, writing 0 will not set it to default value. (default value is
copy_max_bytes = copy_max_bytes_hw)
> > +
> > +
> > +What: /sys/block/<disk>/queue/copy_max_bytes_hw
> > +Date: November 2022
> > +Contact: linux-block@vger.kernel.org
> > +Description:
> > + [RO] Devices that support offloading copy functionality may have
> > + internal limits on the number of bytes that can be offloaded
> > + in a single operation. The `copy_max_bytes_hw`
> > + parameter is set by the device driver to the maximum number of
> > + bytes that can be copied in a single operation. Copy
> > + requests issued to the device must not exceed this limit.
> > + A value of 0 means that the device does not
> > + support copy offload.
>
> [RO] This is the maximum number of kilobytes supported in a
> single data copy offload operation. A value of 0 means that the
> device does not support copy offload.
>
Nack, value is in bytes. Same as discard.
> > +
> > +
> > 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 896b4654ab00..350f3584f691 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 = ULONG_MAX;
> > + lim->max_copy_sectors = ULONG_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 >= MAX_COPY_TOTAL_LENGTH)
>
> Confusing name as LENGTH may be interpreted as bytes. MAX_COPY_SECTORS would be
> better.
>
Agreed, we will make MAX_COPY_TOTAL_LENGTH explicit to bytes.
We also check this limit against user payload length which is in bytes(patch 2).
So there is a inconsistency from our end (patch 1 and 2). We will fix this
in next version.
> > + max_copy_sectors = MAX_COPY_TOTAL_LENGTH;
> > +
> > + 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);
>
>
> --
> Damien Le Moal
> Western Digital Research
>
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 1/9] block: Introduce queue limits for copy-offload support
2023-03-29 10:41 ` Nitesh Shetty
@ 2023-03-29 12:24 ` Damien Le Moal
2023-03-29 12:34 ` Nitesh Shetty
0 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2023-03-29 12:24 UTC (permalink / raw)
To: Nitesh Shetty
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner, bvanassche,
hare, ming.lei, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
On 3/29/23 19:41, Nitesh Shetty wrote:
>>> +What: /sys/block/<disk>/queue/copy_max_bytes
>>> +Date: November 2022
>>> +Contact: linux-block@vger.kernel.org
>>> +Description:
>>> + [RW] While 'copy_max_bytes_hw' is the hardware limit for the
>>> + device, 'copy_max_bytes' setting is the software limit.
>>> + Setting this value lower will make Linux issue smaller size
>>> + copies from block layer.
>>
>> This is the maximum number of bytes that the block
>> layer will allow for a copy request. Must be smaller than
>> or equal to the maximum size allowed by the hardware indicated
>
> Looks good. Will update in next version. We took reference from discard.
>
>> by copy_max_bytes_hw. Write 0 to use the default kernel
>> settings.
>>
>
> Nack, writing 0 will not set it to default value. (default value is
> copy_max_bytes = copy_max_bytes_hw)
It is trivial to make it work that way, which would match how max_sectors_kb
works. Write 0 to return copy_max_bytes being equal to the default
copy_max_bytes_hw.
The other possibility that is also interesting is "write 0 to disable copy
offload and use emulation". This one may actually be more useful.
>
>>> +
>>> +
>>> +What: /sys/block/<disk>/queue/copy_max_bytes_hw
>>> +Date: November 2022
>>> +Contact: linux-block@vger.kernel.org
>>> +Description:
>>> + [RO] Devices that support offloading copy functionality may have
>>> + internal limits on the number of bytes that can be offloaded
>>> + in a single operation. The `copy_max_bytes_hw`
>>> + parameter is set by the device driver to the maximum number of
>>> + bytes that can be copied in a single operation. Copy
>>> + requests issued to the device must not exceed this limit.
>>> + A value of 0 means that the device does not
>>> + support copy offload.
>>
>> [RO] This is the maximum number of kilobytes supported in a
>> single data copy offload operation. A value of 0 means that the
>> device does not support copy offload.
>>
>
> Nack, value is in bytes. Same as discard.
Typo. I meant Bytes. Your text is too long an too convoluted, so unclear.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v8 1/9] block: Introduce queue limits for copy-offload support
2023-03-29 12:24 ` Damien Le Moal
@ 2023-03-29 12:34 ` Nitesh Shetty
0 siblings, 0 replies; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-29 12:34 UTC (permalink / raw)
To: Damien Le Moal
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner, bvanassche,
hare, ming.lei, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]
On Wed, Mar 29, 2023 at 09:24:11PM +0900, Damien Le Moal wrote:
> On 3/29/23 19:41, Nitesh Shetty wrote:
> >>> +What: /sys/block/<disk>/queue/copy_max_bytes
> >>> +Date: November 2022
> >>> +Contact: linux-block@vger.kernel.org
> >>> +Description:
> >>> + [RW] While 'copy_max_bytes_hw' is the hardware limit for the
> >>> + device, 'copy_max_bytes' setting is the software limit.
> >>> + Setting this value lower will make Linux issue smaller size
> >>> + copies from block layer.
> >>
> >> This is the maximum number of bytes that the block
> >> layer will allow for a copy request. Must be smaller than
> >> or equal to the maximum size allowed by the hardware indicated
> >
> > Looks good. Will update in next version. We took reference from discard.
> >
> >> by copy_max_bytes_hw. Write 0 to use the default kernel
> >> settings.
> >>
> >
> > Nack, writing 0 will not set it to default value. (default value is
> > copy_max_bytes = copy_max_bytes_hw)
>
> It is trivial to make it work that way, which would match how max_sectors_kb
> works. Write 0 to return copy_max_bytes being equal to the default
> copy_max_bytes_hw.
>
> The other possibility that is also interesting is "write 0 to disable copy
> offload and use emulation". This one may actually be more useful.
>
We were following discard implementation.
I feel now both options are good. We can finalize based on community feedback.
> >
> >>> +
> >>> +
> >>> +What: /sys/block/<disk>/queue/copy_max_bytes_hw
> >>> +Date: November 2022
> >>> +Contact: linux-block@vger.kernel.org
> >>> +Description:
> >>> + [RO] Devices that support offloading copy functionality may have
> >>> + internal limits on the number of bytes that can be offloaded
> >>> + in a single operation. The `copy_max_bytes_hw`
> >>> + parameter is set by the device driver to the maximum number of
> >>> + bytes that can be copied in a single operation. Copy
> >>> + requests issued to the device must not exceed this limit.
> >>> + A value of 0 means that the device does not
> >>> + support copy offload.
> >>
> >> [RO] This is the maximum number of kilobytes supported in a
> >> single data copy offload operation. A value of 0 means that the
> >> device does not support copy offload.
> >>
> >
> > Nack, value is in bytes. Same as discard.
>
> Typo. I meant Bytes. Your text is too long an too convoluted, so unclear.
>
Acked, will update in next version
> --
> Damien Le Moal
> Western Digital Research
>
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 2/9] block: Add copy offload support infrastructure
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 1/9] block: Introduce queue limits for copy-offload support Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 8:56 ` Damien Le Moal
2023-03-27 8:40 ` [PATCH v8 3/9] block: add emulation for copy Anuj Gupta
` (6 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
Introduce blkdev_issue_copy which takes similar arguments as
copy_file_range and performs copy offload between two bdevs.
Introduce REQ_COPY copy offload operation flag. Create a read-write
bio pair with a token as payload and submitted to the device in order.
Read request populates token with source specific information which
is then passed with write request.
This design is courtesy Mikulas Patocka's token based copy
Larger copy will be divided, based on max_copy_sectors limit.
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
block/blk-lib.c | 236 ++++++++++++++++++++++++++++++++++++++
block/blk.h | 2 +
include/linux/blk_types.h | 25 ++++
include/linux/blkdev.h | 3 +
4 files changed, 266 insertions(+)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index e59c3069e835..cbc6882d1e7a 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -115,6 +115,242 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
}
EXPORT_SYMBOL(blkdev_issue_discard);
+/*
+ * For synchronous copy offload/emulation, wait and process all in-flight BIOs.
+ * This must only be called once all bios have been issued so that the refcount
+ * can only decrease. This just waits for all bios to make it through
+ * bio_copy_*_write_end_io. IO errors are propagated through cio->io_error.
+ */
+static int cio_await_completion(struct cio *cio)
+{
+ int ret = 0;
+
+ if (cio->endio)
+ return 0;
+
+ if (atomic_read(&cio->refcount)) {
+ __set_current_state(TASK_UNINTERRUPTIBLE);
+ blk_io_schedule();
+ }
+
+ ret = cio->comp_len;
+ kfree(cio);
+
+ return ret;
+}
+
+static void blk_copy_offload_write_end_io(struct bio *bio)
+{
+ struct copy_ctx *ctx = bio->bi_private;
+ struct cio *cio = ctx->cio;
+ sector_t clen;
+
+ if (bio->bi_status) {
+ clen = (bio->bi_iter.bi_sector << SECTOR_SHIFT) - cio->pos_out;
+ cio->comp_len = min_t(sector_t, clen, cio->comp_len);
+ }
+ __free_page(bio->bi_io_vec[0].bv_page);
+ bio_put(bio);
+
+ kfree(ctx);
+ if (atomic_dec_and_test(&cio->refcount)) {
+ if (cio->endio) {
+ cio->endio(cio->private, cio->comp_len);
+ kfree(cio);
+ } else
+ blk_wake_io_task(cio->waiter);
+ }
+}
+
+static void blk_copy_offload_read_end_io(struct bio *read_bio)
+{
+ struct copy_ctx *ctx = read_bio->bi_private;
+ struct cio *cio = ctx->cio;
+ sector_t clen;
+
+ if (read_bio->bi_status) {
+ clen = (read_bio->bi_iter.bi_sector << SECTOR_SHIFT)
+ - cio->pos_in;
+ cio->comp_len = min_t(sector_t, clen, cio->comp_len);
+ __free_page(read_bio->bi_io_vec[0].bv_page);
+ bio_put(ctx->write_bio);
+ bio_put(read_bio);
+ kfree(ctx);
+ if (atomic_dec_and_test(&cio->refcount)) {
+ if (cio->endio) {
+ cio->endio(cio->private, cio->comp_len);
+ kfree(cio);
+ } else
+ blk_wake_io_task(cio->waiter);
+ }
+ return;
+ }
+
+ schedule_work(&ctx->dispatch_work);
+ bio_put(read_bio);
+}
+
+static void blk_copy_dispatch_work_fn(struct work_struct *work)
+{
+ struct copy_ctx *ctx = container_of(work, struct copy_ctx,
+ dispatch_work);
+
+ submit_bio(ctx->write_bio);
+}
+
+/*
+ * __blk_copy_offload - Use device's native copy offload feature.
+ * we perform copy operation by sending 2 bio.
+ * 1. First we send a read bio with REQ_COPY flag along with a token and source
+ * and length. Once read bio reaches driver layer, device driver adds all the
+ * source info to token and does a fake completion.
+ * 2. Once read operation completes, we issue write with REQ_COPY flag with same
+ * token. In driver layer, token info is used to form a copy offload command.
+ *
+ * returns the length of bytes copied or negative error value
+ */
+static int __blk_copy_offload(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask)
+{
+ struct cio *cio;
+ struct copy_ctx *ctx;
+ struct bio *read_bio, *write_bio;
+ struct page *token;
+ sector_t copy_len;
+ sector_t rem, max_copy_len;
+
+ cio = kzalloc(sizeof(struct cio), GFP_KERNEL);
+ if (!cio)
+ return -ENOMEM;
+ atomic_set(&cio->refcount, 0);
+ cio->waiter = current;
+ cio->endio = end_io;
+ cio->private = private;
+
+ max_copy_len = min(bdev_max_copy_sectors(bdev_in),
+ bdev_max_copy_sectors(bdev_out)) << SECTOR_SHIFT;
+
+ cio->pos_in = pos_in;
+ cio->pos_out = pos_out;
+ cio->comp_len = len;
+ for (rem = len; rem > 0; rem -= copy_len) {
+ copy_len = min(rem, max_copy_len);
+
+ token = alloc_page(gfp_mask);
+ if (unlikely(!token))
+ goto err_token;
+
+ ctx = kzalloc(sizeof(struct copy_ctx), gfp_mask);
+ if (!ctx)
+ goto err_ctx;
+ read_bio = bio_alloc(bdev_in, 1, REQ_OP_READ | REQ_COPY
+ | REQ_SYNC | REQ_NOMERGE, gfp_mask);
+ if (!read_bio)
+ goto err_read_bio;
+ write_bio = bio_alloc(bdev_out, 1, REQ_OP_WRITE
+ | REQ_COPY | REQ_SYNC | REQ_NOMERGE, gfp_mask);
+ if (!write_bio)
+ goto err_write_bio;
+
+ ctx->cio = cio;
+ ctx->write_bio = write_bio;
+ INIT_WORK(&ctx->dispatch_work, blk_copy_dispatch_work_fn);
+
+ __bio_add_page(read_bio, token, PAGE_SIZE, 0);
+ read_bio->bi_iter.bi_size = copy_len;
+ read_bio->bi_iter.bi_sector = pos_in >> SECTOR_SHIFT;
+ read_bio->bi_end_io = blk_copy_offload_read_end_io;
+ read_bio->bi_private = ctx;
+
+ __bio_add_page(write_bio, token, PAGE_SIZE, 0);
+ write_bio->bi_iter.bi_size = copy_len;
+ write_bio->bi_end_io = blk_copy_offload_write_end_io;
+ write_bio->bi_iter.bi_sector = pos_out >> SECTOR_SHIFT;
+ write_bio->bi_private = ctx;
+
+ atomic_inc(&cio->refcount);
+ submit_bio(read_bio);
+ pos_in += copy_len;
+ pos_out += copy_len;
+ }
+
+ /* Wait for completion of all IO's*/
+ return cio_await_completion(cio);
+
+err_write_bio:
+ bio_put(read_bio);
+err_read_bio:
+ kfree(ctx);
+err_ctx:
+ __free_page(token);
+err_token:
+ cio->comp_len = min_t(sector_t, cio->comp_len, (len - rem));
+ return cio_await_completion(cio);
+}
+
+static inline int blk_copy_sanity_check(struct block_device *bdev_in,
+ loff_t pos_in, struct block_device *bdev_out, loff_t pos_out,
+ size_t len)
+{
+ unsigned int align = max(bdev_logical_block_size(bdev_out),
+ bdev_logical_block_size(bdev_in)) - 1;
+
+ if (bdev_read_only(bdev_out))
+ return -EPERM;
+
+ if ((pos_in & align) || (pos_out & align) || (len & align) || !len ||
+ len >= MAX_COPY_TOTAL_LENGTH)
+ return -EINVAL;
+
+ return 0;
+}
+
+static inline bool blk_check_copy_offload(struct request_queue *q_in,
+ struct request_queue *q_out)
+{
+ return blk_queue_copy(q_in) && blk_queue_copy(q_out);
+}
+
+/*
+ * @bdev_in: source block device
+ * @pos_in: source offset
+ * @bdev_out: destination block device
+ * @pos_out: destination offset
+ * @len: length in bytes to be copied
+ * @end_io: end_io function to be called on completion of copy operation,
+ * for synchronous operation this should be NULL
+ * @private: end_io function will be called with this private data, should be
+ * NULL, if operation is synchronous in nature
+ * @gfp_mask: memory allocation flags (for bio_alloc)
+ *
+ * Returns the length of bytes copied or a negative error value
+ *
+ * Description:
+ * Copy source offset from source block device to destination block
+ * device. length of a source range cannot be zero. Max total length of
+ * copy is limited to MAX_COPY_TOTAL_LENGTH
+ */
+int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask)
+{
+ struct request_queue *q_in = bdev_get_queue(bdev_in);
+ struct request_queue *q_out = bdev_get_queue(bdev_out);
+ int ret = -EINVAL;
+
+ ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
+ if (ret)
+ return ret;
+
+ if (blk_check_copy_offload(q_in, q_out))
+ ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
+ len, end_io, private, gfp_mask);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(blkdev_issue_copy);
+
static int __blkdev_issue_write_zeroes(struct block_device *bdev,
sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
struct bio **biop, unsigned flags)
diff --git a/block/blk.h b/block/blk.h
index d65d96994a94..684b8fa121db 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -311,6 +311,8 @@ static inline bool bio_may_exceed_limits(struct bio *bio,
break;
}
+ if (unlikely(op_is_copy(bio->bi_opf)))
+ return false;
/*
* All drivers must accept single-segments bios that are <= PAGE_SIZE.
* This is a quick and dirty check that relies on the fact that
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a0e339ff3d09..7f586c4b9954 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -423,6 +423,7 @@ enum req_flag_bits {
*/
/* for REQ_OP_WRITE_ZEROES: */
__REQ_NOUNMAP, /* do not free blocks when zeroing */
+ __REQ_COPY, /* copy request */
__REQ_NR_BITS, /* stops here */
};
@@ -452,6 +453,7 @@ enum req_flag_bits {
#define REQ_DRV (__force blk_opf_t)(1ULL << __REQ_DRV)
#define REQ_SWAP (__force blk_opf_t)(1ULL << __REQ_SWAP)
+#define REQ_COPY ((__force blk_opf_t)(1ULL << __REQ_COPY))
#define REQ_FAILFAST_MASK \
(REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
@@ -478,6 +480,11 @@ static inline bool op_is_write(blk_opf_t op)
return !!(op & (__force blk_opf_t)1);
}
+static inline bool op_is_copy(blk_opf_t op)
+{
+ return (op & REQ_COPY);
+}
+
/*
* Check if the bio or request is one that needs special treatment in the
* flush state machine.
@@ -537,4 +544,22 @@ struct blk_rq_stat {
u64 batch;
};
+typedef void (cio_iodone_t)(void *private, int comp_len);
+
+struct cio {
+ struct task_struct *waiter; /* waiting task (NULL if none) */
+ atomic_t refcount;
+ loff_t pos_in;
+ loff_t pos_out;
+ size_t comp_len;
+ cio_iodone_t *endio; /* applicable for async operation */
+ void *private; /* applicable for async operation */
+};
+
+struct copy_ctx {
+ struct cio *cio;
+ struct work_struct dispatch_work;
+ struct bio *write_bio;
+};
+
#endif /* __LINUX_BLK_TYPES_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 200338f2ec2e..1bb43697d43d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1054,6 +1054,9 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, struct bio **biop);
int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp);
+int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask);
#define BLKDEV_ZERO_NOUNMAP (1 << 0) /* do not free blocks */
#define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 2/9] block: Add copy offload support infrastructure
2023-03-27 8:40 ` [PATCH v8 2/9] block: Add copy offload support infrastructure Anuj Gupta
@ 2023-03-29 8:56 ` Damien Le Moal
0 siblings, 0 replies; 27+ messages in thread
From: Damien Le Moal @ 2023-03-29 8:56 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, joshi.k, nitheshshetty, gost.dev,
Nitesh Shetty, linux-block, linux-kernel, linux-nvme,
linux-fsdevel
On 3/27/23 17:40, Anuj Gupta wrote:
> From: Nitesh Shetty <nj.shetty@samsung.com>
>
> Introduce blkdev_issue_copy which takes similar arguments as
> copy_file_range and performs copy offload between two bdevs.
> Introduce REQ_COPY copy offload operation flag. Create a read-write
> bio pair with a token as payload and submitted to the device in order.
> Read request populates token with source specific information which
> is then passed with write request.
> This design is courtesy Mikulas Patocka's token based copy
>
> Larger copy will be divided, based on max_copy_sectors limit.
>
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> ---
> block/blk-lib.c | 236 ++++++++++++++++++++++++++++++++++++++
> block/blk.h | 2 +
> include/linux/blk_types.h | 25 ++++
> include/linux/blkdev.h | 3 +
> 4 files changed, 266 insertions(+)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index e59c3069e835..cbc6882d1e7a 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -115,6 +115,242 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> }
> EXPORT_SYMBOL(blkdev_issue_discard);
>
> +/*
> + * For synchronous copy offload/emulation, wait and process all in-flight BIOs.
> + * This must only be called once all bios have been issued so that the refcount
> + * can only decrease. This just waits for all bios to make it through
> + * bio_copy_*_write_end_io. IO errors are propagated through cio->io_error.
> + */
> +static int cio_await_completion(struct cio *cio)
Why not simply cio_wait_completion() ?
> +{
> + int ret = 0;
No need for the initialization.
> +
> + if (cio->endio)
> + return 0;
> +
> + if (atomic_read(&cio->refcount)) {
> + __set_current_state(TASK_UNINTERRUPTIBLE);
> + blk_io_schedule();
> + }
> +
> + ret = cio->comp_len;
What about cio->io_error as the top comment mentions ? Looking at struct cio,
there is no io_error field.
> + kfree(cio);
> +
> + return ret;
> +}
> +
> +static void blk_copy_offload_write_end_io(struct bio *bio)
> +{
> + struct copy_ctx *ctx = bio->bi_private;
> + struct cio *cio = ctx->cio;
> + sector_t clen;
> +
> + if (bio->bi_status) {
> + clen = (bio->bi_iter.bi_sector << SECTOR_SHIFT) - cio->pos_out;
> + cio->comp_len = min_t(sector_t, clen, cio->comp_len);
I do not understand. You set the length only if there was an error ? What about
the OK case ? Is this to handle partial completions ? If yes, then please add a
comment.
> + }
> + __free_page(bio->bi_io_vec[0].bv_page);
> + bio_put(bio);
> +
> + kfree(ctx);
> + if (atomic_dec_and_test(&cio->refcount)) {
Reverse this condition and return early.
> + if (cio->endio) {
> + cio->endio(cio->private, cio->comp_len);
> + kfree(cio);
> + } else
> + blk_wake_io_task(cio->waiter);
Missing curly braces for the else.
> + }
> +}
> +
> +static void blk_copy_offload_read_end_io(struct bio *read_bio)
> +{
> + struct copy_ctx *ctx = read_bio->bi_private;
> + struct cio *cio = ctx->cio;
> + sector_t clen;
> +
> + if (read_bio->bi_status) {
> + clen = (read_bio->bi_iter.bi_sector << SECTOR_SHIFT)
> + - cio->pos_in;
> + cio->comp_len = min_t(sector_t, clen, cio->comp_len);
> + __free_page(read_bio->bi_io_vec[0].bv_page);
> + bio_put(ctx->write_bio);
> + bio_put(read_bio);
> + kfree(ctx);
> + if (atomic_dec_and_test(&cio->refcount)) {
> + if (cio->endio) {
> + cio->endio(cio->private, cio->comp_len);
> + kfree(cio);
> + } else
> + blk_wake_io_task(cio->waiter);
Missing curly braces for the else.
> + }
> + return;
So you do all this only if there was an error ? What about an OK bio completion
(bi_status == BLK_STS_OK) ?
> + }
> +
> + schedule_work(&ctx->dispatch_work);
> + bio_put(read_bio);
> +}
> +
> +static void blk_copy_dispatch_work_fn(struct work_struct *work)
> +{
> + struct copy_ctx *ctx = container_of(work, struct copy_ctx,
> + dispatch_work);
> +
> + submit_bio(ctx->write_bio);
> +}
> +
> +/*
> + * __blk_copy_offload - Use device's native copy offload feature.
> + * we perform copy operation by sending 2 bio.
> + * 1. First we send a read bio with REQ_COPY flag along with a token and source
> + * and length. Once read bio reaches driver layer, device driver adds all the
> + * source info to token and does a fake completion.
> + * 2. Once read operation completes, we issue write with REQ_COPY flag with same
> + * token. In driver layer, token info is used to form a copy offload command.
> + *
> + * returns the length of bytes copied or negative error value
> + */
> +static int __blk_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> + struct block_device *bdev_out, loff_t pos_out, size_t len,
> + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> +{
> + struct cio *cio;
> + struct copy_ctx *ctx;
> + struct bio *read_bio, *write_bio;
> + struct page *token;
> + sector_t copy_len;
> + sector_t rem, max_copy_len;
> +
> + cio = kzalloc(sizeof(struct cio), GFP_KERNEL);
> + if (!cio)
> + return -ENOMEM;
> + atomic_set(&cio->refcount, 0);
> + cio->waiter = current;
> + cio->endio = end_io;
> + cio->private = private;
> +
> + max_copy_len = min(bdev_max_copy_sectors(bdev_in),
> + bdev_max_copy_sectors(bdev_out)) << SECTOR_SHIFT;
> +
> + cio->pos_in = pos_in;
> + cio->pos_out = pos_out;
> + cio->comp_len = len;
> + for (rem = len; rem > 0; rem -= copy_len) {
> + copy_len = min(rem, max_copy_len);
> +
> + token = alloc_page(gfp_mask);
> + if (unlikely(!token))
> + goto err_token;
> +
> + ctx = kzalloc(sizeof(struct copy_ctx), gfp_mask);
> + if (!ctx)
> + goto err_ctx;
> + read_bio = bio_alloc(bdev_in, 1, REQ_OP_READ | REQ_COPY
> + | REQ_SYNC | REQ_NOMERGE, gfp_mask);
> + if (!read_bio)
> + goto err_read_bio;
> + write_bio = bio_alloc(bdev_out, 1, REQ_OP_WRITE
> + | REQ_COPY | REQ_SYNC | REQ_NOMERGE, gfp_mask);
> + if (!write_bio)
> + goto err_write_bio;
> +
> + ctx->cio = cio;
> + ctx->write_bio = write_bio;
> + INIT_WORK(&ctx->dispatch_work, blk_copy_dispatch_work_fn);
> +
> + __bio_add_page(read_bio, token, PAGE_SIZE, 0);
> + read_bio->bi_iter.bi_size = copy_len;
> + read_bio->bi_iter.bi_sector = pos_in >> SECTOR_SHIFT;
> + read_bio->bi_end_io = blk_copy_offload_read_end_io;
> + read_bio->bi_private = ctx;
> +
> + __bio_add_page(write_bio, token, PAGE_SIZE, 0);
> + write_bio->bi_iter.bi_size = copy_len;
> + write_bio->bi_end_io = blk_copy_offload_write_end_io;
> + write_bio->bi_iter.bi_sector = pos_out >> SECTOR_SHIFT;
> + write_bio->bi_private = ctx;
> +
> + atomic_inc(&cio->refcount);
> + submit_bio(read_bio);
> + pos_in += copy_len;
> + pos_out += copy_len;
> + }
> +
> + /* Wait for completion of all IO's*/
> + return cio_await_completion(cio);
> +
> +err_write_bio:
> + bio_put(read_bio);
> +err_read_bio:
> + kfree(ctx);
> +err_ctx:
> + __free_page(token);
> +err_token:
> + cio->comp_len = min_t(sector_t, cio->comp_len, (len - rem));
Why ? cio_wait_completion() return that, no ?
> + return cio_await_completion(cio);
> +}
> +
> +static inline int blk_copy_sanity_check(struct block_device *bdev_in,
> + loff_t pos_in, struct block_device *bdev_out, loff_t pos_out,
> + size_t len)
> +{
> + unsigned int align = max(bdev_logical_block_size(bdev_out),
> + bdev_logical_block_size(bdev_in)) - 1;
> +
> + if (bdev_read_only(bdev_out))
> + return -EPERM;
> +
> + if ((pos_in & align) || (pos_out & align) || (len & align) || !len ||
> + len >= MAX_COPY_TOTAL_LENGTH)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static inline bool blk_check_copy_offload(struct request_queue *q_in,
> + struct request_queue *q_out)
> +{
> + return blk_queue_copy(q_in) && blk_queue_copy(q_out);
> +}
Not a very useful helper.
> +
> +/*
> + * @bdev_in: source block device
> + * @pos_in: source offset
> + * @bdev_out: destination block device
> + * @pos_out: destination offset
> + * @len: length in bytes to be copied
> + * @end_io: end_io function to be called on completion of copy operation,
> + * for synchronous operation this should be NULL
> + * @private: end_io function will be called with this private data, should be
> + * NULL, if operation is synchronous in nature
> + * @gfp_mask: memory allocation flags (for bio_alloc)
> + *
> + * Returns the length of bytes copied or a negative error value
> + *
> + * Description:
> + * Copy source offset from source block device to destination block
> + * device. length of a source range cannot be zero. Max total length of
> + * copy is limited to MAX_COPY_TOTAL_LENGTH
> + */
> +int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
> + struct block_device *bdev_out, loff_t pos_out, size_t len,
> + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> +{
> + struct request_queue *q_in = bdev_get_queue(bdev_in);
> + struct request_queue *q_out = bdev_get_queue(bdev_out);
> + int ret = -EINVAL;
> +
> + ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
> + if (ret)
> + return ret;
> +
> + if (blk_check_copy_offload(q_in, q_out))
> + ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
> + len, end_io, private, gfp_mask);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(blkdev_issue_copy);
> +
> static int __blkdev_issue_write_zeroes(struct block_device *bdev,
> sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
> struct bio **biop, unsigned flags)
> diff --git a/block/blk.h b/block/blk.h
> index d65d96994a94..684b8fa121db 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -311,6 +311,8 @@ static inline bool bio_may_exceed_limits(struct bio *bio,
> break;
> }
>
> + if (unlikely(op_is_copy(bio->bi_opf)))
> + return false;
> /*
> * All drivers must accept single-segments bios that are <= PAGE_SIZE.
> * This is a quick and dirty check that relies on the fact that
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index a0e339ff3d09..7f586c4b9954 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -423,6 +423,7 @@ enum req_flag_bits {
> */
> /* for REQ_OP_WRITE_ZEROES: */
> __REQ_NOUNMAP, /* do not free blocks when zeroing */
> + __REQ_COPY, /* copy request */
>
> __REQ_NR_BITS, /* stops here */
> };
> @@ -452,6 +453,7 @@ enum req_flag_bits {
>
> #define REQ_DRV (__force blk_opf_t)(1ULL << __REQ_DRV)
> #define REQ_SWAP (__force blk_opf_t)(1ULL << __REQ_SWAP)
> +#define REQ_COPY ((__force blk_opf_t)(1ULL << __REQ_COPY))
>
> #define REQ_FAILFAST_MASK \
> (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER)
> @@ -478,6 +480,11 @@ static inline bool op_is_write(blk_opf_t op)
> return !!(op & (__force blk_opf_t)1);
> }
>
> +static inline bool op_is_copy(blk_opf_t op)
> +{
> + return (op & REQ_COPY);
No need for the parenthesis.
> +}
> +
> /*
> * Check if the bio or request is one that needs special treatment in the
> * flush state machine.
> @@ -537,4 +544,22 @@ struct blk_rq_stat {
> u64 batch;
> };
>
> +typedef void (cio_iodone_t)(void *private, int comp_len);
Not really needed I think.
> +
> +struct cio {
> + struct task_struct *waiter; /* waiting task (NULL if none) */
> + atomic_t refcount;
> + loff_t pos_in;
> + loff_t pos_out;
> + size_t comp_len;
> + cio_iodone_t *endio; /* applicable for async operation */
> + void *private; /* applicable for async operation */
> +};
> +
> +struct copy_ctx {
> + struct cio *cio;
> + struct work_struct dispatch_work;
> + struct bio *write_bio;
> +};
> +
> #endif /* __LINUX_BLK_TYPES_H */
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 200338f2ec2e..1bb43697d43d 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1054,6 +1054,9 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, struct bio **biop);
> int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp);
> +int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
> + struct block_device *bdev_out, loff_t pos_out, size_t len,
> + cio_iodone_t end_io, void *private, gfp_t gfp_mask);
>
> #define BLKDEV_ZERO_NOUNMAP (1 << 0) /* do not free blocks */
> #define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 3/9] block: add emulation for copy
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 1/9] block: Introduce queue limits for copy-offload support Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 2/9] block: Add copy offload support infrastructure Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device Anuj Gupta
` (5 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, Vincent Fu, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
For the devices which does not support copy, copy emulation is added.
It is required for in-kernel users like fabrics, where file descriptor is
not available and hence they can't use copy_file_range.
Copy-emulation is implemented by reading from source into memory and
writing to the corresponding destination asynchronously.
Also emulation is used, if copy offload fails or partially completes.
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
block/blk-lib.c | 171 ++++++++++++++++++++++++++++++++++++++++-
block/blk-map.c | 4 +-
include/linux/blkdev.h | 3 +
3 files changed, 175 insertions(+), 3 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index cbc6882d1e7a..a21819e59b29 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -289,6 +289,169 @@ static int __blk_copy_offload(struct block_device *bdev_in, loff_t pos_in,
return cio_await_completion(cio);
}
+static void *blk_alloc_buf(sector_t req_size, sector_t *alloc_size,
+ gfp_t gfp_mask)
+{
+ int min_size = PAGE_SIZE;
+ void *buf;
+
+ while (req_size >= min_size) {
+ buf = kvmalloc(req_size, gfp_mask);
+ if (buf) {
+ *alloc_size = req_size;
+ return buf;
+ }
+ /* retry half the requested size */
+ req_size >>= 1;
+ }
+
+ return NULL;
+}
+
+static void blk_copy_emulate_write_end_io(struct bio *bio)
+{
+ struct copy_ctx *ctx = bio->bi_private;
+ struct cio *cio = ctx->cio;
+ sector_t clen;
+
+ if (bio->bi_status) {
+ clen = (bio->bi_iter.bi_sector << SECTOR_SHIFT) - cio->pos_out;
+ cio->comp_len = min_t(sector_t, clen, cio->comp_len);
+ }
+ kvfree(page_address(bio->bi_io_vec[0].bv_page));
+ bio_map_kern_endio(bio);
+ kfree(ctx);
+ if (atomic_dec_and_test(&cio->refcount)) {
+ if (cio->endio) {
+ cio->endio(cio->private, cio->comp_len);
+ kfree(cio);
+ } else
+ blk_wake_io_task(cio->waiter);
+ }
+}
+
+static void blk_copy_emulate_read_end_io(struct bio *read_bio)
+{
+ struct copy_ctx *ctx = read_bio->bi_private;
+ struct cio *cio = ctx->cio;
+ sector_t clen;
+
+ if (read_bio->bi_status) {
+ clen = (read_bio->bi_iter.bi_sector << SECTOR_SHIFT) -
+ cio->pos_in;
+ cio->comp_len = min_t(sector_t, clen, cio->comp_len);
+ __free_page(read_bio->bi_io_vec[0].bv_page);
+ bio_map_kern_endio(read_bio);
+ kfree(ctx);
+
+ if (atomic_dec_and_test(&cio->refcount)) {
+ if (cio->endio) {
+ cio->endio(cio->private, cio->comp_len);
+ kfree(cio);
+ } else
+ blk_wake_io_task(cio->waiter);
+ }
+ }
+ schedule_work(&ctx->dispatch_work);
+ kfree(read_bio);
+}
+
+/*
+ * If native copy offload feature is absent, this function tries to emulate,
+ * by copying data from source to a temporary buffer and from buffer to
+ * destination device.
+ * returns the length of bytes copied or negative error value
+ */
+static int __blk_copy_emulate(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask)
+{
+ struct request_queue *in = bdev_get_queue(bdev_in);
+ struct request_queue *out = bdev_get_queue(bdev_out);
+ struct bio *read_bio, *write_bio;
+ void *buf = NULL;
+ struct copy_ctx *ctx;
+ struct cio *cio;
+ sector_t buf_len, req_len, rem = 0;
+ sector_t max_src_hw_len = min_t(unsigned int,
+ queue_max_hw_sectors(in),
+ queue_max_segments(in) << (PAGE_SHIFT - SECTOR_SHIFT))
+ << SECTOR_SHIFT;
+ sector_t max_dst_hw_len = min_t(unsigned int,
+ queue_max_hw_sectors(out),
+ queue_max_segments(out) << (PAGE_SHIFT - SECTOR_SHIFT))
+ << SECTOR_SHIFT;
+ sector_t max_hw_len = min_t(unsigned int,
+ max_src_hw_len, max_dst_hw_len);
+
+ cio = kzalloc(sizeof(struct cio), GFP_KERNEL);
+ if (!cio)
+ return -ENOMEM;
+ atomic_set(&cio->refcount, 0);
+ cio->pos_in = pos_in;
+ cio->pos_out = pos_out;
+ cio->waiter = current;
+ cio->endio = end_io;
+ cio->private = private;
+
+ for (rem = len; rem > 0; rem -= buf_len) {
+ req_len = min_t(int, max_hw_len, rem);
+
+ buf = blk_alloc_buf(req_len, &buf_len, gfp_mask);
+ if (!buf)
+ goto err_alloc_buf;
+
+ ctx = kzalloc(sizeof(struct copy_ctx), gfp_mask);
+ if (!ctx)
+ goto err_ctx;
+
+ read_bio = bio_map_kern(in, buf, buf_len, gfp_mask);
+ if (IS_ERR(read_bio))
+ goto err_read_bio;
+
+ write_bio = bio_map_kern(out, buf, buf_len, gfp_mask);
+ if (IS_ERR(write_bio))
+ goto err_write_bio;
+
+ ctx->cio = cio;
+ ctx->write_bio = write_bio;
+ INIT_WORK(&ctx->dispatch_work, blk_copy_dispatch_work_fn);
+
+ read_bio->bi_iter.bi_sector = pos_in >> SECTOR_SHIFT;
+ read_bio->bi_iter.bi_size = buf_len;
+ read_bio->bi_opf = REQ_OP_READ | REQ_SYNC;
+ bio_set_dev(read_bio, bdev_in);
+ read_bio->bi_end_io = blk_copy_emulate_read_end_io;
+ read_bio->bi_private = ctx;
+
+ write_bio->bi_iter.bi_size = buf_len;
+ write_bio->bi_opf = REQ_OP_WRITE | REQ_SYNC;
+ bio_set_dev(write_bio, bdev_out);
+ write_bio->bi_end_io = blk_copy_emulate_write_end_io;
+ write_bio->bi_iter.bi_sector = pos_out >> SECTOR_SHIFT;
+ write_bio->bi_private = ctx;
+
+ atomic_inc(&cio->refcount);
+ submit_bio(read_bio);
+
+ pos_in += buf_len;
+ pos_out += buf_len;
+ }
+
+ /* Wait for completion of all IO's*/
+ return cio_await_completion(cio);
+
+err_write_bio:
+ bio_put(read_bio);
+err_read_bio:
+ kfree(ctx);
+err_ctx:
+ kvfree(buf);
+err_alloc_buf:
+ cio->comp_len -= min_t(sector_t, cio->comp_len, len - rem);
+ return cio_await_completion(cio);
+}
+
static inline int blk_copy_sanity_check(struct block_device *bdev_in,
loff_t pos_in, struct block_device *bdev_out, loff_t pos_out,
size_t len)
@@ -338,15 +501,21 @@ int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
struct request_queue *q_in = bdev_get_queue(bdev_in);
struct request_queue *q_out = bdev_get_queue(bdev_out);
int ret = -EINVAL;
+ bool offload = false;
ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
if (ret)
return ret;
- if (blk_check_copy_offload(q_in, q_out))
+ offload = blk_check_copy_offload(q_in, q_out);
+ if (offload)
ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
len, end_io, private, gfp_mask);
+ if ((ret != len) || !offload)
+ ret = __blk_copy_emulate(bdev_in, pos_in + ret, bdev_out,
+ pos_out + ret, len - ret, end_io, private, gfp_mask);
+
return ret;
}
EXPORT_SYMBOL_GPL(blkdev_issue_copy);
diff --git a/block/blk-map.c b/block/blk-map.c
index 7b12f4bb4d4c..be2414904319 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -362,7 +362,7 @@ static void bio_invalidate_vmalloc_pages(struct bio *bio)
#endif
}
-static void bio_map_kern_endio(struct bio *bio)
+void bio_map_kern_endio(struct bio *bio)
{
bio_invalidate_vmalloc_pages(bio);
bio_uninit(bio);
@@ -379,7 +379,7 @@ static void bio_map_kern_endio(struct bio *bio)
* Map the kernel address into a bio suitable for io to a block
* device. Returns an error pointer in case of error.
*/
-static struct bio *bio_map_kern(struct request_queue *q, void *data,
+struct bio *bio_map_kern(struct request_queue *q, void *data,
unsigned int len, gfp_t gfp_mask)
{
unsigned long kaddr = (unsigned long)data;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1bb43697d43d..a54153610800 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1057,6 +1057,9 @@ int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
struct block_device *bdev_out, loff_t pos_out, size_t len,
cio_iodone_t end_io, void *private, gfp_t gfp_mask);
+struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len,
+ gfp_t gfp_mask);
+void bio_map_kern_endio(struct bio *bio);
#define BLKDEV_ZERO_NOUNMAP (1 << 0) /* do not free blocks */
#define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (2 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 3/9] block: add emulation for copy Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 12:14 ` Christian Brauner
` (2 more replies)
2023-03-27 8:40 ` [PATCH v8 5/9] nvme: add copy offload support Anuj Gupta
` (4 subsequent siblings)
8 siblings, 3 replies; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
For direct block device opened with O_DIRECT, use copy_file_range to
issue device copy offload, and fallback to generic_copy_file_range incase
device copy offload capability is absent.
Modify checks to allow bdevs to use copy_file_range.
Suggested-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
---
block/blk-lib.c | 22 ++++++++++++++++++++++
block/fops.c | 20 ++++++++++++++++++++
fs/read_write.c | 11 +++++++++--
include/linux/blkdev.h | 3 +++
4 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index a21819e59b29..c288573c7e77 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -475,6 +475,28 @@ static inline bool blk_check_copy_offload(struct request_queue *q_in,
return blk_queue_copy(q_in) && blk_queue_copy(q_out);
}
+int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask)
+{
+ struct request_queue *in_q = bdev_get_queue(bdev_in);
+ struct request_queue *out_q = bdev_get_queue(bdev_out);
+ int ret = -EINVAL;
+ bool offload = false;
+
+ ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
+ if (ret)
+ return ret;
+
+ offload = blk_check_copy_offload(in_q, out_q);
+ if (offload)
+ ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
+ len, end_io, private, gfp_mask);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(blkdev_copy_offload);
+
/*
* @bdev_in: source block device
* @pos_in: source offset
diff --git a/block/fops.c b/block/fops.c
index d2e6be4e3d1c..3b7c05831d5c 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -611,6 +611,25 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
return ret;
}
+static ssize_t blkdev_copy_file_range(struct file *file_in, loff_t pos_in,
+ struct file *file_out, loff_t pos_out,
+ size_t len, unsigned int flags)
+{
+ struct block_device *in_bdev = I_BDEV(bdev_file_inode(file_in));
+ struct block_device *out_bdev = I_BDEV(bdev_file_inode(file_out));
+ int comp_len = 0;
+
+ if ((file_in->f_iocb_flags & IOCB_DIRECT) &&
+ (file_out->f_iocb_flags & IOCB_DIRECT))
+ comp_len = blkdev_copy_offload(in_bdev, pos_in, out_bdev,
+ pos_out, len, NULL, NULL, GFP_KERNEL);
+ if (comp_len != len)
+ comp_len = generic_copy_file_range(file_in, pos_in + comp_len,
+ file_out, pos_out + comp_len, len - comp_len, flags);
+
+ return comp_len;
+}
+
#define BLKDEV_FALLOC_FL_SUPPORTED \
(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
FALLOC_FL_ZERO_RANGE | FALLOC_FL_NO_HIDE_STALE)
@@ -694,6 +713,7 @@ const struct file_operations def_blk_fops = {
.splice_read = generic_file_splice_read,
.splice_write = iter_file_splice_write,
.fallocate = blkdev_fallocate,
+ .copy_file_range = blkdev_copy_file_range,
};
static __init int blkdev_init(void)
diff --git a/fs/read_write.c b/fs/read_write.c
index 7a2ff6157eda..62e925e9b2f0 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -20,6 +20,7 @@
#include <linux/compat.h>
#include <linux/mount.h>
#include <linux/fs.h>
+#include <linux/blkdev.h>
#include "internal.h"
#include <linux/uaccess.h>
@@ -1448,7 +1449,11 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
return -EOVERFLOW;
/* Shorten the copy to EOF */
- size_in = i_size_read(inode_in);
+ if (S_ISBLK(inode_in->i_mode))
+ size_in = bdev_nr_bytes(I_BDEV(file_in->f_mapping->host));
+ else
+ size_in = i_size_read(inode_in);
+
if (pos_in >= size_in)
count = 0;
else
@@ -1709,7 +1714,9 @@ int generic_file_rw_checks(struct file *file_in, struct file *file_out)
/* Don't copy dirs, pipes, sockets... */
if (S_ISDIR(inode_in->i_mode) || S_ISDIR(inode_out->i_mode))
return -EISDIR;
- if (!S_ISREG(inode_in->i_mode) || !S_ISREG(inode_out->i_mode))
+
+ if ((!S_ISREG(inode_in->i_mode) || !S_ISREG(inode_out->i_mode)) &&
+ (!S_ISBLK(inode_in->i_mode) || !S_ISBLK(inode_out->i_mode)))
return -EINVAL;
if (!(file_in->f_mode & FMODE_READ) ||
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index a54153610800..468d5f3378e2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1057,6 +1057,9 @@ int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
int blkdev_issue_copy(struct block_device *bdev_in, loff_t pos_in,
struct block_device *bdev_out, loff_t pos_out, size_t len,
cio_iodone_t end_io, void *private, gfp_t gfp_mask);
+int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
+ struct block_device *bdev_out, loff_t pos_out, size_t len,
+ cio_iodone_t end_io, void *private, gfp_t gfp_mask);
struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len,
gfp_t gfp_mask);
void bio_map_kern_endio(struct bio *bio);
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-27 8:40 ` [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device Anuj Gupta
@ 2023-03-29 12:14 ` Christian Brauner
2023-03-29 12:42 ` Nitesh Shetty
2023-03-29 14:07 ` kernel test robot
2023-03-29 15:30 ` kernel test robot
2 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2023-03-29 12:14 UTC (permalink / raw)
To: Anuj Gupta
Cc: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, bvanassche, hare, ming.lei, damien.lemoal,
joshi.k, nitheshshetty, gost.dev, Nitesh Shetty, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
On Mon, Mar 27, 2023 at 02:10:52PM +0530, Anuj Gupta wrote:
> From: Nitesh Shetty <nj.shetty@samsung.com>
>
> For direct block device opened with O_DIRECT, use copy_file_range to
> issue device copy offload, and fallback to generic_copy_file_range incase
> device copy offload capability is absent.
> Modify checks to allow bdevs to use copy_file_range.
>
> Suggested-by: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> ---
> block/blk-lib.c | 22 ++++++++++++++++++++++
> block/fops.c | 20 ++++++++++++++++++++
> fs/read_write.c | 11 +++++++++--
> include/linux/blkdev.h | 3 +++
> 4 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index a21819e59b29..c288573c7e77 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -475,6 +475,28 @@ static inline bool blk_check_copy_offload(struct request_queue *q_in,
> return blk_queue_copy(q_in) && blk_queue_copy(q_out);
> }
>
> +int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> + struct block_device *bdev_out, loff_t pos_out, size_t len,
> + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> +{
> + struct request_queue *in_q = bdev_get_queue(bdev_in);
> + struct request_queue *out_q = bdev_get_queue(bdev_out);
> + int ret = -EINVAL;
Why initialize to -EINVAL if blk_copy_sanity_check() initializes it
right away anyway?
> + bool offload = false;
Same thing with initializing offload.
> +
> + ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
> + if (ret)
> + return ret;
> +
> + offload = blk_check_copy_offload(in_q, out_q);
> + if (offload)
> + ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
> + len, end_io, private, gfp_mask);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(blkdev_copy_offload);
> +
> /*
> * @bdev_in: source block device
> * @pos_in: source offset
> diff --git a/block/fops.c b/block/fops.c
> index d2e6be4e3d1c..3b7c05831d5c 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -611,6 +611,25 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
> return ret;
> }
>
> +static ssize_t blkdev_copy_file_range(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out,
> + size_t len, unsigned int flags)
> +{
> + struct block_device *in_bdev = I_BDEV(bdev_file_inode(file_in));
> + struct block_device *out_bdev = I_BDEV(bdev_file_inode(file_out));
> + int comp_len = 0;
> +
> + if ((file_in->f_iocb_flags & IOCB_DIRECT) &&
> + (file_out->f_iocb_flags & IOCB_DIRECT))
> + comp_len = blkdev_copy_offload(in_bdev, pos_in, out_bdev,
> + pos_out, len, NULL, NULL, GFP_KERNEL);
> + if (comp_len != len)
> + comp_len = generic_copy_file_range(file_in, pos_in + comp_len,
> + file_out, pos_out + comp_len, len - comp_len, flags);
I'm not deeply familiar with this code but this looks odd. It at least
seems possible that comp_len could be -EINVAL and len 20 at which point
you'd be doing len - comp_len aka 20 - 22 = -2 in generic_copy_file_range().
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-29 12:14 ` Christian Brauner
@ 2023-03-29 12:42 ` Nitesh Shetty
2023-03-30 5:48 ` Christian Brauner
0 siblings, 1 reply; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-29 12:42 UTC (permalink / raw)
To: Christian Brauner
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, bvanassche, hare, ming.lei,
damien.lemoal, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 3628 bytes --]
On Wed, Mar 29, 2023 at 02:14:40PM +0200, Christian Brauner wrote:
> On Mon, Mar 27, 2023 at 02:10:52PM +0530, Anuj Gupta wrote:
> > From: Nitesh Shetty <nj.shetty@samsung.com>
> >
> > For direct block device opened with O_DIRECT, use copy_file_range to
> > issue device copy offload, and fallback to generic_copy_file_range incase
> > device copy offload capability is absent.
> > Modify checks to allow bdevs to use copy_file_range.
> >
> > Suggested-by: Ming Lei <ming.lei@redhat.com>
> > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > ---
> > block/blk-lib.c | 22 ++++++++++++++++++++++
> > block/fops.c | 20 ++++++++++++++++++++
> > fs/read_write.c | 11 +++++++++--
> > include/linux/blkdev.h | 3 +++
> > 4 files changed, 54 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/blk-lib.c b/block/blk-lib.c
> > index a21819e59b29..c288573c7e77 100644
> > --- a/block/blk-lib.c
> > +++ b/block/blk-lib.c
> > @@ -475,6 +475,28 @@ static inline bool blk_check_copy_offload(struct request_queue *q_in,
> > return blk_queue_copy(q_in) && blk_queue_copy(q_out);
> > }
> >
> > +int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> > + struct block_device *bdev_out, loff_t pos_out, size_t len,
> > + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> > +{
> > + struct request_queue *in_q = bdev_get_queue(bdev_in);
> > + struct request_queue *out_q = bdev_get_queue(bdev_out);
> > + int ret = -EINVAL;
>
> Why initialize to -EINVAL if blk_copy_sanity_check() initializes it
> right away anyway?
>
acked.
> > + bool offload = false;
>
> Same thing with initializing offload.
>
acked
> > +
> > + ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
> > + if (ret)
> > + return ret;
> > +
> > + offload = blk_check_copy_offload(in_q, out_q);
> > + if (offload)
> > + ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
> > + len, end_io, private, gfp_mask);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(blkdev_copy_offload);
> > +
> > /*
> > * @bdev_in: source block device
> > * @pos_in: source offset
> > diff --git a/block/fops.c b/block/fops.c
> > index d2e6be4e3d1c..3b7c05831d5c 100644
> > --- a/block/fops.c
> > +++ b/block/fops.c
> > @@ -611,6 +611,25 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > return ret;
> > }
> >
> > +static ssize_t blkdev_copy_file_range(struct file *file_in, loff_t pos_in,
> > + struct file *file_out, loff_t pos_out,
> > + size_t len, unsigned int flags)
> > +{
> > + struct block_device *in_bdev = I_BDEV(bdev_file_inode(file_in));
> > + struct block_device *out_bdev = I_BDEV(bdev_file_inode(file_out));
> > + int comp_len = 0;
> > +
> > + if ((file_in->f_iocb_flags & IOCB_DIRECT) &&
> > + (file_out->f_iocb_flags & IOCB_DIRECT))
> > + comp_len = blkdev_copy_offload(in_bdev, pos_in, out_bdev,
> > + pos_out, len, NULL, NULL, GFP_KERNEL);
> > + if (comp_len != len)
> > + comp_len = generic_copy_file_range(file_in, pos_in + comp_len,
> > + file_out, pos_out + comp_len, len - comp_len, flags);
>
> I'm not deeply familiar with this code but this looks odd. It at least
> seems possible that comp_len could be -EINVAL and len 20 at which point
> you'd be doing len - comp_len aka 20 - 22 = -2 in generic_copy_file_range().
comp_len should be 0 incase of error. We do agree, some function
description needs to be updated. We will recheck this completion path to
make sure not to return negative value, incase of failure.
Thank You,
Nitesh Shetty
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-29 12:42 ` Nitesh Shetty
@ 2023-03-30 5:48 ` Christian Brauner
2023-03-30 15:21 ` Nitesh Shetty
0 siblings, 1 reply; 27+ messages in thread
From: Christian Brauner @ 2023-03-30 5:48 UTC (permalink / raw)
To: Nitesh Shetty
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, bvanassche, hare, ming.lei,
damien.lemoal, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
On Wed, Mar 29, 2023 at 06:12:36PM +0530, Nitesh Shetty wrote:
> On Wed, Mar 29, 2023 at 02:14:40PM +0200, Christian Brauner wrote:
> > On Mon, Mar 27, 2023 at 02:10:52PM +0530, Anuj Gupta wrote:
> > > From: Nitesh Shetty <nj.shetty@samsung.com>
> > >
> > > For direct block device opened with O_DIRECT, use copy_file_range to
> > > issue device copy offload, and fallback to generic_copy_file_range incase
> > > device copy offload capability is absent.
> > > Modify checks to allow bdevs to use copy_file_range.
> > >
> > > Suggested-by: Ming Lei <ming.lei@redhat.com>
> > > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > > ---
> > > block/blk-lib.c | 22 ++++++++++++++++++++++
> > > block/fops.c | 20 ++++++++++++++++++++
> > > fs/read_write.c | 11 +++++++++--
> > > include/linux/blkdev.h | 3 +++
> > > 4 files changed, 54 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/block/blk-lib.c b/block/blk-lib.c
> > > index a21819e59b29..c288573c7e77 100644
> > > --- a/block/blk-lib.c
> > > +++ b/block/blk-lib.c
> > > @@ -475,6 +475,28 @@ static inline bool blk_check_copy_offload(struct request_queue *q_in,
> > > return blk_queue_copy(q_in) && blk_queue_copy(q_out);
> > > }
> > >
> > > +int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> > > + struct block_device *bdev_out, loff_t pos_out, size_t len,
> > > + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> > > +{
> > > + struct request_queue *in_q = bdev_get_queue(bdev_in);
> > > + struct request_queue *out_q = bdev_get_queue(bdev_out);
> > > + int ret = -EINVAL;
> >
> > Why initialize to -EINVAL if blk_copy_sanity_check() initializes it
> > right away anyway?
> >
>
> acked.
>
> > > + bool offload = false;
> >
> > Same thing with initializing offload.
> >
> acked
>
> > > +
> > > + ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + offload = blk_check_copy_offload(in_q, out_q);
> > > + if (offload)
> > > + ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
> > > + len, end_io, private, gfp_mask);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(blkdev_copy_offload);
> > > +
> > > /*
> > > * @bdev_in: source block device
> > > * @pos_in: source offset
> > > diff --git a/block/fops.c b/block/fops.c
> > > index d2e6be4e3d1c..3b7c05831d5c 100644
> > > --- a/block/fops.c
> > > +++ b/block/fops.c
> > > @@ -611,6 +611,25 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > > return ret;
> > > }
> > >
> > > +static ssize_t blkdev_copy_file_range(struct file *file_in, loff_t pos_in,
> > > + struct file *file_out, loff_t pos_out,
> > > + size_t len, unsigned int flags)
> > > +{
> > > + struct block_device *in_bdev = I_BDEV(bdev_file_inode(file_in));
> > > + struct block_device *out_bdev = I_BDEV(bdev_file_inode(file_out));
> > > + int comp_len = 0;
> > > +
> > > + if ((file_in->f_iocb_flags & IOCB_DIRECT) &&
> > > + (file_out->f_iocb_flags & IOCB_DIRECT))
> > > + comp_len = blkdev_copy_offload(in_bdev, pos_in, out_bdev,
> > > + pos_out, len, NULL, NULL, GFP_KERNEL);
> > > + if (comp_len != len)
> > > + comp_len = generic_copy_file_range(file_in, pos_in + comp_len,
> > > + file_out, pos_out + comp_len, len - comp_len, flags);
> >
> > I'm not deeply familiar with this code but this looks odd. It at least
> > seems possible that comp_len could be -EINVAL and len 20 at which point
> > you'd be doing len - comp_len aka 20 - 22 = -2 in generic_copy_file_range().
20 - -22 = 44 ofc
>
> comp_len should be 0 incase of error. We do agree, some function
I mean, not to hammer on this point too much but just to be clear
blk_copy_sanity_check(), which is introduced in the second patch, can
return both -EPERM and -EINVAL and is first called in
blkdev_copy_offload() so it's definitely possible for comp_len to be
negative.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-30 5:48 ` Christian Brauner
@ 2023-03-30 15:21 ` Nitesh Shetty
0 siblings, 0 replies; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-30 15:21 UTC (permalink / raw)
To: Christian Brauner
Cc: Nitesh Shetty, Anuj Gupta, Jens Axboe, Alasdair Kergon,
Mike Snitzer, dm-devel, Keith Busch, Christoph Hellwig,
Sagi Grimberg, James Smart, Chaitanya Kulkarni, Alexander Viro,
bvanassche, hare, ming.lei, damien.lemoal, joshi.k, gost.dev,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
On Thu, Mar 30, 2023 at 11:18 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, Mar 29, 2023 at 06:12:36PM +0530, Nitesh Shetty wrote:
> > On Wed, Mar 29, 2023 at 02:14:40PM +0200, Christian Brauner wrote:
> > > On Mon, Mar 27, 2023 at 02:10:52PM +0530, Anuj Gupta wrote:
> > > > From: Nitesh Shetty <nj.shetty@samsung.com>
> > > >
> > > > For direct block device opened with O_DIRECT, use copy_file_range to
> > > > issue device copy offload, and fallback to generic_copy_file_range incase
> > > > device copy offload capability is absent.
> > > > Modify checks to allow bdevs to use copy_file_range.
> > > >
> > > > Suggested-by: Ming Lei <ming.lei@redhat.com>
> > > > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> > > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > > > ---
> > > > block/blk-lib.c | 22 ++++++++++++++++++++++
> > > > block/fops.c | 20 ++++++++++++++++++++
> > > > fs/read_write.c | 11 +++++++++--
> > > > include/linux/blkdev.h | 3 +++
> > > > 4 files changed, 54 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/block/blk-lib.c b/block/blk-lib.c
> > > > index a21819e59b29..c288573c7e77 100644
> > > > --- a/block/blk-lib.c
> > > > +++ b/block/blk-lib.c
> > > > @@ -475,6 +475,28 @@ static inline bool blk_check_copy_offload(struct request_queue *q_in,
> > > > return blk_queue_copy(q_in) && blk_queue_copy(q_out);
> > > > }
> > > >
> > > > +int blkdev_copy_offload(struct block_device *bdev_in, loff_t pos_in,
> > > > + struct block_device *bdev_out, loff_t pos_out, size_t len,
> > > > + cio_iodone_t end_io, void *private, gfp_t gfp_mask)
> > > > +{
> > > > + struct request_queue *in_q = bdev_get_queue(bdev_in);
> > > > + struct request_queue *out_q = bdev_get_queue(bdev_out);
> > > > + int ret = -EINVAL;
> > >
> > > Why initialize to -EINVAL if blk_copy_sanity_check() initializes it
> > > right away anyway?
> > >
> >
> > acked.
> >
> > > > + bool offload = false;
> > >
> > > Same thing with initializing offload.
> > >
> > acked
> >
> > > > +
> > > > + ret = blk_copy_sanity_check(bdev_in, pos_in, bdev_out, pos_out, len);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + offload = blk_check_copy_offload(in_q, out_q);
> > > > + if (offload)
> > > > + ret = __blk_copy_offload(bdev_in, pos_in, bdev_out, pos_out,
> > > > + len, end_io, private, gfp_mask);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(blkdev_copy_offload);
> > > > +
> > > > /*
> > > > * @bdev_in: source block device
> > > > * @pos_in: source offset
> > > > diff --git a/block/fops.c b/block/fops.c
> > > > index d2e6be4e3d1c..3b7c05831d5c 100644
> > > > --- a/block/fops.c
> > > > +++ b/block/fops.c
> > > > @@ -611,6 +611,25 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > > > return ret;
> > > > }
> > > >
> > > > +static ssize_t blkdev_copy_file_range(struct file *file_in, loff_t pos_in,
> > > > + struct file *file_out, loff_t pos_out,
> > > > + size_t len, unsigned int flags)
> > > > +{
> > > > + struct block_device *in_bdev = I_BDEV(bdev_file_inode(file_in));
> > > > + struct block_device *out_bdev = I_BDEV(bdev_file_inode(file_out));
> > > > + int comp_len = 0;
> > > > +
> > > > + if ((file_in->f_iocb_flags & IOCB_DIRECT) &&
> > > > + (file_out->f_iocb_flags & IOCB_DIRECT))
> > > > + comp_len = blkdev_copy_offload(in_bdev, pos_in, out_bdev,
> > > > + pos_out, len, NULL, NULL, GFP_KERNEL);
> > > > + if (comp_len != len)
> > > > + comp_len = generic_copy_file_range(file_in, pos_in + comp_len,
> > > > + file_out, pos_out + comp_len, len - comp_len, flags);
> > >
> > > I'm not deeply familiar with this code but this looks odd. It at least
> > > seems possible that comp_len could be -EINVAL and len 20 at which point
> > > you'd be doing len - comp_len aka 20 - 22 = -2 in generic_copy_file_range().
>
> 20 - -22 = 44 ofc
>
> >
> > comp_len should be 0 incase of error. We do agree, some function
>
> I mean, not to hammer on this point too much but just to be clear
> blk_copy_sanity_check(), which is introduced in the second patch, can
> return both -EPERM and -EINVAL and is first called in
> blkdev_copy_offload() so it's definitely possible for comp_len to be
> negative.
Acked. Will be updated in the next version.
Thank you,
Nitesh Shetty
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-27 8:40 ` [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device Anuj Gupta
2023-03-29 12:14 ` Christian Brauner
@ 2023-03-29 14:07 ` kernel test robot
2023-03-29 15:30 ` kernel test robot
2 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2023-03-29 14:07 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: oe-kbuild-all, bvanassche, hare, ming.lei, damien.lemoal,
anuj20.g, joshi.k, nitheshshetty, gost.dev, Nitesh Shetty,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
Hi Anuj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on device-mapper-dm/for-next linus/master v6.3-rc4 next-20230329]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230327084103.21601-5-anuj20.g%40samsung.com
patch subject: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
config: loongarch-randconfig-r001-20230329 (https://download.01.org/0day-ci/archive/20230329/202303292151.7DDOUCIt-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/61819d260936954ddd6688548f074e7063dcf39e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
git checkout 61819d260936954ddd6688548f074e7063dcf39e
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303292151.7DDOUCIt-lkp@intel.com/
All errors (new ones prefixed by >>):
loongarch64-linux-ld: fs/read_write.o: in function `.L633':
>> read_write.c:(.text+0x42e0): undefined reference to `I_BDEV'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
2023-03-27 8:40 ` [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device Anuj Gupta
2023-03-29 12:14 ` Christian Brauner
2023-03-29 14:07 ` kernel test robot
@ 2023-03-29 15:30 ` kernel test robot
2 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2023-03-29 15:30 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: oe-kbuild-all, bvanassche, hare, ming.lei, damien.lemoal,
anuj20.g, joshi.k, nitheshshetty, gost.dev, Nitesh Shetty,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
Hi Anuj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on device-mapper-dm/for-next linus/master v6.3-rc4 next-20230329]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230327084103.21601-5-anuj20.g%40samsung.com
patch subject: [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device.
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20230329/202303292349.ED70Fxdw-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/61819d260936954ddd6688548f074e7063dcf39e
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
git checkout 61819d260936954ddd6688548f074e7063dcf39e
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303292349.ED70Fxdw-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: vmlinux.o: in function `generic_copy_file_checks':
>> fs/read_write.c:1453: undefined reference to `I_BDEV'
vim +1453 fs/read_write.c
1398
1399 /*
1400 * Performs necessary checks before doing a file copy
1401 *
1402 * Can adjust amount of bytes to copy via @req_count argument.
1403 * Returns appropriate error code that caller should return or
1404 * zero in case the copy should be allowed.
1405 */
1406 static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
1407 struct file *file_out, loff_t pos_out,
1408 size_t *req_count, unsigned int flags)
1409 {
1410 struct inode *inode_in = file_inode(file_in);
1411 struct inode *inode_out = file_inode(file_out);
1412 uint64_t count = *req_count;
1413 loff_t size_in;
1414 int ret;
1415
1416 ret = generic_file_rw_checks(file_in, file_out);
1417 if (ret)
1418 return ret;
1419
1420 /*
1421 * We allow some filesystems to handle cross sb copy, but passing
1422 * a file of the wrong filesystem type to filesystem driver can result
1423 * in an attempt to dereference the wrong type of ->private_data, so
1424 * avoid doing that until we really have a good reason.
1425 *
1426 * nfs and cifs define several different file_system_type structures
1427 * and several different sets of file_operations, but they all end up
1428 * using the same ->copy_file_range() function pointer.
1429 */
1430 if (flags & COPY_FILE_SPLICE) {
1431 /* cross sb splice is allowed */
1432 } else if (file_out->f_op->copy_file_range) {
1433 if (file_in->f_op->copy_file_range !=
1434 file_out->f_op->copy_file_range)
1435 return -EXDEV;
1436 } else if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb) {
1437 return -EXDEV;
1438 }
1439
1440 /* Don't touch certain kinds of inodes */
1441 if (IS_IMMUTABLE(inode_out))
1442 return -EPERM;
1443
1444 if (IS_SWAPFILE(inode_in) || IS_SWAPFILE(inode_out))
1445 return -ETXTBSY;
1446
1447 /* Ensure offsets don't wrap. */
1448 if (pos_in + count < pos_in || pos_out + count < pos_out)
1449 return -EOVERFLOW;
1450
1451 /* Shorten the copy to EOF */
1452 if (S_ISBLK(inode_in->i_mode))
> 1453 size_in = bdev_nr_bytes(I_BDEV(file_in->f_mapping->host));
1454 else
1455 size_in = i_size_read(inode_in);
1456
1457 if (pos_in >= size_in)
1458 count = 0;
1459 else
1460 count = min(count, size_in - (uint64_t)pos_in);
1461
1462 ret = generic_write_check_limits(file_out, pos_out, &count);
1463 if (ret)
1464 return ret;
1465
1466 /* Don't allow overlapped copying within the same file. */
1467 if (inode_in == inode_out &&
1468 pos_out + count > pos_in &&
1469 pos_out < pos_in + count)
1470 return -EINVAL;
1471
1472 *req_count = count;
1473 return 0;
1474 }
1475
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 5/9] nvme: add copy offload support
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (3 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 4/9] fs, block: copy_file_range for def_blk_ops for direct block device Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns Anuj Gupta
` (3 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, Javier González,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
For device supporting native copy, nvme driver receives read and
write request with BLK_COPY op flags.
For read request the nvme driver populates the payload with source
information.
For write request the driver converts it to nvme copy command using the
source information in the payload and submits to the device.
current design only supports single source range.
This design is courtesy Mikulas Patocka's token based copy
trace event support for nvme_copy_cmd.
Set the device copy limits to queue limits.
Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
Signed-off-by: Javier González <javier.gonz@samsung.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
drivers/nvme/host/constants.c | 1 +
drivers/nvme/host/core.c | 106 +++++++++++++++++++++++++++++++++-
drivers/nvme/host/fc.c | 5 ++
drivers/nvme/host/nvme.h | 7 +++
drivers/nvme/host/pci.c | 27 ++++++++-
drivers/nvme/host/rdma.c | 7 +++
drivers/nvme/host/tcp.c | 16 +++++
drivers/nvme/host/trace.c | 19 ++++++
include/linux/nvme.h | 43 +++++++++++++-
9 files changed, 223 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/constants.c b/drivers/nvme/host/constants.c
index bc523ca02254..01be882b726f 100644
--- a/drivers/nvme/host/constants.c
+++ b/drivers/nvme/host/constants.c
@@ -19,6 +19,7 @@ static const char * const nvme_ops[] = {
[nvme_cmd_resv_report] = "Reservation Report",
[nvme_cmd_resv_acquire] = "Reservation Acquire",
[nvme_cmd_resv_release] = "Reservation Release",
+ [nvme_cmd_copy] = "Copy Offload",
[nvme_cmd_zone_mgmt_send] = "Zone Management Send",
[nvme_cmd_zone_mgmt_recv] = "Zone Management Receive",
[nvme_cmd_zone_append] = "Zone Management Append",
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4eb62a7dac44..e0b5381d572b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -754,6 +754,80 @@ static inline void nvme_setup_flush(struct nvme_ns *ns,
cmnd->common.nsid = cpu_to_le32(ns->head->ns_id);
}
+static inline blk_status_t nvme_setup_copy_read(struct nvme_ns *ns,
+ struct request *req)
+{
+ struct bio *bio = req->bio;
+ struct nvme_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
+
+ memcpy(token->subsys, "nvme", 4);
+ token->ns = ns;
+ token->src_sector = bio->bi_iter.bi_sector;
+ token->sectors = bio->bi_iter.bi_size >> 9;
+
+ return BLK_STS_OK;
+}
+
+static inline blk_status_t nvme_setup_copy_write(struct nvme_ns *ns,
+ struct request *req, struct nvme_command *cmnd)
+{
+ struct nvme_copy_range *range = NULL;
+ struct bio *bio = req->bio;
+ struct nvme_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
+ sector_t src_sector, dst_sector, n_sectors;
+ u64 src_lba, dst_lba, n_lba;
+ unsigned short nr_range = 1;
+ u16 control = 0;
+
+ if (unlikely(memcmp(token->subsys, "nvme", 4)))
+ return BLK_STS_NOTSUPP;
+ if (unlikely(token->ns != ns))
+ return BLK_STS_NOTSUPP;
+
+ src_sector = token->src_sector;
+ dst_sector = bio->bi_iter.bi_sector;
+ n_sectors = token->sectors;
+ if (WARN_ON(n_sectors != bio->bi_iter.bi_size >> 9))
+ return BLK_STS_NOTSUPP;
+
+ src_lba = nvme_sect_to_lba(ns, src_sector);
+ dst_lba = nvme_sect_to_lba(ns, dst_sector);
+ n_lba = nvme_sect_to_lba(ns, n_sectors);
+
+ if (WARN_ON(!n_lba))
+ return BLK_STS_NOTSUPP;
+
+ if (req->cmd_flags & REQ_FUA)
+ control |= NVME_RW_FUA;
+
+ if (req->cmd_flags & REQ_FAILFAST_DEV)
+ control |= NVME_RW_LR;
+
+ memset(cmnd, 0, sizeof(*cmnd));
+ cmnd->copy.opcode = nvme_cmd_copy;
+ cmnd->copy.nsid = cpu_to_le32(ns->head->ns_id);
+ cmnd->copy.sdlba = cpu_to_le64(dst_lba);
+
+ range = kmalloc_array(nr_range, sizeof(*range),
+ GFP_ATOMIC | __GFP_NOWARN);
+ if (!range)
+ return BLK_STS_RESOURCE;
+
+ range[0].slba = cpu_to_le64(src_lba);
+ range[0].nlb = cpu_to_le16(n_lba - 1);
+
+ cmnd->copy.nr_range = 0;
+
+ req->special_vec.bv_page = virt_to_page(range);
+ req->special_vec.bv_offset = offset_in_page(range);
+ req->special_vec.bv_len = sizeof(*range) * nr_range;
+ req->rq_flags |= RQF_SPECIAL_PAYLOAD;
+
+ cmnd->copy.control = cpu_to_le16(control);
+
+ return BLK_STS_OK;
+}
+
static blk_status_t nvme_setup_discard(struct nvme_ns *ns, struct request *req,
struct nvme_command *cmnd)
{
@@ -988,10 +1062,16 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
ret = nvme_setup_discard(ns, req, cmd);
break;
case REQ_OP_READ:
- ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_read);
+ if (unlikely(req->cmd_flags & REQ_COPY))
+ ret = nvme_setup_copy_read(ns, req);
+ else
+ ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_read);
break;
case REQ_OP_WRITE:
- ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_write);
+ if (unlikely(req->cmd_flags & REQ_COPY))
+ ret = nvme_setup_copy_write(ns, req, cmd);
+ else
+ ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_write);
break;
case REQ_OP_ZONE_APPEND:
ret = nvme_setup_rw(ns, req, cmd, nvme_cmd_zone_append);
@@ -1698,6 +1778,26 @@ static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
blk_queue_max_write_zeroes_sectors(queue, UINT_MAX);
}
+static void nvme_config_copy(struct gendisk *disk, struct nvme_ns *ns,
+ struct nvme_id_ns *id)
+{
+ struct nvme_ctrl *ctrl = ns->ctrl;
+ struct request_queue *q = disk->queue;
+
+ if (!(ctrl->oncs & NVME_CTRL_ONCS_COPY)) {
+ blk_queue_max_copy_sectors_hw(q, 0);
+ blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
+ return;
+ }
+
+ /* setting copy limits */
+ if (blk_queue_flag_test_and_set(QUEUE_FLAG_COPY, q))
+ return;
+
+ blk_queue_max_copy_sectors_hw(q,
+ nvme_lba_to_sect(ns, le16_to_cpu(id->mssrl)));
+}
+
static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
{
return uuid_equal(&a->uuid, &b->uuid) &&
@@ -1897,6 +1997,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
set_capacity_and_notify(disk, capacity);
nvme_config_discard(disk, ns);
+ nvme_config_copy(disk, ns, id);
blk_queue_max_write_zeroes_sectors(disk->queue,
ns->ctrl->max_zeroes_sectors);
}
@@ -5345,6 +5446,7 @@ static inline void _nvme_check_size(void)
BUILD_BUG_ON(sizeof(struct nvme_download_firmware) != 64);
BUILD_BUG_ON(sizeof(struct nvme_format_cmd) != 64);
BUILD_BUG_ON(sizeof(struct nvme_dsm_cmd) != 64);
+ BUILD_BUG_ON(sizeof(struct nvme_copy_command) != 64);
BUILD_BUG_ON(sizeof(struct nvme_write_zeroes_cmd) != 64);
BUILD_BUG_ON(sizeof(struct nvme_abort_cmd) != 64);
BUILD_BUG_ON(sizeof(struct nvme_get_log_page_command) != 64);
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 2ed75923507d..db2e22b4ca7f 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2807,6 +2807,11 @@ nvme_fc_queue_rq(struct blk_mq_hw_ctx *hctx,
if (ret)
return ret;
+ if (unlikely((rq->cmd_flags & REQ_COPY) &&
+ (req_op(rq) == REQ_OP_READ))) {
+ blk_mq_end_request(rq, BLK_STS_OK);
+ return BLK_STS_OK;
+ }
/*
* nvme core doesn't quite treat the rq opaquely. Commands such
* as WRITE ZEROES will return a non-zero rq payload_bytes yet
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bf46f122e9e1..257f91ee1f2d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -500,6 +500,13 @@ struct nvme_ns {
};
+struct nvme_copy_token {
+ char subsys[4];
+ struct nvme_ns *ns;
+ u64 src_sector;
+ u64 sectors;
+};
+
/* NVMe ns supports metadata actions by the controller (generate/strip) */
static inline bool nvme_ns_has_pi(struct nvme_ns *ns)
{
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b615906263f3..d30816533922 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -496,16 +496,19 @@ static inline void nvme_sq_copy_cmd(struct nvme_queue *nvmeq,
nvmeq->sq_tail = 0;
}
-static void nvme_commit_rqs(struct blk_mq_hw_ctx *hctx)
+static inline void nvme_commit_sq_db(struct nvme_queue *nvmeq)
{
- struct nvme_queue *nvmeq = hctx->driver_data;
-
spin_lock(&nvmeq->sq_lock);
if (nvmeq->sq_tail != nvmeq->last_sq_tail)
nvme_write_sq_db(nvmeq, true);
spin_unlock(&nvmeq->sq_lock);
}
+static void nvme_commit_rqs(struct blk_mq_hw_ctx *hctx)
+{
+ nvme_commit_sq_db(hctx->driver_data);
+}
+
static inline bool nvme_pci_use_sgls(struct nvme_dev *dev, struct request *req,
int nseg)
{
@@ -849,6 +852,12 @@ static blk_status_t nvme_prep_rq(struct nvme_dev *dev, struct request *req)
if (ret)
return ret;
+ if (unlikely((req->cmd_flags & REQ_COPY) &&
+ (req_op(req) == REQ_OP_READ))) {
+ blk_mq_start_request(req);
+ return BLK_STS_OK;
+ }
+
if (blk_rq_nr_phys_segments(req)) {
ret = nvme_map_data(dev, req, &iod->cmd);
if (ret)
@@ -895,6 +904,18 @@ static blk_status_t nvme_queue_rq(struct blk_mq_hw_ctx *hctx,
ret = nvme_prep_rq(dev, req);
if (unlikely(ret))
return ret;
+ if (unlikely((req->cmd_flags & REQ_COPY) &&
+ (req_op(req) == REQ_OP_READ))) {
+ blk_mq_set_request_complete(req);
+ blk_mq_end_request(req, BLK_STS_OK);
+ /* Commit the sq if copy read was the last req in the list,
+ * as copy read deoesn't update sq db
+ */
+ if (bd->last)
+ nvme_commit_sq_db(nvmeq);
+ return ret;
+ }
+
spin_lock(&nvmeq->sq_lock);
nvme_sq_copy_cmd(nvmeq, &iod->cmd);
nvme_write_sq_db(nvmeq, bd->last);
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index bbad26b82b56..a8bf2a87f42a 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -2043,6 +2043,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
nvme_start_request(rq);
+ if (unlikely((rq->cmd_flags & REQ_COPY) &&
+ (req_op(rq) == REQ_OP_READ))) {
+ blk_mq_end_request(rq, BLK_STS_OK);
+ ret = BLK_STS_OK;
+ goto unmap_qe;
+ }
+
if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
queue->pi_support &&
(c->common.opcode == nvme_cmd_write ||
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 42c0598c31f2..3c238e07b05c 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2364,6 +2364,11 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns,
if (ret)
return ret;
+ if (unlikely((rq->cmd_flags & REQ_COPY) &&
+ (req_op(rq) == REQ_OP_READ))) {
+ return BLK_STS_OK;
+ }
+
req->state = NVME_TCP_SEND_CMD_PDU;
req->status = cpu_to_le16(NVME_SC_SUCCESS);
req->offset = 0;
@@ -2432,6 +2437,17 @@ static blk_status_t nvme_tcp_queue_rq(struct blk_mq_hw_ctx *hctx,
nvme_start_request(rq);
+ if (unlikely((rq->cmd_flags & REQ_COPY) &&
+ (req_op(rq) == REQ_OP_READ))) {
+ blk_mq_set_request_complete(rq);
+ blk_mq_end_request(rq, BLK_STS_OK);
+ /* if copy read is the last req queue tcp reqs */
+ if (bd->last && nvme_tcp_queue_more(queue))
+ queue_work_on(queue->io_cpu, nvme_tcp_wq,
+ &queue->io_work);
+ return ret;
+ }
+
nvme_tcp_queue_request(req, true, bd->last);
return BLK_STS_OK;
diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c
index 1c36fcedea20..da4a7494e5a7 100644
--- a/drivers/nvme/host/trace.c
+++ b/drivers/nvme/host/trace.c
@@ -150,6 +150,23 @@ static const char *nvme_trace_read_write(struct trace_seq *p, u8 *cdw10)
return ret;
}
+static const char *nvme_trace_copy(struct trace_seq *p, u8 *cdw10)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ u64 slba = get_unaligned_le64(cdw10);
+ u8 nr_range = get_unaligned_le16(cdw10 + 8);
+ u16 control = get_unaligned_le16(cdw10 + 10);
+ u32 dsmgmt = get_unaligned_le32(cdw10 + 12);
+ u32 reftag = get_unaligned_le32(cdw10 + 16);
+
+ trace_seq_printf(p,
+ "slba=%llu, nr_range=%u, ctrl=0x%x, dsmgmt=%u, reftag=%u",
+ slba, nr_range, control, dsmgmt, reftag);
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
+
static const char *nvme_trace_dsm(struct trace_seq *p, u8 *cdw10)
{
const char *ret = trace_seq_buffer_ptr(p);
@@ -243,6 +260,8 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq *p,
return nvme_trace_zone_mgmt_send(p, cdw10);
case nvme_cmd_zone_mgmt_recv:
return nvme_trace_zone_mgmt_recv(p, cdw10);
+ case nvme_cmd_copy:
+ return nvme_trace_copy(p, cdw10);
default:
return nvme_trace_common(p, cdw10);
}
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 779507ac750b..6582b26e532c 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -337,7 +337,7 @@ struct nvme_id_ctrl {
__u8 nvscc;
__u8 nwpc;
__le16 acwu;
- __u8 rsvd534[2];
+ __le16 ocfs;
__le32 sgls;
__le32 mnan;
__u8 rsvd544[224];
@@ -365,6 +365,7 @@ enum {
NVME_CTRL_ONCS_WRITE_ZEROES = 1 << 3,
NVME_CTRL_ONCS_RESERVATIONS = 1 << 5,
NVME_CTRL_ONCS_TIMESTAMP = 1 << 6,
+ NVME_CTRL_ONCS_COPY = 1 << 8,
NVME_CTRL_VWC_PRESENT = 1 << 0,
NVME_CTRL_OACS_SEC_SUPP = 1 << 0,
NVME_CTRL_OACS_NS_MNGT_SUPP = 1 << 3,
@@ -414,7 +415,10 @@ struct nvme_id_ns {
__le16 npdg;
__le16 npda;
__le16 nows;
- __u8 rsvd74[18];
+ __le16 mssrl;
+ __le32 mcl;
+ __u8 msrc;
+ __u8 rsvd91[11];
__le32 anagrpid;
__u8 rsvd96[3];
__u8 nsattr;
@@ -796,6 +800,7 @@ enum nvme_opcode {
nvme_cmd_resv_report = 0x0e,
nvme_cmd_resv_acquire = 0x11,
nvme_cmd_resv_release = 0x15,
+ nvme_cmd_copy = 0x19,
nvme_cmd_zone_mgmt_send = 0x79,
nvme_cmd_zone_mgmt_recv = 0x7a,
nvme_cmd_zone_append = 0x7d,
@@ -819,7 +824,8 @@ enum nvme_opcode {
nvme_opcode_name(nvme_cmd_resv_release), \
nvme_opcode_name(nvme_cmd_zone_mgmt_send), \
nvme_opcode_name(nvme_cmd_zone_mgmt_recv), \
- nvme_opcode_name(nvme_cmd_zone_append))
+ nvme_opcode_name(nvme_cmd_zone_append), \
+ nvme_opcode_name(nvme_cmd_copy))
@@ -996,6 +1002,36 @@ struct nvme_dsm_range {
__le64 slba;
};
+struct nvme_copy_command {
+ __u8 opcode;
+ __u8 flags;
+ __u16 command_id;
+ __le32 nsid;
+ __u64 rsvd2;
+ __le64 metadata;
+ union nvme_data_ptr dptr;
+ __le64 sdlba;
+ __u8 nr_range;
+ __u8 rsvd12;
+ __le16 control;
+ __le16 rsvd13;
+ __le16 dspec;
+ __le32 ilbrt;
+ __le16 lbat;
+ __le16 lbatm;
+};
+
+struct nvme_copy_range {
+ __le64 rsvd0;
+ __le64 slba;
+ __le16 nlb;
+ __le16 rsvd18;
+ __le32 rsvd20;
+ __le32 eilbrt;
+ __le16 elbat;
+ __le16 elbatm;
+};
+
struct nvme_write_zeroes_cmd {
__u8 opcode;
__u8 flags;
@@ -1757,6 +1793,7 @@ struct nvme_command {
struct nvme_download_firmware dlfw;
struct nvme_format_cmd format;
struct nvme_dsm_cmd dsm;
+ struct nvme_copy_command copy;
struct nvme_write_zeroes_cmd write_zeroes;
struct nvme_zone_mgmt_send_cmd zms;
struct nvme_zone_mgmt_recv_cmd zmr;
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (4 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 5/9] nvme: add copy offload support Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 13:56 ` kernel test robot
2023-03-29 18:36 ` kernel test robot
2023-03-27 8:40 ` [PATCH v8 7/9] dm: Add support for copy offload Anuj Gupta
` (2 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
Add support for handling target command on target.
For bdev-ns we call into blkdev_issue_copy, which the block layer
completes by a offloaded copy request to backend bdev or by emulating the
request.
For file-ns we call vfs_copy_file_range to service our request.
Currently target always shows copy capability by setting
NVME_CTRL_ONCS_COPY in controller ONCS.
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
drivers/nvme/target/admin-cmd.c | 9 +++--
drivers/nvme/target/io-cmd-bdev.c | 58 +++++++++++++++++++++++++++++++
drivers/nvme/target/io-cmd-file.c | 52 +++++++++++++++++++++++++++
drivers/nvme/target/loop.c | 6 ++++
drivers/nvme/target/nvmet.h | 1 +
5 files changed, 124 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 80099df37314..978786ec6a9e 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -433,8 +433,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
id->nn = cpu_to_le32(NVMET_MAX_NAMESPACES);
id->mnan = cpu_to_le32(NVMET_MAX_NAMESPACES);
id->oncs = cpu_to_le16(NVME_CTRL_ONCS_DSM |
- NVME_CTRL_ONCS_WRITE_ZEROES);
-
+ NVME_CTRL_ONCS_WRITE_ZEROES | NVME_CTRL_ONCS_COPY);
/* XXX: don't report vwc if the underlying device is write through */
id->vwc = NVME_CTRL_VWC_PRESENT;
@@ -536,6 +535,12 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
if (req->ns->bdev)
nvmet_bdev_set_limits(req->ns->bdev, id);
+ else {
+ id->msrc = (u8)to0based(BIO_MAX_VECS - 1);
+ id->mssrl = cpu_to_le16(BIO_MAX_VECS <<
+ (PAGE_SHIFT - SECTOR_SHIFT));
+ id->mcl = cpu_to_le32(le16_to_cpu(id->mssrl));
+ }
/*
* We just provide a single LBA format that matches what the
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index c2d6cea0236b..0af273097aa4 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -46,6 +46,19 @@ void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id)
id->npda = id->npdg;
/* NOWS = Namespace Optimal Write Size */
id->nows = to0based(bdev_io_opt(bdev) / bdev_logical_block_size(bdev));
+
+ /*Copy limits*/
+ if (bdev_max_copy_sectors(bdev)) {
+ id->msrc = id->msrc;
+ id->mssrl = cpu_to_le16((bdev_max_copy_sectors(bdev) <<
+ SECTOR_SHIFT) / bdev_logical_block_size(bdev));
+ id->mcl = cpu_to_le32(id->mssrl);
+ } else {
+ id->msrc = (u8)to0based(BIO_MAX_VECS - 1);
+ id->mssrl = cpu_to_le16((BIO_MAX_VECS << PAGE_SHIFT) /
+ bdev_logical_block_size(bdev));
+ id->mcl = cpu_to_le32(id->mssrl);
+ }
}
void nvmet_bdev_ns_disable(struct nvmet_ns *ns)
@@ -184,6 +197,19 @@ static void nvmet_bio_done(struct bio *bio)
nvmet_req_bio_put(req, bio);
}
+static void nvmet_bdev_copy_end_io(void *private, int comp_len)
+{
+ struct nvmet_req *req = (struct nvmet_req *)private;
+
+ if (comp_len == req->copy_len) {
+ req->cqe->result.u32 = cpu_to_le32(1);
+ nvmet_req_complete(req, errno_to_nvme_status(req, 0));
+ } else {
+ req->cqe->result.u32 = cpu_to_le32(0);
+ nvmet_req_complete(req, blk_to_nvme_status(req, BLK_STS_IOERR));
+ }
+}
+
#ifdef CONFIG_BLK_DEV_INTEGRITY
static int nvmet_bdev_alloc_bip(struct nvmet_req *req, struct bio *bio,
struct sg_mapping_iter *miter)
@@ -450,6 +476,34 @@ static void nvmet_bdev_execute_write_zeroes(struct nvmet_req *req)
}
}
+/* At present we handle only one range entry */
+static void nvmet_bdev_execute_copy(struct nvmet_req *req)
+{
+ struct nvme_copy_range range;
+ struct nvme_command *cmnd = req->cmd;
+ int ret;
+
+
+ ret = nvmet_copy_from_sgl(req, 0, &range, sizeof(range));
+ if (ret)
+ goto out;
+
+ ret = blkdev_issue_copy(req->ns->bdev,
+ le64_to_cpu(cmnd->copy.sdlba) << req->ns->blksize_shift,
+ req->ns->bdev,
+ le64_to_cpu(range.slba) << req->ns->blksize_shift,
+ (le16_to_cpu(range.nlb) + 1) << req->ns->blksize_shift,
+ nvmet_bdev_copy_end_io, (void *)req, GFP_KERNEL);
+ if (ret) {
+ req->cqe->result.u32 = cpu_to_le32(0);
+ nvmet_req_complete(req, blk_to_nvme_status(req, BLK_STS_IOERR));
+ }
+
+ return;
+out:
+ nvmet_req_complete(req, errno_to_nvme_status(req, ret));
+}
+
u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
{
switch (req->cmd->common.opcode) {
@@ -468,6 +522,10 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
case nvme_cmd_write_zeroes:
req->execute = nvmet_bdev_execute_write_zeroes;
return 0;
+ case nvme_cmd_copy:
+ req->execute = nvmet_bdev_execute_copy;
+ return 0;
+
default:
return nvmet_report_invalid_opcode(req);
}
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 2d068439b129..69f198ecec77 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -322,6 +322,49 @@ static void nvmet_file_dsm_work(struct work_struct *w)
}
}
+static void nvmet_file_copy_work(struct work_struct *w)
+{
+ struct nvmet_req *req = container_of(w, struct nvmet_req, f.work);
+ int nr_range;
+ loff_t pos;
+ struct nvme_command *cmnd = req->cmd;
+ int ret = 0, len = 0, src, id;
+
+ nr_range = cmnd->copy.nr_range + 1;
+ pos = le64_to_cpu(req->cmd->copy.sdlba) << req->ns->blksize_shift;
+ if (unlikely(pos + req->transfer_len > req->ns->size)) {
+ nvmet_req_complete(req, errno_to_nvme_status(req, -ENOSPC));
+ return;
+ }
+
+ for (id = 0 ; id < nr_range; id++) {
+ struct nvme_copy_range range;
+
+ ret = nvmet_copy_from_sgl(req, id * sizeof(range), &range,
+ sizeof(range));
+ if (ret)
+ goto out;
+
+ len = (le16_to_cpu(range.nlb) + 1) << (req->ns->blksize_shift);
+ src = (le64_to_cpu(range.slba) << (req->ns->blksize_shift));
+ ret = vfs_copy_file_range(req->ns->file, src, req->ns->file,
+ pos, len, 0);
+out:
+ if (ret != len) {
+ pos += ret;
+ req->cqe->result.u32 = cpu_to_le32(id);
+ nvmet_req_complete(req, ret < 0 ?
+ errno_to_nvme_status(req, ret) :
+ errno_to_nvme_status(req, -EIO));
+ return;
+
+ } else
+ pos += len;
+ }
+
+ nvmet_req_complete(req, 0);
+
+}
static void nvmet_file_execute_dsm(struct nvmet_req *req)
{
if (!nvmet_check_data_len_lte(req, nvmet_dsm_len(req)))
@@ -330,6 +373,12 @@ static void nvmet_file_execute_dsm(struct nvmet_req *req)
queue_work(nvmet_wq, &req->f.work);
}
+static void nvmet_file_execute_copy(struct nvmet_req *req)
+{
+ INIT_WORK(&req->f.work, nvmet_file_copy_work);
+ queue_work(nvmet_wq, &req->f.work);
+}
+
static void nvmet_file_write_zeroes_work(struct work_struct *w)
{
struct nvmet_req *req = container_of(w, struct nvmet_req, f.work);
@@ -376,6 +425,9 @@ u16 nvmet_file_parse_io_cmd(struct nvmet_req *req)
case nvme_cmd_write_zeroes:
req->execute = nvmet_file_execute_write_zeroes;
return 0;
+ case nvme_cmd_copy:
+ req->execute = nvmet_file_execute_copy;
+ return 0;
default:
return nvmet_report_invalid_opcode(req);
}
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index f2d24b2d992f..d18ed8067a15 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -146,6 +146,12 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
return ret;
nvme_start_request(req);
+ if (unlikely((req->cmd_flags & REQ_COPY) &&
+ (req_op(req) == REQ_OP_READ))) {
+ blk_mq_set_request_complete(req);
+ blk_mq_end_request(req, BLK_STS_OK);
+ return BLK_STS_OK;
+ }
iod->cmd.common.flags |= NVME_CMD_SGL_METABUF;
iod->req.port = queue->ctrl->port;
if (!nvmet_req_init(&iod->req, &queue->nvme_cq,
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 89bedfcd974c..69ed4c8469e5 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -393,6 +393,7 @@ struct nvmet_req {
struct device *p2p_client;
u16 error_loc;
u64 error_slba;
+ size_t copy_len;
};
#define NVMET_MAX_MPOOL_BVEC 16
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns
2023-03-27 8:40 ` [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns Anuj Gupta
@ 2023-03-29 13:56 ` kernel test robot
2023-03-29 18:36 ` kernel test robot
1 sibling, 0 replies; 27+ messages in thread
From: kernel test robot @ 2023-03-29 13:56 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: oe-kbuild-all, bvanassche, hare, ming.lei, damien.lemoal,
anuj20.g, joshi.k, nitheshshetty, gost.dev, Nitesh Shetty,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
Hi Anuj,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.3-rc4 next-20230329]
[cannot apply to device-mapper-dm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230327084103.21601-7-anuj20.g%40samsung.com
patch subject: [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns
config: arm64-randconfig-s041-20230329 (https://download.01.org/0day-ci/archive/20230329/202303292148.Pbx4mDpS-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/f846a8ac40882d9d42532e9e2b43560650ef8510
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
git checkout f846a8ac40882d9d42532e9e2b43560650ef8510
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/nvme/target/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303292148.Pbx4mDpS-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int [usertype] val @@ got restricted __le16 [usertype] mssrl @@
drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: expected unsigned int [usertype] val
drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: got restricted __le16 [usertype] mssrl
>> drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: sparse: cast from restricted __le16
>> drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: sparse: cast from restricted __le16
>> drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: sparse: cast from restricted __le16
>> drivers/nvme/target/io-cmd-bdev.c:55:27: sparse: sparse: cast from restricted __le16
drivers/nvme/target/io-cmd-bdev.c:57:29: sparse: sparse: cast from restricted __le16
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int [usertype] val @@ got restricted __le16 [usertype] mssrl @@
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: expected unsigned int [usertype] val
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: got restricted __le16 [usertype] mssrl
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: sparse: cast from restricted __le16
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: sparse: cast from restricted __le16
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: sparse: cast from restricted __le16
drivers/nvme/target/io-cmd-bdev.c:60:27: sparse: sparse: cast from restricted __le16
vim +55 drivers/nvme/target/io-cmd-bdev.c
12
13 void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id)
14 {
15 /* Logical blocks per physical block, 0's based. */
16 const __le16 lpp0b = to0based(bdev_physical_block_size(bdev) /
17 bdev_logical_block_size(bdev));
18
19 /*
20 * For NVMe 1.2 and later, bit 1 indicates that the fields NAWUN,
21 * NAWUPF, and NACWU are defined for this namespace and should be
22 * used by the host for this namespace instead of the AWUN, AWUPF,
23 * and ACWU fields in the Identify Controller data structure. If
24 * any of these fields are zero that means that the corresponding
25 * field from the identify controller data structure should be used.
26 */
27 id->nsfeat |= 1 << 1;
28 id->nawun = lpp0b;
29 id->nawupf = lpp0b;
30 id->nacwu = lpp0b;
31
32 /*
33 * Bit 4 indicates that the fields NPWG, NPWA, NPDG, NPDA, and
34 * NOWS are defined for this namespace and should be used by
35 * the host for I/O optimization.
36 */
37 id->nsfeat |= 1 << 4;
38 /* NPWG = Namespace Preferred Write Granularity. 0's based */
39 id->npwg = lpp0b;
40 /* NPWA = Namespace Preferred Write Alignment. 0's based */
41 id->npwa = id->npwg;
42 /* NPDG = Namespace Preferred Deallocate Granularity. 0's based */
43 id->npdg = to0based(bdev_discard_granularity(bdev) /
44 bdev_logical_block_size(bdev));
45 /* NPDG = Namespace Preferred Deallocate Alignment */
46 id->npda = id->npdg;
47 /* NOWS = Namespace Optimal Write Size */
48 id->nows = to0based(bdev_io_opt(bdev) / bdev_logical_block_size(bdev));
49
50 /*Copy limits*/
51 if (bdev_max_copy_sectors(bdev)) {
52 id->msrc = id->msrc;
53 id->mssrl = cpu_to_le16((bdev_max_copy_sectors(bdev) <<
54 SECTOR_SHIFT) / bdev_logical_block_size(bdev));
> 55 id->mcl = cpu_to_le32(id->mssrl);
56 } else {
57 id->msrc = (u8)to0based(BIO_MAX_VECS - 1);
58 id->mssrl = cpu_to_le16((BIO_MAX_VECS << PAGE_SHIFT) /
59 bdev_logical_block_size(bdev));
60 id->mcl = cpu_to_le32(id->mssrl);
61 }
62 }
63
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns
2023-03-27 8:40 ` [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns Anuj Gupta
2023-03-29 13:56 ` kernel test robot
@ 2023-03-29 18:36 ` kernel test robot
1 sibling, 0 replies; 27+ messages in thread
From: kernel test robot @ 2023-03-29 18:36 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: oe-kbuild-all, bvanassche, hare, ming.lei, damien.lemoal,
anuj20.g, joshi.k, nitheshshetty, gost.dev, Nitesh Shetty,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
Hi Anuj,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.3-rc4 next-20230329]
[cannot apply to device-mapper-dm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230327084103.21601-7-anuj20.g%40samsung.com
patch subject: [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns
config: arm64-randconfig-s041-20230329 (https://download.01.org/0day-ci/archive/20230330/202303300238.vmt9ne37-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/f846a8ac40882d9d42532e9e2b43560650ef8510
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anuj-Gupta/block-Add-copy-offload-support-infrastructure/20230329-162018
git checkout f846a8ac40882d9d42532e9e2b43560650ef8510
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/nvme/target/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303300238.vmt9ne37-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/nvme/target/admin-cmd.c:539:29: sparse: sparse: cast from restricted __le16
vim +539 drivers/nvme/target/admin-cmd.c
490
491 static void nvmet_execute_identify_ns(struct nvmet_req *req)
492 {
493 struct nvme_id_ns *id;
494 u16 status;
495
496 if (le32_to_cpu(req->cmd->identify.nsid) == NVME_NSID_ALL) {
497 req->error_loc = offsetof(struct nvme_identify, nsid);
498 status = NVME_SC_INVALID_NS | NVME_SC_DNR;
499 goto out;
500 }
501
502 id = kzalloc(sizeof(*id), GFP_KERNEL);
503 if (!id) {
504 status = NVME_SC_INTERNAL;
505 goto out;
506 }
507
508 /* return an all zeroed buffer if we can't find an active namespace */
509 status = nvmet_req_find_ns(req);
510 if (status) {
511 status = 0;
512 goto done;
513 }
514
515 if (nvmet_ns_revalidate(req->ns)) {
516 mutex_lock(&req->ns->subsys->lock);
517 nvmet_ns_changed(req->ns->subsys, req->ns->nsid);
518 mutex_unlock(&req->ns->subsys->lock);
519 }
520
521 /*
522 * nuse = ncap = nsze isn't always true, but we have no way to find
523 * that out from the underlying device.
524 */
525 id->ncap = id->nsze =
526 cpu_to_le64(req->ns->size >> req->ns->blksize_shift);
527 switch (req->port->ana_state[req->ns->anagrpid]) {
528 case NVME_ANA_INACCESSIBLE:
529 case NVME_ANA_PERSISTENT_LOSS:
530 break;
531 default:
532 id->nuse = id->nsze;
533 break;
534 }
535
536 if (req->ns->bdev)
537 nvmet_bdev_set_limits(req->ns->bdev, id);
538 else {
> 539 id->msrc = (u8)to0based(BIO_MAX_VECS - 1);
540 id->mssrl = cpu_to_le16(BIO_MAX_VECS <<
541 (PAGE_SHIFT - SECTOR_SHIFT));
542 id->mcl = cpu_to_le32(le16_to_cpu(id->mssrl));
543 }
544
545 /*
546 * We just provide a single LBA format that matches what the
547 * underlying device reports.
548 */
549 id->nlbaf = 0;
550 id->flbas = 0;
551
552 /*
553 * Our namespace might always be shared. Not just with other
554 * controllers, but also with any other user of the block device.
555 */
556 id->nmic = NVME_NS_NMIC_SHARED;
557 id->anagrpid = cpu_to_le32(req->ns->anagrpid);
558
559 memcpy(&id->nguid, &req->ns->nguid, sizeof(id->nguid));
560
561 id->lbaf[0].ds = req->ns->blksize_shift;
562
563 if (req->sq->ctrl->pi_support && nvmet_ns_has_pi(req->ns)) {
564 id->dpc = NVME_NS_DPC_PI_FIRST | NVME_NS_DPC_PI_LAST |
565 NVME_NS_DPC_PI_TYPE1 | NVME_NS_DPC_PI_TYPE2 |
566 NVME_NS_DPC_PI_TYPE3;
567 id->mc = NVME_MC_EXTENDED_LBA;
568 id->dps = req->ns->pi_type;
569 id->flbas = NVME_NS_FLBAS_META_EXT;
570 id->lbaf[0].ms = cpu_to_le16(req->ns->metadata_size);
571 }
572
573 if (req->ns->readonly)
574 id->nsattr |= NVME_NS_ATTR_RO;
575 done:
576 if (!status)
577 status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
578
579 kfree(id);
580 out:
581 nvmet_req_complete(req, status);
582 }
583
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 7/9] dm: Add support for copy offload.
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (5 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 6/9] nvmet: add copy command support for bdev and file ns Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 8:59 ` Damien Le Moal
2023-03-27 8:40 ` [PATCH v8 8/9] dm: Enable copy offload for dm-linear target Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 9/9] null_blk: add support for copy offload Anuj Gupta
8 siblings, 1 reply; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
Before enabling copy for dm target, check if underlying devices and
dm target support copy. Avoid split happening inside dm target.
Fail early if the request needs split, currently splitting copy
request is not supported.
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
---
drivers/md/dm-table.c | 42 +++++++++++++++++++++++++++++++++++
drivers/md/dm.c | 7 ++++++
include/linux/device-mapper.h | 5 +++++
3 files changed, 54 insertions(+)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 7899f5fb4c13..45e894b9e3be 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1863,6 +1863,39 @@ static bool dm_table_supports_nowait(struct dm_table *t)
return true;
}
+static int device_not_copy_capable(struct dm_target *ti, struct dm_dev *dev,
+ sector_t start, sector_t len, void *data)
+{
+ struct request_queue *q = bdev_get_queue(dev->bdev);
+
+ return !blk_queue_copy(q);
+}
+
+static bool dm_table_supports_copy(struct dm_table *t)
+{
+ struct dm_target *ti;
+ unsigned int i;
+
+ for (i = 0; i < t->num_targets; i++) {
+ ti = dm_table_get_target(t, i);
+
+ if (!ti->copy_offload_supported)
+ return false;
+
+ /*
+ * target provides copy support (as implied by setting
+ * 'copy_offload_supported')
+ * and it relies on _all_ data devices having copy support.
+ */
+ if (!ti->type->iterate_devices ||
+ ti->type->iterate_devices(ti,
+ device_not_copy_capable, NULL))
+ return false;
+ }
+
+ return true;
+}
+
static int device_not_discard_capable(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
@@ -1945,6 +1978,15 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
q->limits.discard_misaligned = 0;
}
+ if (!dm_table_supports_copy(t)) {
+ blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
+ /* Must also clear copy limits... */
+ q->limits.max_copy_sectors = 0;
+ q->limits.max_copy_sectors_hw = 0;
+ } else {
+ blk_queue_flag_set(QUEUE_FLAG_COPY, q);
+ }
+
if (!dm_table_supports_secure_erase(t))
q->limits.max_secure_erase_sectors = 0;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2d0f934ba6e6..08ec51000af8 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1693,6 +1693,13 @@ static blk_status_t __split_and_process_bio(struct clone_info *ci)
if (unlikely(ci->is_abnormal_io))
return __process_abnormal_io(ci, ti);
+ if ((unlikely(op_is_copy(ci->bio->bi_opf)) &&
+ max_io_len(ti, ci->sector) < ci->sector_count)) {
+ DMERR("Error, IO size(%u) > max target size(%llu)\n",
+ ci->sector_count, max_io_len(ti, ci->sector));
+ return BLK_STS_IOERR;
+ }
+
/*
* Only support bio polling for normal IO, and the target io is
* exactly inside the dm_io instance (verified in dm_poll_dm_io)
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 7975483816e4..44969a20295e 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -380,6 +380,11 @@ struct dm_target {
* bio_set_dev(). NOTE: ideally a target should _not_ need this.
*/
bool needs_bio_set_dev:1;
+
+ /*
+ * copy offload is supported
+ */
+ bool copy_offload_supported:1;
};
void *dm_per_bio_data(struct bio *bio, size_t data_size);
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 7/9] dm: Add support for copy offload.
2023-03-27 8:40 ` [PATCH v8 7/9] dm: Add support for copy offload Anuj Gupta
@ 2023-03-29 8:59 ` Damien Le Moal
2023-03-29 12:12 ` Nitesh Shetty
0 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2023-03-29 8:59 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, joshi.k, nitheshshetty, gost.dev,
Nitesh Shetty, linux-block, linux-kernel, linux-nvme,
linux-fsdevel
On 3/27/23 17:40, Anuj Gupta wrote:
> From: Nitesh Shetty <nj.shetty@samsung.com>
>
Drop the period at the end of the patch title.
> Before enabling copy for dm target, check if underlying devices and
> dm target support copy. Avoid split happening inside dm target.
> Fail early if the request needs split, currently splitting copy
> request is not supported.
>
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> ---
> drivers/md/dm-table.c | 42 +++++++++++++++++++++++++++++++++++
> drivers/md/dm.c | 7 ++++++
> include/linux/device-mapper.h | 5 +++++
> 3 files changed, 54 insertions(+)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 7899f5fb4c13..45e894b9e3be 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1863,6 +1863,39 @@ static bool dm_table_supports_nowait(struct dm_table *t)
> return true;
> }
>
> +static int device_not_copy_capable(struct dm_target *ti, struct dm_dev *dev,
> + sector_t start, sector_t len, void *data)
> +{
> + struct request_queue *q = bdev_get_queue(dev->bdev);
> +
> + return !blk_queue_copy(q);
> +}
> +
> +static bool dm_table_supports_copy(struct dm_table *t)
> +{
> + struct dm_target *ti;
> + unsigned int i;
> +
> + for (i = 0; i < t->num_targets; i++) {
> + ti = dm_table_get_target(t, i);
> +
> + if (!ti->copy_offload_supported)
> + return false;
> +
> + /*
> + * target provides copy support (as implied by setting
> + * 'copy_offload_supported')
> + * and it relies on _all_ data devices having copy support.
> + */
> + if (!ti->type->iterate_devices ||
> + ti->type->iterate_devices(ti,
> + device_not_copy_capable, NULL))
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int device_not_discard_capable(struct dm_target *ti, struct dm_dev *dev,
> sector_t start, sector_t len, void *data)
> {
> @@ -1945,6 +1978,15 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> q->limits.discard_misaligned = 0;
> }
>
> + if (!dm_table_supports_copy(t)) {
> + blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
> + /* Must also clear copy limits... */
Not a useful comment. The code is clear.
> + q->limits.max_copy_sectors = 0;
> + q->limits.max_copy_sectors_hw = 0;
> + } else {
> + blk_queue_flag_set(QUEUE_FLAG_COPY, q);
> + }
> +
> if (!dm_table_supports_secure_erase(t))
> q->limits.max_secure_erase_sectors = 0;
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 2d0f934ba6e6..08ec51000af8 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1693,6 +1693,13 @@ static blk_status_t __split_and_process_bio(struct clone_info *ci)
> if (unlikely(ci->is_abnormal_io))
> return __process_abnormal_io(ci, ti);
>
> + if ((unlikely(op_is_copy(ci->bio->bi_opf)) &&
> + max_io_len(ti, ci->sector) < ci->sector_count)) {
> + DMERR("Error, IO size(%u) > max target size(%llu)\n",
> + ci->sector_count, max_io_len(ti, ci->sector));
> + return BLK_STS_IOERR;
> + }
> +
> /*
> * Only support bio polling for normal IO, and the target io is
> * exactly inside the dm_io instance (verified in dm_poll_dm_io)
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 7975483816e4..44969a20295e 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -380,6 +380,11 @@ struct dm_target {
> * bio_set_dev(). NOTE: ideally a target should _not_ need this.
> */
> bool needs_bio_set_dev:1;
> +
> + /*
> + * copy offload is supported
> + */
> + bool copy_offload_supported:1;
> };
>
> void *dm_per_bio_data(struct bio *bio, size_t data_size);
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 7/9] dm: Add support for copy offload.
2023-03-29 8:59 ` Damien Le Moal
@ 2023-03-29 12:12 ` Nitesh Shetty
0 siblings, 0 replies; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-29 12:12 UTC (permalink / raw)
To: Damien Le Moal
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner, bvanassche,
hare, ming.lei, joshi.k, nitheshshetty, gost.dev, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 4068 bytes --]
On Wed, Mar 29, 2023 at 05:59:49PM +0900, Damien Le Moal wrote:
> On 3/27/23 17:40, Anuj Gupta wrote:
> > From: Nitesh Shetty <nj.shetty@samsung.com>
> >
>
> Drop the period at the end of the patch title.
Acked
>
> > Before enabling copy for dm target, check if underlying devices and
> > dm target support copy. Avoid split happening inside dm target.
> > Fail early if the request needs split, currently splitting copy
> > request is not supported.
> >
> > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > ---
> > drivers/md/dm-table.c | 42 +++++++++++++++++++++++++++++++++++
> > drivers/md/dm.c | 7 ++++++
> > include/linux/device-mapper.h | 5 +++++
> > 3 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > index 7899f5fb4c13..45e894b9e3be 100644
> > --- a/drivers/md/dm-table.c
> > +++ b/drivers/md/dm-table.c
> > @@ -1863,6 +1863,39 @@ static bool dm_table_supports_nowait(struct dm_table *t)
> > return true;
> > }
> >
> > +static int device_not_copy_capable(struct dm_target *ti, struct dm_dev *dev,
> > + sector_t start, sector_t len, void *data)
> > +{
> > + struct request_queue *q = bdev_get_queue(dev->bdev);
> > +
> > + return !blk_queue_copy(q);
> > +}
> > +
> > +static bool dm_table_supports_copy(struct dm_table *t)
> > +{
> > + struct dm_target *ti;
> > + unsigned int i;
> > +
> > + for (i = 0; i < t->num_targets; i++) {
> > + ti = dm_table_get_target(t, i);
> > +
> > + if (!ti->copy_offload_supported)
> > + return false;
> > +
> > + /*
> > + * target provides copy support (as implied by setting
> > + * 'copy_offload_supported')
> > + * and it relies on _all_ data devices having copy support.
> > + */
> > + if (!ti->type->iterate_devices ||
> > + ti->type->iterate_devices(ti,
> > + device_not_copy_capable, NULL))
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > static int device_not_discard_capable(struct dm_target *ti, struct dm_dev *dev,
> > sector_t start, sector_t len, void *data)
> > {
> > @@ -1945,6 +1978,15 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> > q->limits.discard_misaligned = 0;
> > }
> >
> > + if (!dm_table_supports_copy(t)) {
> > + blk_queue_flag_clear(QUEUE_FLAG_COPY, q);
> > + /* Must also clear copy limits... */
>
> Not a useful comment. The code is clear.
Acked
>
> > + q->limits.max_copy_sectors = 0;
> > + q->limits.max_copy_sectors_hw = 0;
> > + } else {
> > + blk_queue_flag_set(QUEUE_FLAG_COPY, q);
> > + }
> > +
> > if (!dm_table_supports_secure_erase(t))
> > q->limits.max_secure_erase_sectors = 0;
> >
> > diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> > index 2d0f934ba6e6..08ec51000af8 100644
> > --- a/drivers/md/dm.c
> > +++ b/drivers/md/dm.c
> > @@ -1693,6 +1693,13 @@ static blk_status_t __split_and_process_bio(struct clone_info *ci)
> > if (unlikely(ci->is_abnormal_io))
> > return __process_abnormal_io(ci, ti);
> >
> > + if ((unlikely(op_is_copy(ci->bio->bi_opf)) &&
> > + max_io_len(ti, ci->sector) < ci->sector_count)) {
> > + DMERR("Error, IO size(%u) > max target size(%llu)\n",
> > + ci->sector_count, max_io_len(ti, ci->sector));
> > + return BLK_STS_IOERR;
> > + }
> > +
> > /*
> > * Only support bio polling for normal IO, and the target io is
> > * exactly inside the dm_io instance (verified in dm_poll_dm_io)
> > diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> > index 7975483816e4..44969a20295e 100644
> > --- a/include/linux/device-mapper.h
> > +++ b/include/linux/device-mapper.h
> > @@ -380,6 +380,11 @@ struct dm_target {
> > * bio_set_dev(). NOTE: ideally a target should _not_ need this.
> > */
> > bool needs_bio_set_dev:1;
> > +
> > + /*
> > + * copy offload is supported
> > + */
> > + bool copy_offload_supported:1;
> > };
> >
> > void *dm_per_bio_data(struct bio *bio, size_t data_size);
>
> --
> Damien Le Moal
> Western Digital Research
>
>
Thank you,
Nitesh Shetty
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 8/9] dm: Enable copy offload for dm-linear target
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (6 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 7/9] dm: Add support for copy offload Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-27 8:40 ` [PATCH v8 9/9] null_blk: add support for copy offload Anuj Gupta
8 siblings, 0 replies; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, linux-block, linux-kernel,
linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
Setting copy_offload_supported flag to enable offload.
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
---
drivers/md/dm-linear.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 3e622dcc9dbd..785a6822d06c 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -62,6 +62,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
ti->num_discard_bios = 1;
ti->num_secure_erase_bios = 1;
ti->num_write_zeroes_bios = 1;
+ ti->copy_offload_supported = 1;
ti->private = lc;
return 0;
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v8 9/9] null_blk: add support for copy offload
2023-03-27 8:40 ` [PATCH v8 0/9] Implement copy offload support Anuj Gupta
` (7 preceding siblings ...)
2023-03-27 8:40 ` [PATCH v8 8/9] dm: Enable copy offload for dm-linear target Anuj Gupta
@ 2023-03-27 8:40 ` Anuj Gupta
2023-03-29 9:04 ` Damien Le Moal
8 siblings, 1 reply; 27+ messages in thread
From: Anuj Gupta @ 2023-03-27 8:40 UTC (permalink / raw)
To: Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel, Keith Busch,
Christoph Hellwig, Sagi Grimberg, James Smart, Chaitanya Kulkarni,
Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, damien.lemoal, anuj20.g, joshi.k,
nitheshshetty, gost.dev, Nitesh Shetty, Vincent Fu, linux-block,
linux-kernel, linux-nvme, linux-fsdevel
From: Nitesh Shetty <nj.shetty@samsung.com>
Implementaion is based on existing read and write infrastructure.
Suggested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
drivers/block/null_blk/main.c | 94 +++++++++++++++++++++++++++++++
drivers/block/null_blk/null_blk.h | 7 +++
2 files changed, 101 insertions(+)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 9e6b032c8ecc..84c5fbcd67a5 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1257,6 +1257,81 @@ static int null_transfer(struct nullb *nullb, struct page *page,
return err;
}
+static inline int nullb_setup_copy_read(struct nullb *nullb,
+ struct bio *bio)
+{
+ struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
+
+ memcpy(token->subsys, "nullb", 5);
+ token->sector_in = bio->bi_iter.bi_sector;
+ token->nullb = nullb;
+ token->sectors = bio->bi_iter.bi_size >> SECTOR_SHIFT;
+
+ return 0;
+}
+
+static inline int nullb_setup_copy_write(struct nullb *nullb,
+ struct bio *bio, bool is_fua)
+{
+ struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
+ sector_t sector_in, sector_out;
+ void *in, *out;
+ size_t rem, temp;
+ unsigned long offset_in, offset_out;
+ struct nullb_page *t_page_in, *t_page_out;
+ int ret = -EIO;
+
+ if (unlikely(memcmp(token->subsys, "nullb", 5)))
+ return -EOPNOTSUPP;
+ if (unlikely(token->nullb != nullb))
+ return -EOPNOTSUPP;
+ if (WARN_ON(token->sectors != bio->bi_iter.bi_size >> SECTOR_SHIFT))
+ return -EOPNOTSUPP;
+
+ sector_in = token->sector_in;
+ sector_out = bio->bi_iter.bi_sector;
+ rem = token->sectors << SECTOR_SHIFT;
+
+ spin_lock_irq(&nullb->lock);
+ while (rem > 0) {
+ temp = min_t(size_t, nullb->dev->blocksize, rem);
+ offset_in = (sector_in & SECTOR_MASK) << SECTOR_SHIFT;
+ offset_out = (sector_out & SECTOR_MASK) << SECTOR_SHIFT;
+
+ if (null_cache_active(nullb) && !is_fua)
+ null_make_cache_space(nullb, PAGE_SIZE);
+
+ t_page_in = null_lookup_page(nullb, sector_in, false,
+ !null_cache_active(nullb));
+ if (!t_page_in)
+ goto err;
+ t_page_out = null_insert_page(nullb, sector_out,
+ !null_cache_active(nullb) || is_fua);
+ if (!t_page_out)
+ goto err;
+
+ in = kmap_local_page(t_page_in->page);
+ out = kmap_local_page(t_page_out->page);
+
+ memcpy(out + offset_out, in + offset_in, temp);
+ kunmap_local(out);
+ kunmap_local(in);
+ __set_bit(sector_out & SECTOR_MASK, t_page_out->bitmap);
+
+ if (is_fua)
+ null_free_sector(nullb, sector_out, true);
+
+ rem -= temp;
+ sector_in += temp >> SECTOR_SHIFT;
+ sector_out += temp >> SECTOR_SHIFT;
+ }
+
+ ret = 0;
+err:
+ spin_unlock_irq(&nullb->lock);
+ return ret;
+}
+
static int null_handle_rq(struct nullb_cmd *cmd)
{
struct request *rq = cmd->rq;
@@ -1267,6 +1342,14 @@ static int null_handle_rq(struct nullb_cmd *cmd)
struct req_iterator iter;
struct bio_vec bvec;
+ if (rq->cmd_flags & REQ_COPY) {
+ if (op_is_write(req_op(rq)))
+ return nullb_setup_copy_write(nullb, rq->bio,
+ rq->cmd_flags & REQ_FUA);
+ else
+ return nullb_setup_copy_read(nullb, rq->bio);
+ }
+
spin_lock_irq(&nullb->lock);
rq_for_each_segment(bvec, rq, iter) {
len = bvec.bv_len;
@@ -1294,6 +1377,14 @@ static int null_handle_bio(struct nullb_cmd *cmd)
struct bio_vec bvec;
struct bvec_iter iter;
+ if (bio->bi_opf & REQ_COPY) {
+ if (op_is_write(bio_op(bio)))
+ return nullb_setup_copy_write(nullb, bio,
+ bio->bi_opf & REQ_FUA);
+ else
+ return nullb_setup_copy_read(nullb, bio);
+ }
+
spin_lock_irq(&nullb->lock);
bio_for_each_segment(bvec, bio, iter) {
len = bvec.bv_len;
@@ -2146,6 +2237,9 @@ static int null_add_dev(struct nullb_device *dev)
list_add_tail(&nullb->list, &nullb_list);
mutex_unlock(&lock);
+ blk_queue_max_copy_sectors_hw(nullb->disk->queue, 1024);
+ blk_queue_flag_set(QUEUE_FLAG_COPY, nullb->disk->queue);
+
pr_info("disk %s created\n", nullb->disk_name);
return 0;
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index eb5972c50be8..94e524e7306a 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -67,6 +67,13 @@ enum {
NULL_Q_MQ = 2,
};
+struct nullb_copy_token {
+ char subsys[5];
+ struct nullb *nullb;
+ u64 sector_in;
+ u64 sectors;
+};
+
struct nullb_device {
struct nullb *nullb;
struct config_item item;
--
2.35.1.500.gb896f729e2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v8 9/9] null_blk: add support for copy offload
2023-03-27 8:40 ` [PATCH v8 9/9] null_blk: add support for copy offload Anuj Gupta
@ 2023-03-29 9:04 ` Damien Le Moal
2023-03-29 12:22 ` Nitesh Shetty
0 siblings, 1 reply; 27+ messages in thread
From: Damien Le Moal @ 2023-03-29 9:04 UTC (permalink / raw)
To: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner
Cc: bvanassche, hare, ming.lei, joshi.k, nitheshshetty, gost.dev,
Nitesh Shetty, Vincent Fu, linux-block, linux-kernel, linux-nvme,
linux-fsdevel
On 3/27/23 17:40, Anuj Gupta wrote:
> From: Nitesh Shetty <nj.shetty@samsung.com>
>
> Implementaion is based on existing read and write infrastructure.
>
> Suggested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
> ---
> drivers/block/null_blk/main.c | 94 +++++++++++++++++++++++++++++++
> drivers/block/null_blk/null_blk.h | 7 +++
> 2 files changed, 101 insertions(+)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 9e6b032c8ecc..84c5fbcd67a5 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1257,6 +1257,81 @@ static int null_transfer(struct nullb *nullb, struct page *page,
> return err;
> }
>
> +static inline int nullb_setup_copy_read(struct nullb *nullb,
> + struct bio *bio)
> +{
> + struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
> +
> + memcpy(token->subsys, "nullb", 5);
> + token->sector_in = bio->bi_iter.bi_sector;
> + token->nullb = nullb;
> + token->sectors = bio->bi_iter.bi_size >> SECTOR_SHIFT;
> +
> + return 0;
> +}
> +
> +static inline int nullb_setup_copy_write(struct nullb *nullb,
> + struct bio *bio, bool is_fua)
> +{
> + struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
> + sector_t sector_in, sector_out;
> + void *in, *out;
> + size_t rem, temp;
> + unsigned long offset_in, offset_out;
> + struct nullb_page *t_page_in, *t_page_out;
> + int ret = -EIO;
> +
> + if (unlikely(memcmp(token->subsys, "nullb", 5)))
> + return -EOPNOTSUPP;
> + if (unlikely(token->nullb != nullb))
> + return -EOPNOTSUPP;
> + if (WARN_ON(token->sectors != bio->bi_iter.bi_size >> SECTOR_SHIFT))
> + return -EOPNOTSUPP;
EOPNOTSUPP is strange. These are EINVAL, no ?.
> +
> + sector_in = token->sector_in;
> + sector_out = bio->bi_iter.bi_sector;
> + rem = token->sectors << SECTOR_SHIFT;
> +
> + spin_lock_irq(&nullb->lock);
> + while (rem > 0) {
> + temp = min_t(size_t, nullb->dev->blocksize, rem);
> + offset_in = (sector_in & SECTOR_MASK) << SECTOR_SHIFT;
> + offset_out = (sector_out & SECTOR_MASK) << SECTOR_SHIFT;
> +
> + if (null_cache_active(nullb) && !is_fua)
> + null_make_cache_space(nullb, PAGE_SIZE);
> +
> + t_page_in = null_lookup_page(nullb, sector_in, false,
> + !null_cache_active(nullb));
> + if (!t_page_in)
> + goto err;
> + t_page_out = null_insert_page(nullb, sector_out,
> + !null_cache_active(nullb) || is_fua);
> + if (!t_page_out)
> + goto err;
> +
> + in = kmap_local_page(t_page_in->page);
> + out = kmap_local_page(t_page_out->page);
> +
> + memcpy(out + offset_out, in + offset_in, temp);
> + kunmap_local(out);
> + kunmap_local(in);
> + __set_bit(sector_out & SECTOR_MASK, t_page_out->bitmap);
> +
> + if (is_fua)
> + null_free_sector(nullb, sector_out, true);
> +
> + rem -= temp;
> + sector_in += temp >> SECTOR_SHIFT;
> + sector_out += temp >> SECTOR_SHIFT;
> + }
> +
> + ret = 0;
> +err:
> + spin_unlock_irq(&nullb->lock);
> + return ret;
> +}
> +
> static int null_handle_rq(struct nullb_cmd *cmd)
> {
> struct request *rq = cmd->rq;
> @@ -1267,6 +1342,14 @@ static int null_handle_rq(struct nullb_cmd *cmd)
> struct req_iterator iter;
> struct bio_vec bvec;
>
> + if (rq->cmd_flags & REQ_COPY) {
> + if (op_is_write(req_op(rq)))
> + return nullb_setup_copy_write(nullb, rq->bio,
> + rq->cmd_flags & REQ_FUA);
> + else
No need for this else.
> + return nullb_setup_copy_read(nullb, rq->bio);
> + }
> +
> spin_lock_irq(&nullb->lock);
> rq_for_each_segment(bvec, rq, iter) {
> len = bvec.bv_len;
> @@ -1294,6 +1377,14 @@ static int null_handle_bio(struct nullb_cmd *cmd)
> struct bio_vec bvec;
> struct bvec_iter iter;
>
> + if (bio->bi_opf & REQ_COPY) {
> + if (op_is_write(bio_op(bio)))
> + return nullb_setup_copy_write(nullb, bio,
> + bio->bi_opf & REQ_FUA);
> + else
No need for this else.
> + return nullb_setup_copy_read(nullb, bio);
> + }
> +
> spin_lock_irq(&nullb->lock);
> bio_for_each_segment(bvec, bio, iter) {
> len = bvec.bv_len;
> @@ -2146,6 +2237,9 @@ static int null_add_dev(struct nullb_device *dev)
> list_add_tail(&nullb->list, &nullb_list);
> mutex_unlock(&lock);
>
> + blk_queue_max_copy_sectors_hw(nullb->disk->queue, 1024);
> + blk_queue_flag_set(QUEUE_FLAG_COPY, nullb->disk->queue);
This should NOT be unconditionally enabled with a magic value of 1K sectors. The
max copy sectors needs to be set with a configfs attribute so that we can
enable/disable the copy offload support, to be able to exercise both block layer
emulation and native device support.
> +
> pr_info("disk %s created\n", nullb->disk_name);
>
> return 0;
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index eb5972c50be8..94e524e7306a 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -67,6 +67,13 @@ enum {
> NULL_Q_MQ = 2,
> };
>
> +struct nullb_copy_token {
> + char subsys[5];
> + struct nullb *nullb;
> + u64 sector_in;
> + u64 sectors;
> +};
> +
> struct nullb_device {
> struct nullb *nullb;
> struct config_item item;
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v8 9/9] null_blk: add support for copy offload
2023-03-29 9:04 ` Damien Le Moal
@ 2023-03-29 12:22 ` Nitesh Shetty
0 siblings, 0 replies; 27+ messages in thread
From: Nitesh Shetty @ 2023-03-29 12:22 UTC (permalink / raw)
To: Damien Le Moal
Cc: Anuj Gupta, Jens Axboe, Alasdair Kergon, Mike Snitzer, dm-devel,
Keith Busch, Christoph Hellwig, Sagi Grimberg, James Smart,
Chaitanya Kulkarni, Alexander Viro, Christian Brauner, bvanassche,
hare, ming.lei, joshi.k, nitheshshetty, gost.dev, Vincent Fu,
linux-block, linux-kernel, linux-nvme, linux-fsdevel
[-- Attachment #1: Type: text/plain, Size: 5829 bytes --]
On Wed, Mar 29, 2023 at 06:04:49PM +0900, Damien Le Moal wrote:
> On 3/27/23 17:40, Anuj Gupta wrote:
> > From: Nitesh Shetty <nj.shetty@samsung.com>
> >
> > Implementaion is based on existing read and write infrastructure.
> >
> > Suggested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
> > ---
> > drivers/block/null_blk/main.c | 94 +++++++++++++++++++++++++++++++
> > drivers/block/null_blk/null_blk.h | 7 +++
> > 2 files changed, 101 insertions(+)
> >
> > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> > index 9e6b032c8ecc..84c5fbcd67a5 100644
> > --- a/drivers/block/null_blk/main.c
> > +++ b/drivers/block/null_blk/main.c
> > @@ -1257,6 +1257,81 @@ static int null_transfer(struct nullb *nullb, struct page *page,
> > return err;
> > }
> >
> > +static inline int nullb_setup_copy_read(struct nullb *nullb,
> > + struct bio *bio)
> > +{
> > + struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
> > +
> > + memcpy(token->subsys, "nullb", 5);
> > + token->sector_in = bio->bi_iter.bi_sector;
> > + token->nullb = nullb;
> > + token->sectors = bio->bi_iter.bi_size >> SECTOR_SHIFT;
> > +
> > + return 0;
> > +}
> > +
> > +static inline int nullb_setup_copy_write(struct nullb *nullb,
> > + struct bio *bio, bool is_fua)
> > +{
> > + struct nullb_copy_token *token = bvec_kmap_local(&bio->bi_io_vec[0]);
> > + sector_t sector_in, sector_out;
> > + void *in, *out;
> > + size_t rem, temp;
> > + unsigned long offset_in, offset_out;
> > + struct nullb_page *t_page_in, *t_page_out;
> > + int ret = -EIO;
> > +
> > + if (unlikely(memcmp(token->subsys, "nullb", 5)))
> > + return -EOPNOTSUPP;
> > + if (unlikely(token->nullb != nullb))
> > + return -EOPNOTSUPP;
> > + if (WARN_ON(token->sectors != bio->bi_iter.bi_size >> SECTOR_SHIFT))
> > + return -EOPNOTSUPP;
>
> EOPNOTSUPP is strange. These are EINVAL, no ?.
>
acked, will update in next revision.
> > +
> > + sector_in = token->sector_in;
> > + sector_out = bio->bi_iter.bi_sector;
> > + rem = token->sectors << SECTOR_SHIFT;
> > +
> > + spin_lock_irq(&nullb->lock);
> > + while (rem > 0) {
> > + temp = min_t(size_t, nullb->dev->blocksize, rem);
> > + offset_in = (sector_in & SECTOR_MASK) << SECTOR_SHIFT;
> > + offset_out = (sector_out & SECTOR_MASK) << SECTOR_SHIFT;
> > +
> > + if (null_cache_active(nullb) && !is_fua)
> > + null_make_cache_space(nullb, PAGE_SIZE);
> > +
> > + t_page_in = null_lookup_page(nullb, sector_in, false,
> > + !null_cache_active(nullb));
> > + if (!t_page_in)
> > + goto err;
> > + t_page_out = null_insert_page(nullb, sector_out,
> > + !null_cache_active(nullb) || is_fua);
> > + if (!t_page_out)
> > + goto err;
> > +
> > + in = kmap_local_page(t_page_in->page);
> > + out = kmap_local_page(t_page_out->page);
> > +
> > + memcpy(out + offset_out, in + offset_in, temp);
> > + kunmap_local(out);
> > + kunmap_local(in);
> > + __set_bit(sector_out & SECTOR_MASK, t_page_out->bitmap);
> > +
> > + if (is_fua)
> > + null_free_sector(nullb, sector_out, true);
> > +
> > + rem -= temp;
> > + sector_in += temp >> SECTOR_SHIFT;
> > + sector_out += temp >> SECTOR_SHIFT;
> > + }
> > +
> > + ret = 0;
> > +err:
> > + spin_unlock_irq(&nullb->lock);
> > + return ret;
> > +}
> > +
> > static int null_handle_rq(struct nullb_cmd *cmd)
> > {
> > struct request *rq = cmd->rq;
> > @@ -1267,6 +1342,14 @@ static int null_handle_rq(struct nullb_cmd *cmd)
> > struct req_iterator iter;
> > struct bio_vec bvec;
> >
> > + if (rq->cmd_flags & REQ_COPY) {
> > + if (op_is_write(req_op(rq)))
> > + return nullb_setup_copy_write(nullb, rq->bio,
> > + rq->cmd_flags & REQ_FUA);
> > + else
>
> No need for this else.
>
acked
> > + return nullb_setup_copy_read(nullb, rq->bio);
> > + }
> > +
> > spin_lock_irq(&nullb->lock);
> > rq_for_each_segment(bvec, rq, iter) {
> > len = bvec.bv_len;
> > @@ -1294,6 +1377,14 @@ static int null_handle_bio(struct nullb_cmd *cmd)
> > struct bio_vec bvec;
> > struct bvec_iter iter;
> >
> > + if (bio->bi_opf & REQ_COPY) {
> > + if (op_is_write(bio_op(bio)))
> > + return nullb_setup_copy_write(nullb, bio,
> > + bio->bi_opf & REQ_FUA);
> > + else
>
> No need for this else.
>
acked
> > + return nullb_setup_copy_read(nullb, bio);
> > + }
> > +
> > spin_lock_irq(&nullb->lock);
> > bio_for_each_segment(bvec, bio, iter) {
> > len = bvec.bv_len;
> > @@ -2146,6 +2237,9 @@ static int null_add_dev(struct nullb_device *dev)
> > list_add_tail(&nullb->list, &nullb_list);
> > mutex_unlock(&lock);
> >
> > + blk_queue_max_copy_sectors_hw(nullb->disk->queue, 1024);
> > + blk_queue_flag_set(QUEUE_FLAG_COPY, nullb->disk->queue);
>
> This should NOT be unconditionally enabled with a magic value of 1K sectors. The
> max copy sectors needs to be set with a configfs attribute so that we can
> enable/disable the copy offload support, to be able to exercise both block layer
> emulation and native device support.
>
acked
> > +
> > pr_info("disk %s created\n", nullb->disk_name);
> >
> > return 0;
> > diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> > index eb5972c50be8..94e524e7306a 100644
> > --- a/drivers/block/null_blk/null_blk.h
> > +++ b/drivers/block/null_blk/null_blk.h
> > @@ -67,6 +67,13 @@ enum {
> > NULL_Q_MQ = 2,
> > };
> >
> > +struct nullb_copy_token {
> > + char subsys[5];
> > + struct nullb *nullb;
> > + u64 sector_in;
> > + u64 sectors;
> > +};
> > +
> > struct nullb_device {
> > struct nullb *nullb;
> > struct config_item item;
>
> --
> Damien Le Moal
> Western Digital Research
>
>
Thank you,
Nitesh Shetty
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread