Re: [PATCH v6 3/3] tools/libxc: use superpages during restore of HVM guest

[Olaf Hering <olaf@aepfle.de>]

[-- Attachment #1.1: Type: text/plain, Size: 5795 bytes --]

On Sat, Aug 26, Olaf Hering wrote:

> +static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count,

> +    /*
> +     * Scan the entire superpage because several batches will fit into
> +     * a superpage, and it is unknown which pfn triggered the allocation.
> +     */
> +    order = SUPERPAGE_1GB_SHIFT;
> +    pfn = min_pfn = (min_pfn >> order) << order;

Scanning an entire superpage again and again looked expensive, but with
the debug change below it turned out that the loop which peeks at each
single bit in populated_pfns is likely not a bootleneck.

Migrating a domU with a simple workload that touches pages to mark them
dirty will set the min_pfn/max_pfn to a large range anyway after the
first iteration. This large range may also happen with an idle domU. A
small domU takes 78 seconds to migrate, and just the freeing part takes
1.4 seconds. Similar for a large domain, the loop takes 1% of the time.

     78 seconds, 1.4 seconds, 2119 calls  (8GB, 12*512M memdirty)
    695 seconds, 7.6 seconds, 18076 calls (72GB, 12*5G memdirty)

Olaf

    track time spent if decrease_reservation is needed

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index 0fa0fbea4d..5ec8b6fee6 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -353,6 +353,9 @@ struct xc_sr_context
                     struct xc_sr_bitmap attempted_1g;
                     struct xc_sr_bitmap attempted_2m;
                     struct xc_sr_bitmap allocated_pfns;
+
+                    unsigned long tv_nsec;
+                    unsigned long iterations;
                 } restore;
             };
         } x86_hvm;
diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c
index 8cd9289d1a..f6aad329e2 100644
--- a/tools/libxc/xc_sr_restore.c
+++ b/tools/libxc/xc_sr_restore.c
@@ -769,6 +769,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     {
         ctx.restore.ops = restore_ops_x86_hvm;
         if ( restore(&ctx) )
+            ;
             return -1;
     }
     else
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index 2b0eca0c7c..11758b3f7d 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <arpa/inet.h>
+#include <time.h>
 
 #include "xc_sr_common_x86.h"
 
@@ -248,6 +249,12 @@ static int x86_hvm_stream_complete(struct xc_sr_context *ctx)
 
 static int x86_hvm_cleanup(struct xc_sr_context *ctx)
 {
+    xc_interface *xch = ctx->xch;
+    errno = 0;
+    PERROR("tv_nsec %lu.%lu iterations %lu",
+            ctx->x86_hvm.restore.tv_nsec / 1000000000UL,
+            ctx->x86_hvm.restore.tv_nsec % 1000000000UL,
+            ctx->x86_hvm.restore.iterations);
     free(ctx->x86_hvm.restore.context);
     xc_sr_bitmap_free(&ctx->x86_hvm.restore.attempted_1g);
     xc_sr_bitmap_free(&ctx->x86_hvm.restore.attempted_2m);
@@ -440,6 +447,28 @@ static int x86_hvm_allocate_pfn(struct xc_sr_context *ctx, xen_pfn_t pfn)
     return rc;
 }
 
+static void diff_timespec(struct xc_sr_context *ctx, const struct timespec *old, const struct timespec *new, struct timespec *diff)
+{
+    xc_interface *xch = ctx->xch;
+    if (new->tv_sec == old->tv_sec && new->tv_nsec == old->tv_nsec)
+        PERROR("%s: time did not move: %ld/%ld == %ld/%ld", __func__, old->tv_sec, old->tv_nsec, new->tv_sec, new->tv_nsec);
+    if ( (new->tv_sec < old->tv_sec) || (new->tv_sec == old->tv_sec && new->tv_nsec < old->tv_nsec) )
+    {
+        PERROR("%s: time went backwards: %ld/%ld -> %ld/%ld", __func__, old->tv_sec, old->tv_nsec, new->tv_sec, new->tv_nsec);
+        diff->tv_sec = diff->tv_nsec = 0;
+        return;
+    }
+    if ((new->tv_nsec - old->tv_nsec) < 0) {
+        diff->tv_sec = new->tv_sec - old->tv_sec - 1;
+        diff->tv_nsec = new->tv_nsec - old->tv_nsec + 1000000000UL;
+    } else {
+        diff->tv_sec = new->tv_sec - old->tv_sec;
+        diff->tv_nsec = new->tv_nsec - old->tv_nsec;
+    }
+    if (diff->tv_sec < 0)
+        PERROR("%s: time diff broken. old: %ld/%ld new: %ld/%ld diff: %ld/%ld ", __func__, old->tv_sec, old->tv_nsec, new->tv_sec, new->tv_nsec, diff->tv_sec, diff->tv_nsec);
+}
+
 static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count,
                                  const xen_pfn_t *original_pfns,
                                  const uint32_t *types)
@@ -448,6 +477,7 @@ static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count,
     xen_pfn_t pfn, min_pfn = original_pfns[0], max_pfn = original_pfns[0];
     unsigned i, freed = 0, order;
     int rc = -1;
+    struct timespec a, b, d;
 
     for ( i = 0; i < count; ++i )
     {
@@ -474,6 +504,8 @@ static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count,
         }
     }
 
+    if (clock_gettime(CLOCK_MONOTONIC, &a))
+        PERROR("clock_gettime start");
     /*
      * Scan the entire superpage because several batches will fit into
      * a superpage, and it is unknown which pfn triggered the allocation.
@@ -504,10 +536,17 @@ static int x86_hvm_populate_pfns(struct xc_sr_context *ctx, unsigned count,
         }
         pfn++;
     }
-    if ( freed )
+    if ( 0 && freed )
         DPRINTF("freed %u between %" PRI_xen_pfn " %" PRI_xen_pfn "\n",
                 freed, min_pfn, max_pfn);
 
+    if (clock_gettime(CLOCK_MONOTONIC, &b))
+        PERROR("clock_gettime end");
+
+    diff_timespec(ctx, &a, &b, &d);
+    ctx->x86_hvm.restore.tv_nsec += d.tv_nsec + (1000000000UL * d.tv_sec);
+    ctx->x86_hvm.restore.iterations++;
+
     rc = 0;
 
  err:

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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