[PATCH v2] introduce grant copy for user land

[PATCH v2] introduce grant copy for user land

[Thanos Makatos]

This patch introduces the interface to allow user-space applications
execute grant-copy operations. This is done by sending an ioctl to the
grant device.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
---
 drivers/xen/gntdev.c      |  171 +++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/xen/gntdev.h |   69 ++++++++++++++++++
 2 files changed, 240 insertions(+)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 51f4c95..7b4a8e0 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -705,6 +705,174 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u)
 	return rc;
 }
 
+static int gntdev_gcopy_batch(int nr_segments, unsigned long gcopy_cb,
+		struct gntdev_grant_copy_segment __user *__segments, int dir,
+		int src, int dst) {
+
+	static const int batch_size = PAGE_SIZE / (sizeof(struct page*) +
+		sizeof(struct gnttab_copy) + sizeof(struct gntdev_grant_copy_segment));
+	struct page **pages = (struct page **)gcopy_cb;
+	struct gnttab_copy *batch = (struct gnttab_copy *)((unsigned long)pages +
+			sizeof(struct page*) * batch_size);
+	struct gntdev_grant_copy_segment *segments =
+		(struct gntdev_grant_copy_segment *)((unsigned long)batch +
+				sizeof(struct gnttab_copy) * batch_size);
+	unsigned int nr_pinned = 0, nr_segs2cp = 0;
+	int err = 0, i;
+	const int write = dir == GNTCOPY_IOCTL_g2s;
+
+	nr_segments = min(nr_segments, batch_size);
+
+	if (unlikely(copy_from_user(segments, __segments,
+			   sizeof(struct gntdev_grant_copy_segment) * nr_segments))) {
+		pr_debug("failed to copy %d segments from user", nr_segments);
+		err = -EFAULT;
+		goto out;
+	}
+
+	for (i = 0; i < nr_segments; i++) {
+
+		xen_pfn_t pgaddr;
+		unsigned long start, offset;
+		struct gntdev_grant_copy_segment *seg = &segments[i];
+
+		if (dir == GNTCOPY_IOCTL_s2g || dir == GNTCOPY_IOCTL_g2s) {
+
+			start = (unsigned long)seg->self.iov.iov_base & PAGE_MASK;
+			offset = (unsigned long)seg->self.iov.iov_base & ~PAGE_MASK;
+			if (unlikely(offset + seg->self.iov.iov_len > PAGE_SIZE)) {
+				pr_warn("segments crossing page boundaries not yet "
+						"implemented\n");
+				err = -ENOSYS;
+				goto out;
+			}
+
+			err = get_user_pages_fast(start, 1, write, &pages[i]);
+			if (unlikely(err != 1)) {
+				pr_debug("failed to get user page %lu", start);
+				err = -EFAULT;
+				goto out;
+			}
+
+			nr_pinned++;
+
+			pgaddr = pfn_to_mfn(page_to_pfn(pages[i]));
+		}
+
+		nr_segs2cp++;
+
+		switch (dir) {
+		case GNTCOPY_IOCTL_g2s: /* copy from guest */
+			batch[i].len = seg->self.iov.iov_len;
+			batch[i].source.u.ref = seg->self.ref;
+			batch[i].source.domid = src;
+			batch[i].source.offset = seg->self.offset;
+			batch[i].dest.u.gmfn = pgaddr;
+			batch[i].dest.domid = DOMID_SELF;
+			batch[i].dest.offset = offset;
+			batch[i].flags = GNTCOPY_source_gref;
+			break;
+		case GNTCOPY_IOCTL_s2g: /* copy to guest */
+			batch[i].len = seg->self.iov.iov_len;
+			batch[i].source.u.gmfn = pgaddr;
+			batch[i].source.domid = DOMID_SELF;
+			batch[i].source.offset = offset;
+			batch[i].dest.u.ref = seg->self.ref;
+			batch[i].dest.domid = dst;
+			batch[i].dest.offset = seg->self.offset;
+			batch[i].flags = GNTCOPY_dest_gref;
+			break;
+		case GNTCOPY_IOCTL_g2g: /* copy guest to guest */
+			batch[i].len = seg->g2g.len;
+			batch[i].source.u.ref = seg->g2g.src.ref;
+			batch[i].source.domid = src;
+			batch[i].source.offset = seg->g2g.src.offset;
+			batch[i].dest.u.ref = seg->g2g.dst.ref;
+			batch[i].dest.domid = dst;
+			batch[i].dest.offset = seg->g2g.dst.offset;
+			batch[i].flags = GNTCOPY_source_gref | GNTCOPY_dest_gref;
+			break;
+		default:
+			pr_debug("invalid grant-copy direction %d\n", dir);
+			err = -EINVAL;
+			goto out;
+		}
+	}
+
+	gnttab_batch_copy(batch, nr_segs2cp);
+	for (i = 0; i < nr_segs2cp; i++) {
+		err = put_user(batch[i].status, &__segments[i].status);
+		if (unlikely(err)) {
+			pr_debug("failed to copy error code %d to user: %d\n",
+					batch[i].status, err);
+			goto out;
+		}
+	}
+
+out:
+	for (i = 0; i < nr_pinned; i++)
+		put_page(pages[i]);
+
+	if (unlikely(err))
+		return err;
+
+	return nr_segs2cp;
+}
+
+static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u)
+{
+	struct ioctl_gntdev_grant_copy op;
+	unsigned int remaining;
+	int err = 0;
+	unsigned long gcopy_cb = 0;
+
+	if (unlikely(copy_from_user(&op, u, sizeof(op)))) {
+		pr_debug("failed to copy grant-copy parameters from user");
+		err = -EFAULT;
+		goto out;
+	}
+
+	if (unlikely(op.dir != GNTCOPY_IOCTL_s2g && op.dir != GNTCOPY_IOCTL_g2s &&
+			op.dir != GNTCOPY_IOCTL_g2g)) {
+		pr_debug("invalid copy direction %d\n", op.dir);
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (!op.count) {
+		pr_debug("no segments to transfer");
+		err = 0;
+		goto out;
+	}
+
+	gcopy_cb = get_zeroed_page(GFP_KERNEL);
+	if (unlikely(!gcopy_cb)) {
+		pr_debug("failed to allocate page");
+		err = -ENOMEM;
+		goto out;
+	}
+
+	remaining = op.count;
+
+	while (remaining > 0) {
+		err = gntdev_gcopy_batch(remaining, gcopy_cb,
+				op.segments + (op.count - remaining), op.dir, op.src, op.dst);
+		if (unlikely(err < 0)) {
+			pr_debug("failed to grant-copy %d segments: %d",
+					op.count - remaining, err);
+			goto out;
+		}
+		remaining -= err;
+	}
+
+	err = 0;
+
+out:
+	if (likely(gcopy_cb))
+		free_page(gcopy_cb);
+	return err;
+}
+
 static long gntdev_ioctl(struct file *flip,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -724,6 +892,9 @@ static long gntdev_ioctl(struct file *flip,
 	case IOCTL_GNTDEV_SET_UNMAP_NOTIFY:
 		return gntdev_ioctl_notify(priv, ptr);
 
+	case IOCTL_GNTDEV_GRANT_COPY:
+		return gntdev_ioctl_grant_copy(priv, ptr);
+
 	default:
 		pr_debug("priv %p, unknown cmd %x\n", priv, cmd);
 		return -ENOIOCTLCMD;
diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h
index 5304bd3..17da5e9 100644
--- a/include/uapi/xen/gntdev.h
+++ b/include/uapi/xen/gntdev.h
@@ -33,6 +33,12 @@
 #ifndef __LINUX_PUBLIC_GNTDEV_H__
 #define __LINUX_PUBLIC_GNTDEV_H__
 
+#ifdef __KERNEL__
+#include <linux/uio.h>
+#else
+#include <sys/uio.h>
+#endif
+
 struct ioctl_gntdev_grant_ref {
 	/* The domain ID of the grant to be mapped. */
 	uint32_t domid;
@@ -142,6 +148,69 @@ struct ioctl_gntdev_unmap_notify {
 	uint32_t event_channel_port;
 };
 
+struct gntdev_grant_copy_segment {
+
+	union {
+		/* copy from (to) self to (from) guest */
+		struct {
+			/*
+			 * source address and length
+			 */
+			struct iovec iov;
+
+			/* the granted page */
+			uint32_t ref;
+
+			/* offset in the granted page */
+			uint16_t offset;
+		} self;
+
+		/* copy from guest to guest */
+		struct {
+			uint16_t len;
+
+			struct {
+				/* the granted page */
+				uint32_t ref;
+
+				/* offset in the granted page */
+				uint16_t offset;
+			} src, dst;
+		} g2g;
+	};
+
+	/* grant copy result (GNTST_XXX) */
+	int16_t status;
+};
+
+/* grant-copy from self to guest */
+#define GNTCOPY_IOCTL_s2g 0
+/* grant-copy from guest to self */
+#define GNTCOPY_IOCTL_g2s 1
+/* grant-copy from guest to guest */
+#define GNTCOPY_IOCTL_g2g 2
+
+#define IOCTL_GNTDEV_GRANT_COPY \
+_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
+struct ioctl_gntdev_grant_copy {
+	/*
+	 * copy direction, see GNTCOPY_IOCTL_XXX
+	 */
+	int dir;
+
+	/*
+	 * domain IDs:
+	 *	When the source (destination) domain is the one executing the
+	 *	grant-copy operation, the @src (@dst) parameter is ignored.
+	 */
+	uint32_t src;
+	uint32_t dst;
+
+	unsigned int count;
+
+	struct gntdev_grant_copy_segment __user *segments;
+};
+
 /* Clear (set to zero) the byte specified by index */
 #define UNMAP_NOTIFY_CLEAR_BYTE 0x1
 /* Send an interrupt on the indicated event channel */
-- 
1.7.9.5

Re: [PATCH v2] introduce grant copy for user land

[Boris Ostrovsky]

On 12/02/2014 11:13 AM, Thanos Makatos wrote:
> This patch introduces the interface to allow user-space applications
> execute grant-copy operations. This is done by sending an ioctl to the
> grant device.
>
> Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
> ---
>   drivers/xen/gntdev.c      |  171 +++++++++++++++++++++++++++++++++++++++++++++
>   include/uapi/xen/gntdev.h |   69 ++++++++++++++++++
>   2 files changed, 240 insertions(+)
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index 51f4c95..7b4a8e0 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -705,6 +705,174 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u)
>   	return rc;
>   }
>   
> +static int gntdev_gcopy_batch(int nr_segments, unsigned long gcopy_cb,
> +		struct gntdev_grant_copy_segment __user *__segments, int dir,
> +		int src, int dst) {
> +
> +	static const int batch_size = PAGE_SIZE / (sizeof(struct page*) +
> +		sizeof(struct gnttab_copy) + sizeof(struct gntdev_grant_copy_segment));
> +	struct page **pages = (struct page **)gcopy_cb;
> +	struct gnttab_copy *batch = (struct gnttab_copy *)((unsigned long)pages +
> +			sizeof(struct page*) * batch_size);
> +	struct gntdev_grant_copy_segment *segments =
> +		(struct gntdev_grant_copy_segment *)((unsigned long)batch +
> +				sizeof(struct gnttab_copy) * batch_size);
> +	unsigned int nr_pinned = 0, nr_segs2cp = 0;
> +	int err = 0, i;
> +	const int write = dir == GNTCOPY_IOCTL_g2s;
> +
> +	nr_segments = min(nr_segments, batch_size);
> +
> +	if (unlikely(copy_from_user(segments, __segments,
> +			   sizeof(struct gntdev_grant_copy_segment) * nr_segments))) {
> +		pr_debug("failed to copy %d segments from user", nr_segments);
> +		err = -EFAULT;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < nr_segments; i++) {
> +
> +		xen_pfn_t pgaddr;
> +		unsigned long start, offset;
> +		struct gntdev_grant_copy_segment *seg = &segments[i];
> +
> +		if (dir == GNTCOPY_IOCTL_s2g || dir == GNTCOPY_IOCTL_g2s) {
> +
> +			start = (unsigned long)seg->self.iov.iov_base & PAGE_MASK;
> +			offset = (unsigned long)seg->self.iov.iov_base & ~PAGE_MASK;
> +			if (unlikely(offset + seg->self.iov.iov_len > PAGE_SIZE)) {
> +				pr_warn("segments crossing page boundaries not yet "
> +						"implemented\n");
> +				err = -ENOSYS;
> +				goto out;
> +			}
> +
> +			err = get_user_pages_fast(start, 1, write, &pages[i]);
> +			if (unlikely(err != 1)) {
> +				pr_debug("failed to get user page %lu", start);
> +				err = -EFAULT;
> +				goto out;
> +			}
> +
> +			nr_pinned++;
> +
> +			pgaddr = pfn_to_mfn(page_to_pfn(pages[i]));
> +		}
> +
> +		nr_segs2cp++;
> +
> +		switch (dir) {
> +		case GNTCOPY_IOCTL_g2s: /* copy from guest */
> +			batch[i].len = seg->self.iov.iov_len;
> +			batch[i].source.u.ref = seg->self.ref;
> +			batch[i].source.domid = src;
> +			batch[i].source.offset = seg->self.offset;
> +			batch[i].dest.u.gmfn = pgaddr;
> +			batch[i].dest.domid = DOMID_SELF;
> +			batch[i].dest.offset = offset;
> +			batch[i].flags = GNTCOPY_source_gref;
> +			break;
> +		case GNTCOPY_IOCTL_s2g: /* copy to guest */
> +			batch[i].len = seg->self.iov.iov_len;
> +			batch[i].source.u.gmfn = pgaddr;
> +			batch[i].source.domid = DOMID_SELF;
> +			batch[i].source.offset = offset;
> +			batch[i].dest.u.ref = seg->self.ref;
> +			batch[i].dest.domid = dst;
> +			batch[i].dest.offset = seg->self.offset;
> +			batch[i].flags = GNTCOPY_dest_gref;
> +			break;
> +		case GNTCOPY_IOCTL_g2g: /* copy guest to guest */
> +			batch[i].len = seg->g2g.len;
> +			batch[i].source.u.ref = seg->g2g.src.ref;
> +			batch[i].source.domid = src;
> +			batch[i].source.offset = seg->g2g.src.offset;
> +			batch[i].dest.u.ref = seg->g2g.dst.ref;
> +			batch[i].dest.domid = dst;
> +			batch[i].dest.offset = seg->g2g.dst.offset;
> +			batch[i].flags = GNTCOPY_source_gref | GNTCOPY_dest_gref;
> +			break;
> +		default:
> +			pr_debug("invalid grant-copy direction %d\n", dir);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +	}
> +
> +	gnttab_batch_copy(batch, nr_segs2cp);
> +	for (i = 0; i < nr_segs2cp; i++) {

Can nr_segs2cp be not equal to nr_segments here? If you got to this 
point you have gone through the full loop.

> +		err = put_user(batch[i].status, &__segments[i].status);
> +		if (unlikely(err)) {
> +			pr_debug("failed to copy error code %d to user: %d\n",
> +					batch[i].status, err);
> +			goto out;
> +		}
> +	}
> +
> +out:
> +	for (i = 0; i < nr_pinned; i++)
> +		put_page(pages[i]);
> +
> +	if (unlikely(err))
> +		return err;
> +
> +	return nr_segs2cp;

And I think here it can be either 0 (which is the case of an error) or 
nr_segments. If you error out of the 'for' loop you haven't actually 
copied anything, even though nr_segs2cp might be non-zero.


-boris

Re: [PATCH v2] introduce grant copy for user land

[David Vrabel]

On 02/12/14 16:13, Thanos Makatos wrote:
>  
> +struct gntdev_grant_copy_segment {
> +
> +	union {
> +		/* copy from (to) self to (from) guest */
> +		struct {
> +			/*
> +			 * source address and length
> +			 */
> +			struct iovec iov;
> +
> +			/* the granted page */
> +			uint32_t ref;
> +
> +			/* offset in the granted page */
> +			uint16_t offset;
> +		} self;
> +
> +		/* copy from guest to guest */
> +		struct {
> +			uint16_t len;
> +
> +			struct {
> +				/* the granted page */
> +				uint32_t ref;
> +
> +				/* offset in the granted page */
> +				uint16_t offset;
> +			} src, dst;
> +		} g2g;
> +	};
> +
> +	/* grant copy result (GNTST_XXX) */
> +	int16_t status;
> +};

I asked for this ioctl to mirror the hypercall.

Which looks like:

struct gnttab_copy {
        /* IN parameters. */
        struct {
                union {
                        grant_ref_t ref;
                        xen_pfn_t   gmfn;
                } u;
                domid_t  domid;
                uint16_t offset;
        } source, dest;
        uint16_t      len;
        uint16_t      flags;          /* GNTCOPY_* */
        /* OUT parameters. */
        int16_t       status;
};

i.e., each operation specifies the domid of the source and destination
and whether it includes a ref or a virtual address.

David

Re: [PATCH v2] introduce grant copy for user land

[Thanos Makatos]

> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 05 December 2014 6:06 PM
> To: Thanos Makatos; xen-devel@lists.xenproject.org
> Cc: David Vrabel
> Subject: Re: [Xen-devel] [PATCH v2] introduce grant copy for user land
> 
> On 12/02/2014 11:13 AM, Thanos Makatos wrote:
> > This patch introduces the interface to allow user-space applications
> > execute grant-copy operations. This is done by sending an ioctl to the
> > grant device.
> >
> > Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
> > ---
> >   drivers/xen/gntdev.c      |  171
> +++++++++++++++++++++++++++++++++++++++++++++
> >   include/uapi/xen/gntdev.h |   69 ++++++++++++++++++
> >   2 files changed, 240 insertions(+)
> >
> > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> > index 51f4c95..7b4a8e0 100644
> > --- a/drivers/xen/gntdev.c
> > +++ b/drivers/xen/gntdev.c
> > @@ -705,6 +705,174 @@ static long gntdev_ioctl_notify(struct gntdev_priv
> *priv, void __user *u)
> >   	return rc;
> >   }
> >
> > +static int gntdev_gcopy_batch(int nr_segments, unsigned long gcopy_cb,
> > +		struct gntdev_grant_copy_segment __user *__segments,
> int dir,
> > +		int src, int dst) {
> > +
> > +	static const int batch_size = PAGE_SIZE / (sizeof(struct page*) +
> > +		sizeof(struct gnttab_copy) + sizeof(struct
> gntdev_grant_copy_segment));
> > +	struct page **pages = (struct page **)gcopy_cb;
> > +	struct gnttab_copy *batch = (struct gnttab_copy *)((unsigned
> long)pages +
> > +			sizeof(struct page*) * batch_size);
> > +	struct gntdev_grant_copy_segment *segments =
> > +		(struct gntdev_grant_copy_segment *)((unsigned
> long)batch +
> > +				sizeof(struct gnttab_copy) * batch_size);
> > +	unsigned int nr_pinned = 0, nr_segs2cp = 0;
> > +	int err = 0, i;
> > +	const int write = dir == GNTCOPY_IOCTL_g2s;
> > +
> > +	nr_segments = min(nr_segments, batch_size);
> > +
> > +	if (unlikely(copy_from_user(segments, __segments,
> > +			   sizeof(struct gntdev_grant_copy_segment) *
> nr_segments))) {
> > +		pr_debug("failed to copy %d segments from user",
> nr_segments);
> > +		err = -EFAULT;
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < nr_segments; i++) {
> > +
> > +		xen_pfn_t pgaddr;
> > +		unsigned long start, offset;
> > +		struct gntdev_grant_copy_segment *seg = &segments[i];
> > +
> > +		if (dir == GNTCOPY_IOCTL_s2g || dir ==
> GNTCOPY_IOCTL_g2s) {
> > +
> > +			start = (unsigned long)seg->self.iov.iov_base &
> PAGE_MASK;
> > +			offset = (unsigned long)seg->self.iov.iov_base &
> ~PAGE_MASK;
> > +			if (unlikely(offset + seg->self.iov.iov_len >
> PAGE_SIZE)) {
> > +				pr_warn("segments crossing page
> boundaries not yet "
> > +						"implemented\n");
> > +				err = -ENOSYS;
> > +				goto out;
> > +			}
> > +
> > +			err = get_user_pages_fast(start, 1, write, &pages[i]);
> > +			if (unlikely(err != 1)) {
> > +				pr_debug("failed to get user page %lu",
> start);
> > +				err = -EFAULT;
> > +				goto out;
> > +			}
> > +
> > +			nr_pinned++;
> > +
> > +			pgaddr = pfn_to_mfn(page_to_pfn(pages[i]));
> > +		}
> > +
> > +		nr_segs2cp++;
> > +
> > +		switch (dir) {
> > +		case GNTCOPY_IOCTL_g2s: /* copy from guest */
> > +			batch[i].len = seg->self.iov.iov_len;
> > +			batch[i].source.u.ref = seg->self.ref;
> > +			batch[i].source.domid = src;
> > +			batch[i].source.offset = seg->self.offset;
> > +			batch[i].dest.u.gmfn = pgaddr;
> > +			batch[i].dest.domid = DOMID_SELF;
> > +			batch[i].dest.offset = offset;
> > +			batch[i].flags = GNTCOPY_source_gref;
> > +			break;
> > +		case GNTCOPY_IOCTL_s2g: /* copy to guest */
> > +			batch[i].len = seg->self.iov.iov_len;
> > +			batch[i].source.u.gmfn = pgaddr;
> > +			batch[i].source.domid = DOMID_SELF;
> > +			batch[i].source.offset = offset;
> > +			batch[i].dest.u.ref = seg->self.ref;
> > +			batch[i].dest.domid = dst;
> > +			batch[i].dest.offset = seg->self.offset;
> > +			batch[i].flags = GNTCOPY_dest_gref;
> > +			break;
> > +		case GNTCOPY_IOCTL_g2g: /* copy guest to guest */
> > +			batch[i].len = seg->g2g.len;
> > +			batch[i].source.u.ref = seg->g2g.src.ref;
> > +			batch[i].source.domid = src;
> > +			batch[i].source.offset = seg->g2g.src.offset;
> > +			batch[i].dest.u.ref = seg->g2g.dst.ref;
> > +			batch[i].dest.domid = dst;
> > +			batch[i].dest.offset = seg->g2g.dst.offset;
> > +			batch[i].flags = GNTCOPY_source_gref |
> GNTCOPY_dest_gref;
> > +			break;
> > +		default:
> > +			pr_debug("invalid grant-copy direction %d\n", dir);
> > +			err = -EINVAL;
> > +			goto out;
> > +		}
> > +	}
> > +
> > +	gnttab_batch_copy(batch, nr_segs2cp);
> > +	for (i = 0; i < nr_segs2cp; i++) {
> 
> Can nr_segs2cp be not equal to nr_segments here? If you got to this
> point you have gone through the full loop.

Correct.

> > +		err = put_user(batch[i].status, &__segments[i].status);
> > +		if (unlikely(err)) {
> > +			pr_debug("failed to copy error code %d to
> user: %d\n",
> > +					batch[i].status, err);
> > +			goto out;
> > +		}
> > +	}
> > +
> > +out:
> > +	for (i = 0; i < nr_pinned; i++)
> > +		put_page(pages[i]);
> > +
> > +	if (unlikely(err))
> > +		return err;
> > +
> > +	return nr_segs2cp;
> 
> And I think here it can be either 0 (which is the case of an error) or
> nr_segments. If you error out of the 'for' loop you haven't actually
> copied anything, even though nr_segs2cp might be non-zero.

If an error has occurred, "err" will be returned so the value of either "nr_segs2cp" or "nr_segments" is irrelevant. If no error occurs, then we have copied "nr_segments", therefore this is what we should return. I think I got something wrong and I thought there was some corner case so that why we needed an extra variable, but it doesn't seem to be the case. I'll replace "nr_segments" with "nr_segments".

> -boris
>