[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 performamnce (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>
---
 xen/common/grant_table.c |   80 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 770403b..e9a7320 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2233,6 +2233,18 @@ static s16 gnttab_copy_claim_buf(
     return rc;
 }
 
+static bool_t gnttab_copy_buf_valid(const struct gnttab_copy *op,
+                                    const struct gnttab_copy_ptr *p,
+                                    const 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->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)
@@ -2269,23 +2281,38 @@ static s16 gnttab_copy_one(const struct gnttab_copy *op,
 {
     s16 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, &op->source, src, 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, &op->dest, dest, 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;
 }
 
@@ -2296,21 +2323,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;
+        {
+            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