[Xen-devel] [PATCH 3/3] grant-table: defer releasing pages acquired in a grant copy

[David Vrabel <david.vrabel@xxxxxxxxxx>]

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.

XenServer's performance benchmarks show modest improvements in network
receive throughput (netback uses grant copy in the guest Rx path) and
no regressions in disk performance (using tapdisk3 which grant copies
as the backend).

                         Baseline   Deferred Release
Interhost receive to VM   7.2 Gb/s  ~9 Gbit/s
Interhost aggregate      24 Gb/s    28 Gb/s
Intrahost single stream  14 Gb/s    14 Gb/s
Intrahost aggregate      34 Gb/s    36 Gb/s
Aggregate disk write    900 MB/s   900 MB/s
Aggregate disk read     890 MB/s   890 MB/s

Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
Reviewed-by: Tim Deegan <tim@xxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/common/grant_table.c |   83 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 66 insertions(+), 17 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f5b25bc..e44fe20 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2236,6 +2236,17 @@ static int gnttab_copy_claim_buf(const struct 
gnttab_copy *op,
     return rc;
 }
 
+static bool_t gnttab_copy_buf_valid(const struct gnttab_copy_ptr *p,
+                                    const struct gnttab_copy_buf *b,
+                                    bool_t has_gref)
+{
+    if ( !b->virt )
+        return 0;
+    if ( has_gref )
+        return b->have_grant && p->u.ref == b->ptr.u.ref;
+    return p->u.gmfn == b->ptr.u.gmfn;
+}
+
 static int gnttab_copy_buf(const struct gnttab_copy *op,
                            struct gnttab_copy_buf *dest,
                            const struct gnttab_copy_buf *src)
@@ -2274,23 +2285,40 @@ static int gnttab_copy_one(const struct gnttab_copy *op,
 {
     int rc;
 
-    rc = gnttab_copy_lock_domains(op, src, dest);
-    if ( rc < 0 )
-        goto out;
+    if ( !src->domain || op->source.domid != src->ptr.domid ||
+         !dest->domain || op->dest.domid != dest->ptr.domid )
+    {
+        gnttab_copy_release_buf(src);
+        gnttab_copy_release_buf(dest);
+        gnttab_copy_unlock_domains(src, dest);
 
-    rc = gnttab_copy_claim_buf(op, &op->source, src, GNTCOPY_source_gref);
-    if ( rc < 0 )
-        goto out;
+        rc = gnttab_copy_lock_domains(op, src, dest);
+        if ( rc < 0 )
+            goto out;
+    }
 
-    rc = gnttab_copy_claim_buf(op, &op->dest, dest, GNTCOPY_dest_gref);
-    if ( rc < 0 )
-        goto out;
+    /* Different source? */
+    if ( !gnttab_copy_buf_valid(&op->source, src,
+                                op->flags & GNTCOPY_source_gref) )
+    {
+        gnttab_copy_release_buf(src);
+        rc = gnttab_copy_claim_buf(op, &op->source, src, GNTCOPY_source_gref);
+        if ( rc < 0 )
+            goto out;
+    }
+
+    /* Different dest? */
+    if ( !gnttab_copy_buf_valid(&op->dest, dest,
+                                op->flags & GNTCOPY_dest_gref) )
+    {
+        gnttab_copy_release_buf(dest);
+        rc = gnttab_copy_claim_buf(op, &op->dest, dest, GNTCOPY_dest_gref);
+        if ( rc < 0 )
+            goto out;
+    }
 
     rc = gnttab_copy_buf(op, dest, src);
  out:
-    gnttab_copy_release_buf(src);
-    gnttab_copy_release_buf(dest);
-    gnttab_copy_unlock_domains(src, dest);
     return rc;
 }
 
@@ -2301,21 +2329,42 @@ static long gnttab_copy(
     struct gnttab_copy op;
     struct gnttab_copy_buf src = {};
     struct gnttab_copy_buf dest = {};
+    long rc = 0;
 
     for ( i = 0; i < count; i++ )
     {
-        if (i && hypercall_preempt_check())
-            return i;
+        if ( i && hypercall_preempt_check() )
+        {
+            rc = i;
+            break;
+        }
+
         if ( unlikely(__copy_from_guest(&op, uop, 1)) )
-            return -EFAULT;
+        {
+            rc = -EFAULT;
+            break;
+        }
 
         op.status = gnttab_copy_one(&op, &dest, &src);
+        if ( op.status != GNTST_okay )
+        {
+            gnttab_copy_release_buf(&src);
+            gnttab_copy_release_buf(&dest);
+        }
 
         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);
+    gnttab_copy_release_buf(&dest);
+    gnttab_copy_unlock_domains(&src, &dest);
+
+    return rc;
 }
 
 static long
-- 
1.7.10.4


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