[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCHv1] grant-table: defer releasing pages acquired in a grant copy



Acquiring a page for the source or destination of a grant copy is an
expensive operation.  A common use case is for two adjacent grant copy
ops to operate on either the same source or the same destination page.

Instead of always acquiring and releasing destination and source pages
for each operation, release the page once it is no longer valid for
the next op.

If either the source or destination domains changes both pages are
released as it is unlikely that either will still be valid.

Netback uses grant copies in its guest Rx path and initial performance
measurements show significant gains in guest Rx from off-host (e.g.,
from 7.2 Gbit/s to 9.6 Gbit/s).  This improves to 11 Gbit/s if netback
fully coaleceses to guest packets into the least number of ring slots.

Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
 xen/common/grant_table.c         |  347 +++++++++++++++++++++++++++-----------
 xen/include/public/grant_table.h |    2 +-
 2 files changed, 245 insertions(+), 104 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index fe52b63..6db46b4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2077,152 +2077,293 @@ __acquire_grant_for_copy(
     return rc;
 }
 
-static void
-__gnttab_copy(
-    struct gnttab_copy *op)
+struct gnttab_copy_buf {
+    /* guest provided. */
+    domid_t domid;
+    bool_t is_ref;
+    union {
+        grant_ref_t ref;
+        xen_pfn_t gfn;
+    } u;
+    unsigned int offset;
+    unsigned int len;
+
+    /* mapped etc. */
+    struct domain *domain;
+    unsigned long frame;
+    struct page_info *page;
+    void *virt;
+    bool_t have_grant;
+    bool_t have_type;
+};
+
+static int gnttab_copy_lock_domain(struct gnttab_copy *op,
+                                   domid_t domid, unsigned int gref_flag,
+                                   struct gnttab_copy_buf *buf)
 {
-    struct domain *sd = NULL, *dd = NULL;
-    unsigned long s_frame, d_frame;
-    struct page_info *s_pg = NULL, *d_pg = NULL;
-    char *sp, *dp;
-    s16 rc = GNTST_okay;
-    int have_d_grant = 0, have_s_grant = 0;
-    int src_is_gref, dest_is_gref;
+    s16 rc;
 
-    if ( ((op->source.offset + op->len) > PAGE_SIZE) ||
-         ((op->dest.offset + op->len) > PAGE_SIZE) )
-        PIN_FAIL(error_out, GNTST_bad_copy_arg, "copy beyond page area.\n");
+    if ( domid != DOMID_SELF && !(op->flags & gref_flag) )
+        PIN_FAIL(out, GNTST_permission_denied,
+                 "only allow copy-by-mfn for DOMID_SELF.\n");
 
-    src_is_gref = op->flags & GNTCOPY_source_gref;
-    dest_is_gref = op->flags & GNTCOPY_dest_gref;
+    if ( domid == DOMID_SELF )
+        buf->domain = rcu_lock_current_domain();
+    else 
+    {
+        buf->domain = rcu_lock_domain_by_id(domid);
+        if ( buf->domain == NULL )
+            PIN_FAIL(out, GNTST_bad_domain,
+                     "couldn't find %d\n", op->source.domid);
+    }
 
-    if ( (op->source.domid != DOMID_SELF && !src_is_gref ) ||
-         (op->dest.domid   != DOMID_SELF && !dest_is_gref)   )
-        PIN_FAIL(error_out, GNTST_permission_denied,
-                 "only allow copy-by-mfn for DOMID_SELF.\n");
+    buf->domid = domid;
+    rc = GNTST_okay;
+out:
+    return rc;
+}
 
-    if ( op->source.domid == DOMID_SELF )
-        sd = rcu_lock_current_domain();
-    else if ( (sd = rcu_lock_domain_by_id(op->source.domid)) == NULL )
-        PIN_FAIL(error_out, GNTST_bad_domain,
-                 "couldn't find %d\n", op->source.domid);
+static void gnttab_copy_unlock_domains(struct gnttab_copy_buf *src,
+                                       struct gnttab_copy_buf *dest)
+{
+    if ( src->domain )
+    {
+        rcu_unlock_domain(src->domain);
+        src->domain = NULL;
+    }
+    if ( dest->domain ) {
+        rcu_unlock_domain(dest->domain);
+        dest->domain = NULL;
+    }
+}
 
-    if ( op->dest.domid == DOMID_SELF )
-        dd = rcu_lock_current_domain();
-    else if ( (dd = rcu_lock_domain_by_id(op->dest.domid)) == NULL )
-        PIN_FAIL(error_out, GNTST_bad_domain,
-                 "couldn't find %d\n", op->dest.domid);
+static s16 gnttab_copy_lock_domains(struct gnttab_copy *op,
+                                    struct gnttab_copy_buf *src,
+                                    struct gnttab_copy_buf *dest)
+{
+    s16 rc;
 
-    rc = xsm_grant_copy(XSM_HOOK, sd, dd);
-    if ( rc )
+    rc = gnttab_copy_lock_domain(op, op->source.domid, GNTCOPY_source_gref, 
src);
+    if ( rc < 0 )
+        return rc;
+    rc = gnttab_copy_lock_domain(op, op->dest.domid, GNTCOPY_dest_gref, dest);
+    if ( rc < 0 )
+        return rc;
+    
+    rc = xsm_grant_copy(XSM_HOOK, src->domain, dest->domain);
+    if ( rc < 0 )
     {
-        rc = GNTST_permission_denied;
-        goto error_out;
+        gnttab_copy_unlock_domains(src, dest);
+        return GNTST_permission_denied;
     }
 
-    if ( src_is_gref )
+    return 0;
+}
+
+static void gnttab_copy_release_buf(struct gnttab_copy_buf *buf, bool_t 
read_only)
+{
+    if ( buf->virt )
     {
-        unsigned source_off, source_len;
-        rc = __acquire_grant_for_copy(sd, op->source.u.ref,
-                                      current->domain->domain_id, 1,
-                                      &s_frame, &s_pg,
-                                      &source_off, &source_len, 1);
-        if ( rc != GNTST_okay )
-            goto error_out;
-        have_s_grant = 1;
-        if ( op->source.offset < source_off ||
-             op->len > source_len )
-            PIN_FAIL(error_out, GNTST_general_error,
-                     "copy source out of bounds: %d < %d || %d > %d\n",
-                     op->source.offset, source_off,
-                     op->len, source_len);
+        unmap_domain_page(buf->virt);
+        buf->virt = NULL;
     }
-    else
+    if ( buf->have_type )
     {
-        rc = __get_paged_frame(op->source.u.gmfn, &s_frame, &s_pg, 1, sd);
-        if ( rc != GNTST_okay )
-            PIN_FAIL(error_out, rc,
-                     "source frame %lx invalid.\n", s_frame);
+        put_page_type(buf->page);
+        buf->have_type = 0;
     }
+    if ( buf->page )
+    {
+        put_page(buf->page);
+        buf->page = NULL;
+    }
+    if ( buf->have_grant )
+    {
+        __release_grant_for_copy(buf->domain, buf->u.ref, read_only);
+        buf->have_grant = 0;
+    }
+}
+
+static s16 gnttab_copy_claim_buf(
+    struct gnttab_copy *op,
+    struct gnttab_copy_ptr *ptr,
+    struct gnttab_copy_buf *buf,
+    bool_t read_only)
+{
+    unsigned int is_gref;
+    s16 rc;
 
-    if ( dest_is_gref )
+    is_gref = op->flags & (read_only ? GNTCOPY_source_gref : 
GNTCOPY_dest_gref);
+
+    if ( is_gref )
     {
-        unsigned dest_off, dest_len;
-        rc = __acquire_grant_for_copy(dd, op->dest.u.ref,
-                                      current->domain->domain_id, 0,
-                                      &d_frame, &d_pg, &dest_off, &dest_len, 
1);
+        rc = __acquire_grant_for_copy(buf->domain, ptr->u.ref,
+                                      current->domain->domain_id, read_only,
+                                      &buf->frame, &buf->page,
+                                      &buf->offset, &buf->len, 1);
         if ( rc != GNTST_okay )
-            goto error_out;
-        have_d_grant = 1;
-        if ( op->dest.offset < dest_off ||
-             op->len > dest_len )
-            PIN_FAIL(error_out, GNTST_general_error,
-                     "copy dest out of bounds: %d < %d || %d > %d\n",
-                     op->dest.offset, dest_off,
-                     op->len, dest_len);
+            goto out;
+        buf->u.ref = ptr->u.ref;
+        buf->have_grant = 1;
     }
     else
     {
-        rc = __get_paged_frame(op->dest.u.gmfn, &d_frame, &d_pg, 0, dd);
+        rc = __get_paged_frame(ptr->u.gmfn, &buf->frame, &buf->page, 1, 
buf->domain);
         if ( rc != GNTST_okay )
-            PIN_FAIL(error_out, rc,
-                     "destination frame %lx invalid.\n", d_frame);
+            PIN_FAIL(out, rc,
+                     "source frame %lx invalid.\n", ptr->u.gmfn);
+
+        buf->u.gfn = ptr->u.gmfn;
+        buf->offset = 0;
+        buf->len = PAGE_SIZE;
     }
 
-    if ( !get_page_type(d_pg, PGT_writable_page) )
+    if ( !read_only )
     {
-        if ( !dd->is_dying )
-            gdprintk(XENLOG_WARNING, "Could not get dst frame %lx\n", d_frame);
-        rc = GNTST_general_error;
-        goto error_out;
+        if ( !get_page_type(buf->page, PGT_writable_page) )
+        {
+            if ( !buf->domain->is_dying )
+                gdprintk(XENLOG_WARNING, "Could not get writable frame %lx\n", 
buf->frame);
+            rc = GNTST_general_error;
+            goto out;
+        }
+        buf->have_type = 1;
     }
 
-    sp = map_domain_page(s_frame);
-    dp = map_domain_page(d_frame);
+    buf->virt = map_domain_page(buf->frame);
+    rc = GNTST_okay;
 
-    memcpy(dp + op->dest.offset, sp + op->source.offset, op->len);
+out:
+    return rc;
+}
 
-    unmap_domain_page(dp);
-    unmap_domain_page(sp);
+static bool_t gnttab_copy_buf_valid(struct gnttab_copy *op,
+                                    struct gnttab_copy_ptr *p,
+                                    struct gnttab_copy_buf *b,
+                                    unsigned int gref_flag)
+{
+    if ( !b->virt )
+        return 0;
+    if ( op->flags & gref_flag )
+        return b->have_grant && p->u.ref == b->u.ref;
+    else
+        return p->u.gmfn == b->u.gfn;
+}
 
-    gnttab_mark_dirty(dd, d_frame);
+static int gnttab_copy_buf(struct gnttab_copy *op,
+                           struct gnttab_copy_buf *dest,
+                           struct gnttab_copy_buf *src)
+{
+    s16 rc;
 
-    put_page_type(d_pg);
- error_out:
-    if ( d_pg )
-        put_page(d_pg);
-    if ( s_pg )
-        put_page(s_pg);
-    if ( have_s_grant )
-        __release_grant_for_copy(sd, op->source.u.ref, 1);
-    if ( have_d_grant )
-        __release_grant_for_copy(dd, op->dest.u.ref, 0);
-    if ( sd )
-        rcu_unlock_domain(sd);
-    if ( dd )
-        rcu_unlock_domain(dd);
-    op->status = rc;
+    if ( ((op->source.offset + op->len) > PAGE_SIZE) ||
+         ((op->dest.offset + op->len) > PAGE_SIZE) )
+        PIN_FAIL(out, GNTST_bad_copy_arg, "copy beyond page area.\n");
+
+    if ( op->source.offset < src->offset ||
+         op->source.offset + op->len > src->offset + src->len )
+        PIN_FAIL(out, GNTST_general_error,
+                 "copy source out of bounds: %d < %d || %d > %d\n",
+                 op->source.offset, src->offset,
+                 op->len, src->len);
+
+    if ( op->dest.offset < dest->offset ||
+         op->dest.offset + op->len > dest->offset + dest->len )
+        PIN_FAIL(out, GNTST_general_error,
+                 "copy dest out of bounds: %d < %d || %d > %d\n",
+                 op->dest.offset, dest->offset,
+                     op->len, dest->len);
+
+    memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset, 
op->len);
+    rc = GNTST_okay;
+out:
+    return rc;
 }
 
-static long
-gnttab_copy(
+static s16 gnttab_copy_one(struct gnttab_copy *op,
+                           struct gnttab_copy_buf *dest,
+                           struct gnttab_copy_buf *src)
+{
+    s16 rc;
+
+    if ( !src->domain || op->source.domid != src->domid
+         || !dest->domain || op->dest.domid != dest->domid )
+    {
+        gnttab_copy_release_buf(src, 1);
+        gnttab_copy_release_buf(dest, 0);
+        gnttab_copy_unlock_domains(src, dest);
+
+        rc = gnttab_copy_lock_domains(op, src, dest);
+        if ( rc < 0 )
+            goto out;
+    }
+
+    /* Different source? */
+    if ( !gnttab_copy_buf_valid(op, &op->source, src, GNTCOPY_source_gref) )
+    {
+        gnttab_copy_release_buf(src, 1);
+        rc = gnttab_copy_claim_buf(op, &op->source, src, 1);
+        if ( rc < 0 )
+            goto out;
+    }
+
+    /* Different dest? */
+    if ( !gnttab_copy_buf_valid(op, &op->dest, dest, GNTCOPY_dest_gref) )
+    {
+        gnttab_copy_release_buf(dest, 0);
+        rc = gnttab_copy_claim_buf(op, &op->dest, dest, 0);
+        if ( rc < 0 )
+            goto out;
+    }
+        
+    rc = gnttab_copy_buf(op, dest, src);
+out:
+    return rc;
+}
+
+static long gnttab_copy(
     XEN_GUEST_HANDLE_PARAM(gnttab_copy_t) uop, unsigned int count)
 {
-    int i;
+    unsigned int i;
     struct gnttab_copy op;
+    struct gnttab_copy_buf src = { 0, };
+    struct gnttab_copy_buf dest = { 0, };
+    long rc = 0;
 
     for ( i = 0; i < count; i++ )
     {
         if (i && hypercall_preempt_check())
-            return i;
+        {
+            rc = i;
+            break;
+        }
+
         if ( unlikely(__copy_from_guest(&op, uop, 1)) )
-            return -EFAULT;
-        __gnttab_copy(&op);
+        {
+            rc = -EFAULT;
+            break;
+        }
+
+        op.status = gnttab_copy_one(&op, &dest, &src);
+        if ( op.status != GNTST_okay )
+        {
+            gnttab_copy_release_buf(&src, 1);
+            gnttab_copy_release_buf(&dest, 0);
+        }
+
         if ( unlikely(__copy_field_to_guest(uop, &op, status)) )
-            return -EFAULT;
+        {
+            rc = -EFAULT;
+            break;
+        }
         guest_handle_add_offset(uop, 1);
     }
-    return 0;
+
+    gnttab_copy_release_buf(&src, 1);
+    gnttab_copy_release_buf(&dest, 0);
+    gnttab_copy_unlock_domains(&src, &dest);
+
+    return rc;
 }
 
 static long
diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
index 20d4e77..c8619ed 100644
--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -453,7 +453,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
 
 struct gnttab_copy {
     /* IN parameters. */
-    struct {
+    struct gnttab_copy_ptr {
         union {
             grant_ref_t ref;
             xen_pfn_t   gmfn;
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.