[Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
justify passing precopy_stats by value.

Under no circumstances can the precopy callback ever be executing in a
separate address space.

Switch the callback to passing by pointer which is far more efficient, and
drop the typedef (because none of the other callback have this oddity).

This is no functional change, as there are no users of this interface yet.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

Appologies for this being late.  I did intend to get it sorted before code
freeze, but I was otherwise busy.
---
 tools/libxc/include/xenguest.h |  8 +-------
 tools/libxc/xc_sr_save.c       | 19 +++++++++----------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
index b4b2e19..f6a61ab 100644
--- a/tools/libxc/include/xenguest.h
+++ b/tools/libxc/include/xenguest.h
@@ -47,12 +47,6 @@ struct precopy_stats
     long dirty_count; /* -1 if unknown */
 };
 
-/*
- * A precopy_policy callback may not be running in the same address
- * space as libxc an so precopy_stats is passed by value.
- */
-typedef int (*precopy_policy_t)(struct precopy_stats, void *);
-
 /* callbacks provided by xc_domain_save */
 struct save_callbacks {
     /* Called after expiration of checkpoint interval,
@@ -72,7 +66,7 @@ struct save_callbacks {
 #define XGS_POLICY_CONTINUE_PRECOPY 0  /* Remain in the precopy phase. */
 #define XGS_POLICY_STOP_AND_COPY    1  /* Immediately suspend and transmit the
                                         * remaining dirty pages. */
-    precopy_policy_t precopy_policy;
+    int (*precopy_policy)(struct precopy_stats *, void *);
 
     /*
      * Called after the guest's dirty pages have been
diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c
index 5a40e58..beceb6a 100644
--- a/tools/libxc/xc_sr_save.c
+++ b/tools/libxc/xc_sr_save.c
@@ -478,11 +478,11 @@ static int update_progress_string(struct xc_sr_context 
*ctx, char **str)
 #define SPP_MAX_ITERATIONS      5
 #define SPP_TARGET_DIRTY_COUNT 50
 
-static int simple_precopy_policy(struct precopy_stats stats, void *user)
+static int simple_precopy_policy(struct precopy_stats *stats, void *user)
 {
-    return ((stats.dirty_count >= 0 &&
-            stats.dirty_count < SPP_TARGET_DIRTY_COUNT) ||
-            stats.iteration >= SPP_MAX_ITERATIONS)
+    return ((stats->dirty_count >= 0 &&
+             stats->dirty_count < SPP_TARGET_DIRTY_COUNT) ||
+            stats->iteration >= SPP_MAX_ITERATIONS)
         ? XGS_POLICY_STOP_AND_COPY
         : XGS_POLICY_CONTINUE_PRECOPY;
 }
@@ -502,7 +502,9 @@ static int send_memory_live(struct xc_sr_context *ctx)
     DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
                                     &ctx->save.dirty_bitmap_hbuf);
 
-    precopy_policy_t precopy_policy = ctx->save.callbacks->precopy_policy;
+    typeof(ctx->save.callbacks->precopy_policy) precopy_policy =
+        ctx->save.callbacks->precopy_policy ?: simple_precopy_policy;
+
     void *data = ctx->save.callbacks->data;
 
     struct precopy_stats *policy_stats;
@@ -515,14 +517,11 @@ static int send_memory_live(struct xc_sr_context *ctx)
         { .dirty_count   = ctx->save.p2m_size };
     policy_stats = &ctx->save.stats;
 
-    if ( precopy_policy == NULL )
-         precopy_policy = simple_precopy_policy;
-
     bitmap_set(dirty_bitmap, ctx->save.p2m_size);
 
     for ( ; ; )
     {
-        policy_decision = precopy_policy(*policy_stats, data);
+        policy_decision = precopy_policy(policy_stats, data);
         x++;
 
         if ( stats.dirty_count > 0 && policy_decision != XGS_POLICY_ABORT )
@@ -543,7 +542,7 @@ static int send_memory_live(struct xc_sr_context *ctx)
         policy_stats->total_written += policy_stats->dirty_count;
         policy_stats->dirty_count   = -1;
 
-        policy_decision = precopy_policy(*policy_stats, data);
+        policy_decision = precopy_policy(policy_stats, data);
 
         if ( policy_decision != XGS_POLICY_CONTINUE_PRECOPY )
            break;
-- 
2.1.4


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