From: Selva Jove <selvajove@gmail.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Damien Le Moal <Damien.LeMoal@wdc.com>,
SelvaKumar S <selvakuma.s1@samsung.com>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
"axboe@kernel.dk" <axboe@kernel.dk>,
"sagi@grimberg.me" <sagi@grimberg.me>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
"snitzer@redhat.com" <snitzer@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"nj.shetty@samsung.com" <nj.shetty@samsung.com>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"dm-devel@redhat.com" <dm-devel@redhat.com>,
"mpatocka@redhat.com" <mpatocka@redhat.com>,
"joshi.k@samsung.com" <joshi.k@samsung.com>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"kbusch@kernel.org" <kbusch@kernel.org>,
"javier.gonz@samsung.com" <javier.gonz@samsung.com>,
"hch@lst.de" <hch@lst.de>,
"bvanassche@acm.org" <bvanassche@acm.org>
Subject: Re: [dm-devel] [RFC PATCH v4 2/3] block: add simple copy support
Date: Tue, 5 Jan 2021 19:52:48 +0530 [thread overview]
Message-ID: <CAHqX9vYCRpX9FSCEaf2+xbYkB-zm7wRTmUka9zGs4D1T6nZb_A@mail.gmail.com> (raw)
In-Reply-To: <20210104190254.GB6919@magnolia>
Hi Darrick,
On Tue, Jan 5, 2021 at 12:33 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> SelvaKumar S: This didn't show up on dm-devel, sorry for the OT reply...
>
> On Mon, Jan 04, 2021 at 12:47:11PM +0000, Damien Le Moal wrote:
> > On 2021/01/04 19:48, SelvaKumar S wrote:
> > > Add new BLKCOPY ioctl that offloads copying of one or more sources
> > > ranges to a destination in the device. Accepts copy_ranges that contains
> > > destination, no of sources and pointer to the array of source
> > > ranges. Each range_entry contains start and length of source
> > > ranges (in bytes).
> > >
> > > Introduce REQ_OP_COPY, a no-merge copy offload operation. Create
> > > bio with control information as payload and submit to the device.
> > > REQ_OP_COPY(19) is a write op and takes zone_write_lock when submitted
> > > to zoned device.
> > >
> > > If the device doesn't support copy or copy offload is disabled, then
> > > copy is emulated by allocating memory of total copy size. The source
> > > ranges are read into memory by chaining bio for each source ranges and
> > > submitting them async and the last bio waits for completion. After data
> > > is read, it is written to the destination.
> > >
> > > bio_map_kern() is used to allocate bio and add pages of copy buffer to
> > > bio. As bio->bi_private and bio->bi_end_io is needed for chaining the
> > > bio and over written, invalidate_kernel_vmap_range() for read is called
> > > in the caller.
> > >
> > > Introduce queue limits for simple copy and other helper functions.
> > > Add device limits as sysfs entries.
> > > - copy_offload
> > > - max_copy_sectors
> > > - max_copy_ranges_sectors
> > > - max_copy_nr_ranges
> > >
> > > copy_offload(= 0) is disabled by default.
> > > max_copy_sectors = 0 indicates the device doesn't support native copy.
> > >
> > > Native copy offload is not supported for stacked devices and is done via
> > > copy emulation.
> > >
> > > Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
> > > 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>
> > > ---
> > > block/blk-core.c | 94 ++++++++++++++--
> > > block/blk-lib.c | 223 ++++++++++++++++++++++++++++++++++++++
> > > block/blk-merge.c | 2 +
> > > block/blk-settings.c | 10 ++
> > > block/blk-sysfs.c | 50 +++++++++
> > > block/blk-zoned.c | 1 +
> > > block/bounce.c | 1 +
> > > block/ioctl.c | 43 ++++++++
> > > include/linux/bio.h | 1 +
> > > include/linux/blk_types.h | 15 +++
> > > include/linux/blkdev.h | 13 +++
> > > include/uapi/linux/fs.h | 13 +++
>
> This series should also be cc'd to linux-api since you're adding a new
> userspace api.
>
Sure. Will cc linux-api
>
> Alternately, save yourself the trouble of passing userspace API review
> by hooking this up to the existing copy_file_range call in the vfs. /me
> hopes you sent blktests to check the operation of this thing, since none
> of the original patches made it to this list.
>
As the initial version had only source bdev, copy_file_rage call was not
viable. With this version, we have two bdev for source and destination.
I'll relook at that. Sure. Will add a new blktests for simple copy.
>
> If you really /do/ need a new kernel call for this, then please send in
> a manpage documenting the fields of struct range_entry and copy_range,
> and how users are supposed to use this.
>
Sure. Will send a manpage documentation once the plumbing is concrete.
> <now jumping to the ioctl definition because Damien already reviewed the
> plumbing...>
>
> > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > > index f44eb0a04afd..5cadb176317a 100644
> > > --- a/include/uapi/linux/fs.h
> > > +++ b/include/uapi/linux/fs.h
> > > @@ -64,6 +64,18 @@ struct fstrim_range {
> > > __u64 minlen;
> > > };
> > >
> > > +struct range_entry {
> > > + __u64 src;
>
> Is this an offset? Or the fd of an open bdev?
This is the source offset.
>
> > > + __u64 len;
> > > +};
> > > +
> > > +struct copy_range {
> > > + __u64 dest;
> > > + __u64 nr_range;
> > > + __u64 range_list;
>
> Hm, I think this is a pointer? Why not just put the range_entry array
> at the end of struct copy_range?
>
As the size of the range_entry array can change dynamically depending on
nr_range, it was difficult to do copy_from_user() on copy_range structure in the
ioctl. So I decided to keep that as a pointer to range_entry array
instead of keeping
array at the end.
> > > + __u64 rsvd;
>
> Also needs a flags argument so we don't have to add BLKCOPY2 in like 3
> months.
>
'rsvd' field is kept to support future copies. Will rename it.
> --D
>
> > > +};
> > > +
> > > /* extent-same (dedupe) ioctls; these MUST match the btrfs ioctl definitions */
> > > #define FILE_DEDUPE_RANGE_SAME 0
> > > #define FILE_DEDUPE_RANGE_DIFFERS 1
> > > @@ -184,6 +196,7 @@ struct fsxattr {
> > > #define BLKSECDISCARD _IO(0x12,125)
> > > #define BLKROTATIONAL _IO(0x12,126)
> > > #define BLKZEROOUT _IO(0x12,127)
> > > +#define BLKCOPY _IOWR(0x12, 128, struct copy_range)
> > > /*
> > > * A jump here: 130-131 are reserved for zoned block devices
> > > * (see uapi/linux/blkzoned.h)
> > >
> >
> >
> > --
> > Damien Le Moal
> > Western Digital Research
> >
> >
> >
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
> >
Thanks,
Selva
next prev parent reply other threads:[~2021-01-05 14:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20210104104237epcas5p1ed2d167f50060eb0567ade5d9a1df701@epcas5p1.samsung.com>
2021-01-04 10:41 ` [RFC PATCH v4 0/3] add simple copy support SelvaKumar S
[not found] ` <CGME20210104104245epcas5p26ed395efbf74e78a4e44048a6d7d6ba7@epcas5p2.samsung.com>
2021-01-04 10:41 ` [RFC PATCH v4 1/3] block: export bio_map_kern() SelvaKumar S
2021-01-04 12:15 ` Damien Le Moal
2021-01-04 12:42 ` Selva Jove
[not found] ` <CGME20210104104249epcas5p458d1b5c39b5acfad4e7786eca5dd5c49@epcas5p4.samsung.com>
2021-01-04 10:41 ` [RFC PATCH v4 2/3] block: add simple copy support SelvaKumar S
2021-01-04 12:47 ` Damien Le Moal
2021-01-04 19:02 ` [dm-devel] " Darrick J. Wong
2021-01-05 14:22 ` Selva Jove [this message]
2021-01-05 12:24 ` Selva Jove
2021-01-05 23:33 ` Damien Le Moal
[not found] ` <CGME20210104104254epcas5p212bb42457cfbaed5aeaeaa5b6625922b@epcas5p2.samsung.com>
2021-01-04 10:41 ` [RFC PATCH v4 3/3] nvme: " SelvaKumar S
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHqX9vYCRpX9FSCEaf2+xbYkB-zm7wRTmUka9zGs4D1T6nZb_A@mail.gmail.com \
--to=selvajove@gmail.com \
--cc=Damien.LeMoal@wdc.com \
--cc=Johannes.Thumshirn@wdc.com \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=darrick.wong@oracle.com \
--cc=dm-devel@redhat.com \
--cc=hch@lst.de \
--cc=javier.gonz@samsung.com \
--cc=joshi.k@samsung.com \
--cc=kbusch@kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mpatocka@redhat.com \
--cc=nj.shetty@samsung.com \
--cc=sagi@grimberg.me \
--cc=selvakuma.s1@samsung.com \
--cc=snitzer@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).