From: "Darrick J. Wong" <djwong@kernel.org>
To: Nitesh Shetty <nitheshshetty@gmail.com>
Cc: snitzer@redhat.com, linux-nvme@lists.infradead.org,
dm-devel@redhat.com, hch@lst.de, agk@redhat.com,
bvanassche@acm.org, linux-scsi@vger.kernel.org,
willy@infradead.org, nj.shetty@samsung.com, kch@kernel.org,
SelvaKumar S <selvakuma.s1@samsung.com>,
selvajove@gmail.com, linux-block@vger.kernel.org,
mpatocka@redhat.com, javier.gonz@samsung.com, kbusch@kernel.org,
axboe@kernel.dk, damien.lemoal@wdc.com, joshi.k@samsung.com,
martin.petersen@oracle.com, linux-api@vger.kernel.org,
johannes.thumshirn@wdc.com, linux-fsdevel@vger.kernel.org,
joshiiitr@gmail.com, asml.silence@gmail.com
Subject: Re: [dm-devel] [PATCH 4/7] block: Introduce a new ioctl for simple copy
Date: Wed, 18 Aug 2021 09:17:50 -0700 [thread overview]
Message-ID: <20210818161750.GF12664@magnolia> (raw)
In-Reply-To: <CAOSviJ2+deUdDXTWbWXaSxNX2t6cnhPg7KCDA4C2qm74KD9vdQ@mail.gmail.com>
On Wed, Aug 18, 2021 at 09:07:54PM +0530, Nitesh Shetty wrote:
> On Wed, Aug 18, 2021 at 5:06 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Tue, Aug 17, 2021 at 03:44:20PM +0530, SelvaKumar S wrote:
> > > From: Nitesh Shetty <nj.shetty@samsung.com>
> > >
> > > Add new BLKCOPY ioctl that offloads copying of one or more sources ranges
> > > to a destination in the device. COPY ioctl accepts a 'copy_range'
> > > structure that contains destination (in sectors), no of sources and
> > > pointer to the array of source ranges. Each source range is represented by
> > > 'range_entry' that contains start and length of source ranges (in sectors)
> > >
> > > MAX_COPY_NR_RANGE, limits the number of entries for the IOCTL and
> > > MAX_COPY_TOTAL_LENGTH limits the total copy length, IOCTL can handle.
> > >
> > > Example code, to issue BLKCOPY:
> > > /* Sample example to copy three source-ranges [0, 8] [16, 8] [32,8] to
> > > * [64,24], on the same device */
> > >
> > > int main(void)
> > > {
> > > int ret, fd;
> > > struct range_entry source_range[] = {{.src = 0, .len = 8},
> > > {.src = 16, .len = 8}, {.src = 32, .len = 8},};
> > > struct copy_range cr;
> > >
> > > cr.dest = 64;
> > > cr.nr_range = 3;
> > > cr.range_list = (__u64)&source_range;
> > >
> > > fd = open("/dev/nvme0n1", O_RDWR);
> > > if (fd < 0) return 1;
> > >
> > > ret = ioctl(fd, BLKCOPY, &cr);
> > > if (ret < 0) printf("copy failure\n");
> > >
> > > close(fd);
> > >
> > > return ret;
> > > }
> > >
> > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> > > Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
> > > Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> > > ---
> > > block/ioctl.c | 33 +++++++++++++++++++++++++++++++++
> > > include/uapi/linux/fs.h | 8 ++++++++
> > > 2 files changed, 41 insertions(+)
> > >
> > > diff --git a/block/ioctl.c b/block/ioctl.c
> > > index eb0491e90b9a..2af56d01e9fe 100644
> > > --- a/block/ioctl.c
> > > +++ b/block/ioctl.c
> > > @@ -143,6 +143,37 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
> > > GFP_KERNEL, flags);
> > > }
> > >
> > > +static int blk_ioctl_copy(struct block_device *bdev, fmode_t mode,
> > > + unsigned long arg)
> > > +{
> > > + struct copy_range crange;
> > > + struct range_entry *rlist;
> > > + int ret;
> > > +
> > > + if (!(mode & FMODE_WRITE))
> > > + return -EBADF;
> > > +
> > > + if (copy_from_user(&crange, (void __user *)arg, sizeof(crange)))
> > > + return -EFAULT;
> > > +
> > > + rlist = kmalloc_array(crange.nr_range, sizeof(*rlist),
> > > + GFP_KERNEL);
> > > + if (!rlist)
> > > + return -ENOMEM;
> > > +
> > > + if (copy_from_user(rlist, (void __user *)crange.range_list,
> > > + sizeof(*rlist) * crange.nr_range)) {
> > > + ret = -EFAULT;
> > > + goto out;
> > > + }
> > > +
> > > + ret = blkdev_issue_copy(bdev, crange.nr_range, rlist, bdev, crange.dest,
> > > + GFP_KERNEL, 0);
> > > +out:
> > > + kfree(rlist);
> > > + return ret;
> > > +}
> > > +
> > > static int blk_ioctl_zeroout(struct block_device *bdev, fmode_t mode,
> > > unsigned long arg)
> > > {
> > > @@ -468,6 +499,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
> > > case BLKSECDISCARD:
> > > return blk_ioctl_discard(bdev, mode, arg,
> > > BLKDEV_DISCARD_SECURE);
> > > + case BLKCOPY:
> > > + return blk_ioctl_copy(bdev, mode, arg);
> > > case BLKZEROOUT:
> > > return blk_ioctl_zeroout(bdev, mode, arg);
> > > case BLKGETDISKSEQ:
> > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > > index 7a97b588d892..4183688ff398 100644
> > > --- a/include/uapi/linux/fs.h
> > > +++ b/include/uapi/linux/fs.h
> > > @@ -76,6 +76,13 @@ struct range_entry {
> > > __u64 len;
> > > };
> > >
> > > +struct copy_range {
> > > + __u64 dest;
> > > + __u64 nr_range;
> >
> > If the maximum number of elements in the range list is 1<<12, there's no
> > need for this to be larger than a u16, right?
> >
> > > + __u64 range_list;
> >
> > Pointers embedded in a structure are /not/ a good idea, because this
> > will create a lot of compatibility headaches for 32-bit binaries running
> > on 64-bit kernels. Please just make the size of this header structure
> > a multiple of 8 bytes and put the range_entry list immediately after it.
> >
> > struct copy_range {
> > __s64 dest_offset;
> > __u32 nr_range_entries;
> > __u32 flags;
> > __u64 reserved[2];
> > };
> >
> > struct __user range_entry *re = ((struct range_entry *)(copyhead + 1));
> >
> > copy_from_user(&urk, re...);
> >
> > --D
> >
> Thanks, this is better. 'Reserved' field was there to be used for
> future extension of the interface.
> Now that you mentioned 'flags', it seems we can do away with
> 'reserved' fields altogether?
We still want the reserved-must-be-zero fields so that adding the first
field or two doesn't require changes to the pointer arithmetic.
Also, I suppose you could make the relationship between copy_range and
range_entry more explicit:
struct copy_range {
__s64 dest_offset;
__u32 nr_range_entries;
__u32 flags;
__u64 reserved[2];
/* must come last */
struct range_entry entries[];
};
struct __user range_entry *re = ©head->entries[0];
--D
>
> Regards,
> Nitesh Shetty
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-08-18 16:18 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20210817101741epcas5p174ca0a539587da6a67b9f58cd13f2bad@epcas5p1.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 0/7] add simple copy support SelvaKumar S
[not found] ` <CGME20210817101747epcas5p1242e63ec29b127b03b6f9f5f1b57f86e@epcas5p1.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 1/7] block: make bio_map_kern() non static SelvaKumar S
[not found] ` <CGME20210817101753epcas5p4f4257f8edda27e184ecbb273b700ccbc@epcas5p4.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 2/7] block: Introduce queue limits for copy-offload support SelvaKumar S
2021-08-17 13:08 ` Greg KH
2021-08-17 14:42 ` Nitesh Shetty
[not found] ` <CGME20210817101758epcas5p1ec353b3838d64654e69488229256d9eb@epcas5p1.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 3/7] block: copy offload support infrastructure SelvaKumar S
2021-08-17 17:14 ` Bart Van Assche
2021-08-17 20:41 ` Mikulas Patocka
2021-08-17 21:53 ` Douglas Gilbert
2021-08-17 22:06 ` Bart Van Assche
2021-08-20 10:39 ` Kanchan Joshi
2021-08-20 21:18 ` Bart Van Assche
2021-08-26 7:46 ` Nitesh Shetty
2021-08-17 20:35 ` kernel test robot
2021-08-18 18:35 ` Martin K. Petersen
2021-08-20 11:11 ` Kanchan Joshi
[not found] ` <CGME20210817101803epcas5p10cda1d52f8a8f1172e34b1f9cf8eef3b@epcas5p1.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 4/7] block: Introduce a new ioctl for simple copy SelvaKumar S
2021-08-17 13:09 ` Greg KH
2021-08-17 13:10 ` Greg KH
2021-08-17 14:48 ` Nitesh Shetty
2021-08-17 23:36 ` Darrick J. Wong
2021-08-18 15:37 ` Nitesh Shetty
2021-08-18 16:17 ` Darrick J. Wong [this message]
[not found] ` <CGME20210817101809epcas5p39eed3531ed82f5f08127eb3dba1fc50f@epcas5p3.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 5/7] block: add emulation " SelvaKumar S
2021-08-17 22:10 ` kernel test robot
[not found] ` <CGME20210817101814epcas5p41db3d7269f5139efcaf2ca685cd04a16@epcas5p4.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 6/7] nvme: add simple copy support SelvaKumar S
[not found] ` <CGME20210817101822epcas5p470644cf681d5e8db5367dc7998305c65@epcas5p4.samsung.com>
2021-08-17 10:14 ` [dm-devel] [PATCH 7/7] dm kcopyd: add simple copy offload support SelvaKumar S
2021-08-17 20:29 ` Mikulas Patocka
2021-08-17 23:37 ` [dm-devel] [PATCH 0/7] add simple copy support Darrick J. Wong
2021-08-18 15:40 ` Nitesh Shetty
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=20210818161750.GF12664@magnolia \
--to=djwong@kernel.org \
--cc=agk@redhat.com \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=damien.lemoal@wdc.com \
--cc=dm-devel@redhat.com \
--cc=hch@lst.de \
--cc=javier.gonz@samsung.com \
--cc=johannes.thumshirn@wdc.com \
--cc=joshi.k@samsung.com \
--cc=joshiiitr@gmail.com \
--cc=kbusch@kernel.org \
--cc=kch@kernel.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@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=nitheshshetty@gmail.com \
--cc=nj.shetty@samsung.com \
--cc=selvajove@gmail.com \
--cc=selvakuma.s1@samsung.com \
--cc=snitzer@redhat.com \
--cc=willy@infradead.org \
/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).