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

Re: [Xen-devel] [PATCH v4 09/14] libxc/save: adjust the memory allocation for migration





On 05/12/2015 08:14 PM, Andrew Cooper wrote:
On 12/05/15 12:25, Yang Hongyang wrote:
Move the memory allocation before the concrete live/nolive save
in order to avoid the free/alloc memory loop when using Remus.

Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
  tools/libxc/xc_sr_common.h |  1 +
  tools/libxc/xc_sr_save.c   | 61 ++++++++++++++++++++++------------------------
  2 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
index c0f90d4..276d00a 100644
--- a/tools/libxc/xc_sr_common.h
+++ b/tools/libxc/xc_sr_common.h
@@ -190,6 +190,7 @@ struct xc_sr_context
              unsigned nr_batch_pfns;
              unsigned long *deferred_pages;
              unsigned long nr_deferred_pages;
+            xc_hypercall_buffer_t dirty_bitmap_hbuf;
          } save;

          struct /* Restore data. */
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 368aacb..0953c35 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -475,24 +475,12 @@ static int update_progress_string(struct xc_sr_context 
*ctx,
  static int send_domain_memory_live(struct xc_sr_context *ctx)
  {
      xc_interface *xch = ctx->xch;
-    DECLARE_HYPERCALL_BUFFER(unsigned long, dirty_bitmap);
      xc_shadow_op_stats_t stats = { 0, ctx->save.p2m_size };
      char *progress_str = NULL;
      unsigned x;
      int rc = -1;
-
-    dirty_bitmap = xc_hypercall_buffer_alloc_pages(
-        xch, dirty_bitmap, NRPAGES(bitmap_size(ctx->save.p2m_size)));
-
-    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
-                                  sizeof(*ctx->save.batch_pfns));
-    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
-    if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
-    {
-        ERROR("Unable to allocate memory for to_{send,fix}/batch bitmaps");
-        goto out;
-    }
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    (&ctx->save.dirty_bitmap_hbuf));

You can drop this extra bracketing now that
DECLARE_HYPERCALL_BUFFER_SHADOW() has been fixed.

Ok, will drop the bracketing in all DECLARE_HYPERCALL_BUFFER_SHADOW() calls



      rc = enable_logdirty(ctx);
      if ( rc )
@@ -593,10 +581,6 @@ static int send_domain_memory_live(struct xc_sr_context 
*ctx)
    out:
      xc_set_progress_prefix(xch, NULL);
      free(progress_str);
-    xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
-                                   NRPAGES(bitmap_size(ctx->save.p2m_size)));
-    free(ctx->save.deferred_pages);
-    free(ctx->save.batch_pfns);
      return rc;
  }

@@ -609,17 +593,6 @@ static int send_domain_memory_nonlive(struct xc_sr_context 
*ctx)
      xc_interface *xch = ctx->xch;
      int rc = -1;

-    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
-                                  sizeof(*ctx->save.batch_pfns));
-    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
-
-    if ( !ctx->save.batch_pfns || !ctx->save.deferred_pages )
-    {
-        PERROR("Failed to allocate memory for nonlive tracking structures");
-        errno = ENOMEM;
-        goto err;
-    }
-
      rc = suspend_domain(ctx);
      if ( rc )
          goto err;
@@ -631,15 +604,30 @@ static int send_domain_memory_nonlive(struct 
xc_sr_context *ctx)
          goto err;

   err:
-    free(ctx->save.deferred_pages);
-    free(ctx->save.batch_pfns);
-
      return rc;
  }

  static int setup(struct xc_sr_context *ctx)
  {
+    xc_interface *xch = ctx->xch;
      int rc;
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    (&ctx->save.dirty_bitmap_hbuf));
+
+    dirty_bitmap = xc_hypercall_buffer_alloc_pages(
+                   xch, dirty_bitmap, 
NRPAGES(bitmap_size(ctx->save.p2m_size)));
+    ctx->save.batch_pfns = malloc(MAX_BATCH_SIZE *
+                                  sizeof(*ctx->save.batch_pfns));
+    ctx->save.deferred_pages = calloc(1, bitmap_size(ctx->save.p2m_size));
+
+    if ( !ctx->save.batch_pfns || !dirty_bitmap || !ctx->save.deferred_pages )
+    {
+        ERROR("Unable to allocate memory for dirty bitmaps, batch pfns and"
+              " deferred pages");
+        rc = -1;
+        errno = ENOMEM;
+        goto err;
+    }

      rc = ctx->save.ops.setup(ctx);
      if ( rc )
@@ -655,12 +643,21 @@ static int setup(struct xc_sr_context *ctx)
  static void cleanup(struct xc_sr_context *ctx)
  {
      xc_interface *xch = ctx->xch;
+    DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
+                                    (&ctx->save.dirty_bitmap_hbuf));

And also drop this bracketing.

+

      xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
                        NULL, 0, NULL, 0, NULL);

      if ( ctx->save.ops.cleanup(ctx) )
          PERROR("Failed to clean up");
+
+    xc_hypercall_buffer_free_pages(xch, dirty_bitmap,
+                                   NRPAGES(bitmap_size(ctx->save.p2m_size)));
+    free(ctx->save.deferred_pages);
+    free(ctx->save.batch_pfns);
+

Stray blank line.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

  }

  /*

.


--
Thanks,
Yang.

_______________________________________________
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®.