[PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[Ian Campbell]

Otherwise users which deal with multiple domains need to do their own
locking and cannot save/migrate multiple domains in parallel (should
they want to).

Also made a bunch of cleanup along the way, mainly to make it easier
to figure out what was going on with the twisty maze of macros
redefining functions as macros and redefining the macros etc. I'm sure
there is plenty more straightening out which could be done but I don't
have the stomach for it this morning.

Shriram, does this have any impact on Remus?

[PATCH 1 of 8] libxc: save/restore: remove static context variables

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID c43bf40e2f29eb02c7dcf1a2317f243f1af5f659
# Parent  0727b2fcc33fd5cdf847fde0d2be261be3d6b279
libxc: save/restore: remove static context variables

20544:ad9d75d74bd5 and 20545:cc7d66ba0dad seemingly intended to change these
global static variables into stack variables but didn't remove the static
qualifier.

Also zero the entire struct once with memset rather than clearing fields
piecemeal in two different places.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 0727b2fcc33f -r c43bf40e2f29 tools/libxc/xc_domain_restore.c
--- a/tools/libxc/xc_domain_restore.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_restore.c	Tue May 24 10:14:10 2011 +0100
@@ -1133,23 +1133,19 @@ int xc_domain_restore(xc_interface *xch,
 
     int orig_io_fd_flags;
 
-    static struct restore_ctx _ctx = {
-        .live_p2m = NULL,
-        .p2m = NULL,
-    };
-    static struct restore_ctx *ctx = &_ctx;
+    struct restore_ctx _ctx;
+    struct restore_ctx *ctx = &_ctx;
     struct domain_info_context *dinfo = &ctx->dinfo;
 
     pagebuf_init(&pagebuf);
     memset(&tailbuf, 0, sizeof(tailbuf));
     tailbuf.ishvm = hvm;
 
-    /* For info only */
-    ctx->nr_pfns = 0;
-
     if ( superpages )
         return 1;
 
+    memset(ctx, 0, sizeof(*ctx));
+
     ctxt = xc_hypercall_buffer_alloc(xch, ctxt, sizeof(*ctxt));
 
     if ( ctxt == NULL )
diff -r 0727b2fcc33f -r c43bf40e2f29 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -958,11 +958,8 @@ int xc_domain_save(xc_interface *xch, in
     unsigned long mfn;
 
     struct outbuf ob;
-    static struct save_ctx _ctx = {
-        .live_p2m = NULL,
-        .live_m2p = NULL,
-    };
-    static struct save_ctx *ctx = &_ctx;
+    struct save_ctx _ctx;
+    struct save_ctx *ctx = &_ctx;
     struct domain_info_context *dinfo = &ctx->dinfo;
 
     int completed = 0;
@@ -976,6 +973,8 @@ int xc_domain_save(xc_interface *xch, in
 
     outbuf_init(xch, &ob, OUTBUF_SIZE);
 
+    memset(ctx, 0, sizeof(*ctx));
+
     /* If no explicit control parameters given, use defaults */
     max_iters  = max_iters  ? : DEF_MAX_ITERS;
     max_factor = max_factor ? : DEF_MAX_FACTOR;

[PATCH 2 of 8] libxc: save: drop code under ADAPTIVE_SAVE ifdef

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID 0a7ee235f5139cce2a3c1220086d16b847279dc6
# Parent  c43bf40e2f29eb02c7dcf1a2317f243f1af5f659
libxc: save: drop code under ADAPTIVE_SAVE ifdef.

The ifdef was added in 2005 (7702:b3c2bc39d815) but, as far as I can see, was
never enabled by default. Dropping it will help untangle some macros redefining
functions etc.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r c43bf40e2f29 -r 0a7ee235f513 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -252,98 +252,6 @@ static inline int write_buffer(xc_interf
         return write_exact(fd, buf, len);
 }
 
-#ifdef ADAPTIVE_SAVE
-
-/*
-** We control the rate at which we transmit (or save) to minimize impact
-** on running domains (including the target if we're doing live migrate).
-*/
-
-#define MAX_MBIT_RATE    500      /* maximum transmit rate for migrate */
-#define START_MBIT_RATE  100      /* initial transmit rate for migrate */
-
-/* Scaling factor to convert between a rate (in Mb/s) and time (in usecs) */
-#define RATE_TO_BTU      781250
-
-/* Amount in bytes we allow ourselves to send in a burst */
-#define BURST_BUDGET (100*1024)
-
-/* We keep track of the current and previous transmission rate */
-static int mbit_rate, ombit_rate = 0;
-
-/* Have we reached the maximum transmission rate? */
-#define RATE_IS_MAX() (mbit_rate == MAX_MBIT_RATE)
-
-static inline void initialize_mbit_rate()
-{
-    mbit_rate = START_MBIT_RATE;
-}
-
-static int ratewrite(xc_interface *xch, int io_fd, int live, void *buf, int n)
-{
-    static int budget = 0;
-    static int burst_time_us = -1;
-    static struct timeval last_put = { 0 };
-    struct timeval now;
-    struct timespec delay;
-    long long delta;
-
-    if ( START_MBIT_RATE == 0 )
-        return noncached_write(io_fd, live, buf, n);
-
-    budget -= n;
-    if ( budget < 0 )
-    {
-        if ( mbit_rate != ombit_rate )
-        {
-            burst_time_us = RATE_TO_BTU / mbit_rate;
-            ombit_rate = mbit_rate;
-            DPRINTF("rate limit: %d mbit/s burst budget %d slot time %d\n",
-                    mbit_rate, BURST_BUDGET, burst_time_us);
-        }
-        if ( last_put.tv_sec == 0 )
-        {
-            budget += BURST_BUDGET;
-            gettimeofday(&last_put, NULL);
-        }
-        else
-        {
-            while ( budget < 0 )
-            {
-                gettimeofday(&now, NULL);
-                delta = tv_delta(&now, &last_put);
-                while ( delta > burst_time_us )
-                {
-                    budget += BURST_BUDGET;
-                    last_put.tv_usec += burst_time_us;
-                    if ( last_put.tv_usec > 1000000 )
-                    {
-                        last_put.tv_usec -= 1000000;
-                        last_put.tv_sec++;
-                    }
-                    delta -= burst_time_us;
-                }
-                if ( budget > 0 )
-                    break;
-                delay.tv_sec = 0;
-                delay.tv_nsec = 1000 * (burst_time_us - delta);
-                while ( delay.tv_nsec > 0 )
-                    if ( nanosleep(&delay, &delay) == 0 )
-                        break;
-            }
-        }
-    }
-    return noncached_write(io_fd, live, buf, n);
-}
-
-#else /* ! ADAPTIVE SAVE */
-
-#define RATE_IS_MAX() (0)
-#define ratewrite(xch, _io_fd, _live, _buf, _n) noncached_write((xch), (_io_fd), (_live), (_buf), (_n))
-#define initialize_mbit_rate()
-
-#endif
-
 /* like write_buffer for ratewrite, which returns number of bytes written */
 static inline int ratewrite_buffer(xc_interface *xch,
                                    int dobuf, struct outbuf* ob, int fd,
@@ -352,7 +260,7 @@ static inline int ratewrite_buffer(xc_in
     if ( dobuf )
         return outbuf_hardwrite(xch, ob, fd, buf, len) ? -1 : len;
     else
-        return ratewrite(xch, fd, live, buf, len);
+        return noncached_write(xch, fd, live, buf, len);
 }
 
 static int print_stats(xc_interface *xch, uint32_t domid, int pages_sent,
@@ -392,16 +300,6 @@ static int print_stats(xc_interface *xch
                 (int)((stats->dirty_count*PAGE_SIZE)/(wall_delta*(1000/8))),
                 stats->dirty_count);
 
-#ifdef ADAPTIVE_SAVE
-    if ( ((stats->dirty_count*PAGE_SIZE)/(wall_delta*(1000/8))) > mbit_rate )
-    {
-        mbit_rate = (int)((stats->dirty_count*PAGE_SIZE)/(wall_delta*(1000/8)))
-            + 50;
-        if ( mbit_rate > MAX_MBIT_RATE )
-            mbit_rate = MAX_MBIT_RATE;
-    }
-#endif
-
     d0_cpu_last = d0_cpu_now;
     d1_cpu_last = d1_cpu_now;
     wall_last   = wall_now;
@@ -979,8 +877,6 @@ int xc_domain_save(xc_interface *xch, in
     max_iters  = max_iters  ? : DEF_MAX_ITERS;
     max_factor = max_factor ? : DEF_MAX_FACTOR;
 
-    initialize_mbit_rate();
-
     if ( !get_platform_info(xch, dom,
                             &ctx->max_mfn, &ctx->hvirt_start, &ctx->pt_levels, &dinfo->guest_width) )
     {
@@ -1170,9 +1066,6 @@ int xc_domain_save(xc_interface *xch, in
 
   copypages:
 #define wrexact(fd, buf, len) write_buffer(xch, last_iter, &ob, (fd), (buf), (len))
-#ifdef ratewrite
-#undef ratewrite
-#endif
 #define ratewrite(fd, live, buf, len) ratewrite_buffer(xch, last_iter, &ob, (fd), (live), (buf), (len))
 
     /* Now write out each data page, canonicalising page tables as we go... */
@@ -1509,8 +1402,7 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( live )
         {
-            if ( ((sent_this_iter > sent_last_iter) && RATE_IS_MAX()) ||
-                 (iter >= max_iters) ||
+            if ( (iter >= max_iters) ||
                  (sent_this_iter+skip_this_iter < 50) ||
                  (total_sent > dinfo->p2m_size*max_factor) )
             {

[PATCH 3 of 8] libxc: save: rename ratewrite to uncached

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID 0d97d90876b0e59648bd403794812941bcb8d13e
# Parent  0a7ee235f5139cce2a3c1220086d16b847279dc6
libxc: save: rename ratewrite to uncached.

It doesn't do any ratelimiting...

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 0a7ee235f513 -r 0d97d90876b0 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -252,8 +252,8 @@ static inline int write_buffer(xc_interf
         return write_exact(fd, buf, len);
 }
 
-/* like write_buffer for ratewrite, which returns number of bytes written */
-static inline int ratewrite_buffer(xc_interface *xch,
+/* like write_buffer for noncached, which returns number of bytes written */
+static inline int write_uncached(xc_interface *xch,
                                    int dobuf, struct outbuf* ob, int fd,
                                    int live, void* buf, size_t len)
 {
@@ -1066,7 +1066,7 @@ int xc_domain_save(xc_interface *xch, in
 
   copypages:
 #define wrexact(fd, buf, len) write_buffer(xch, last_iter, &ob, (fd), (buf), (len))
-#define ratewrite(fd, live, buf, len) ratewrite_buffer(xch, last_iter, &ob, (fd), (live), (buf), (len))
+#define ratewrite(fd, live, buf, len) write_uncached(xch, last_iter, &ob, (fd), (live), (buf), (len))
 
     /* Now write out each data page, canonicalising page tables as we go... */
     for ( ; ; )

[PATCH 4 of 8] libxc: save: noncached write doesn't use live parameter

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID d6eb4fc290ba248e6830c5df11c8cb511bfd6db9
# Parent  0d97d90876b0e59648bd403794812941bcb8d13e
libxc: save: noncached write doesn't use live parameter.

so drop it.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 0d97d90876b0 -r d6eb4fc290ba tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -152,7 +152,7 @@ static uint64_t tv_delta(struct timeval 
 }
 
 static int noncached_write(xc_interface *xch,
-                           int fd, int live, void *buffer, int len) 
+                           int fd, void *buffer, int len) 
 {
     static int write_count = 0;
     int rc = (write_exact(fd, buffer, len) == 0) ? len : -1;
@@ -255,12 +255,12 @@ static inline int write_buffer(xc_interf
 /* like write_buffer for noncached, which returns number of bytes written */
 static inline int write_uncached(xc_interface *xch,
                                    int dobuf, struct outbuf* ob, int fd,
-                                   int live, void* buf, size_t len)
+                                   void* buf, size_t len)
 {
     if ( dobuf )
         return outbuf_hardwrite(xch, ob, fd, buf, len) ? -1 : len;
     else
-        return noncached_write(xch, fd, live, buf, len);
+        return noncached_write(xch, fd, buf, len);
 }
 
 static int print_stats(xc_interface *xch, uint32_t domid, int pages_sent,
@@ -1066,7 +1066,7 @@ int xc_domain_save(xc_interface *xch, in
 
   copypages:
 #define wrexact(fd, buf, len) write_buffer(xch, last_iter, &ob, (fd), (buf), (len))
-#define ratewrite(fd, live, buf, len) write_uncached(xch, last_iter, &ob, (fd), (live), (buf), (len))
+#define wruncached(fd, live, buf, len) write_uncached(xch, last_iter, &ob, (fd), (buf), (len))
 
     /* Now write out each data page, canonicalising page tables as we go... */
     for ( ; ; )
@@ -1300,7 +1300,7 @@ int xc_domain_save(xc_interface *xch, in
                        run of pages we may have previously acumulated */
                     if ( run )
                     {
-                        if ( ratewrite(io_fd, live, 
+                        if ( wruncached(io_fd, live,
                                        (char*)region_base+(PAGE_SIZE*(j-run)), 
                                        PAGE_SIZE*run) != PAGE_SIZE*run )
                         {
@@ -1332,7 +1332,7 @@ int xc_domain_save(xc_interface *xch, in
                         goto out;
                     }
 
-                    if ( ratewrite(io_fd, live, page, PAGE_SIZE) != PAGE_SIZE )
+                    if ( wruncached(io_fd, live, page, PAGE_SIZE) != PAGE_SIZE )
                     {
                         PERROR("Error when writing to state file (4b)"
                               " (errno %d)", errno);
@@ -1349,7 +1349,7 @@ int xc_domain_save(xc_interface *xch, in
             if ( run )
             {
                 /* write out the last accumulated run of pages */
-                if ( ratewrite(io_fd, live, 
+                if ( wruncached(io_fd, live,
                                (char*)region_base+(PAGE_SIZE*(j-run)), 
                                PAGE_SIZE*run) != PAGE_SIZE*run )
                 {

[PATCH 5 of 8] libxc: save: move static "write_count" variable into outbuf

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID da70ac9b134d29b09a887ee0ad9a5dc4a5531b3b
# Parent  d6eb4fc290ba248e6830c5df11c8cb511bfd6db9
libxc: save: move static "write_count" variable into outbuf.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r d6eb4fc290ba -r da70ac9b134d tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -59,6 +59,7 @@ struct outbuf {
     void* buf;
     size_t size;
     size_t pos;
+    int write_count;
 };
 
 #define OUTBUF_SIZE (16384 * 1024)
@@ -152,19 +153,19 @@ static uint64_t tv_delta(struct timeval 
 }
 
 static int noncached_write(xc_interface *xch,
+                           struct outbuf* ob,
                            int fd, void *buffer, int len) 
 {
-    static int write_count = 0;
     int rc = (write_exact(fd, buffer, len) == 0) ? len : -1;
 
-    write_count += len;
-    if ( write_count >= (MAX_PAGECACHE_USAGE * PAGE_SIZE) )
+    ob->write_count += len;
+    if ( ob->write_count >= (MAX_PAGECACHE_USAGE * PAGE_SIZE) )
     {
         /* Time to discard cache - dont care if this fails */
         int saved_errno = errno;
         discard_file_cache(xch, fd, 0 /* no flush */);
         errno = saved_errno;
-        write_count = 0;
+        ob->write_count = 0;
     }
 
     return rc;
@@ -260,7 +261,7 @@ static inline int write_uncached(xc_inte
     if ( dobuf )
         return outbuf_hardwrite(xch, ob, fd, buf, len) ? -1 : len;
     else
-        return noncached_write(xch, fd, buf, len);
+        return noncached_write(xch, ob, fd, buf, len);
 }
 
 static int print_stats(xc_interface *xch, uint32_t domid, int pages_sent,

[PATCH 6 of 8] libxc: save: encapsulate time stats in a struct

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID c6265f8b07948c5e5042b18f612e17f187921229
# Parent  da70ac9b134d29b09a887ee0ad9a5dc4a5531b3b
libxc: save: encapsulate time stats in a struct

As a precursor to making non-static.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r da70ac9b134d -r c6265f8b0794 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -264,32 +264,35 @@ static inline int write_uncached(xc_inte
         return noncached_write(xch, ob, fd, buf, len);
 }
 
+struct time_stats {
+    struct timeval wall;
+    long long d0_cpu, d1_cpu;
+};
+
 static int print_stats(xc_interface *xch, uint32_t domid, int pages_sent,
                        xc_shadow_op_stats_t *stats, int print)
 {
-    static struct timeval wall_last;
-    static long long      d0_cpu_last;
-    static long long      d1_cpu_last;
+    static struct time_stats last;
+    struct time_stats now;
 
-    struct timeval        wall_now;
-    long long             wall_delta;
-    long long             d0_cpu_now, d0_cpu_delta;
-    long long             d1_cpu_now, d1_cpu_delta;
+    long long wall_delta;
+    long long d0_cpu_delta;
+    long long d1_cpu_delta;
 
-    gettimeofday(&wall_now, NULL);
+    gettimeofday(&now.wall, NULL);
 
-    d0_cpu_now = xc_domain_get_cpu_usage(xch, 0, /* FIXME */ 0)/1000;
-    d1_cpu_now = xc_domain_get_cpu_usage(xch, domid, /* FIXME */ 0)/1000;
+    now.d0_cpu = xc_domain_get_cpu_usage(xch, 0, /* FIXME */ 0)/1000;
+    now.d1_cpu = xc_domain_get_cpu_usage(xch, domid, /* FIXME */ 0)/1000;
 
-    if ( (d0_cpu_now == -1) || (d1_cpu_now == -1) )
+    if ( (now.d0_cpu == -1) || (now.d1_cpu == -1) )
         DPRINTF("ARRHHH!!\n");
 
-    wall_delta = tv_delta(&wall_now,&wall_last)/1000;
+    wall_delta = tv_delta(&now.wall,&last.wall)/1000;
     if ( wall_delta == 0 )
         wall_delta = 1;
 
-    d0_cpu_delta = (d0_cpu_now - d0_cpu_last)/1000;
-    d1_cpu_delta = (d1_cpu_now - d1_cpu_last)/1000;
+    d0_cpu_delta = (now.d0_cpu - last.d0_cpu)/1000;
+    d1_cpu_delta = (now.d1_cpu - last.d1_cpu)/1000;
 
     if ( print )
         DPRINTF("delta %lldms, dom0 %d%%, target %d%%, sent %dMb/s, "
@@ -301,9 +304,7 @@ static int print_stats(xc_interface *xch
                 (int)((stats->dirty_count*PAGE_SIZE)/(wall_delta*(1000/8))),
                 stats->dirty_count);
 
-    d0_cpu_last = d0_cpu_now;
-    d1_cpu_last = d1_cpu_now;
-    wall_last   = wall_now;
+    last = now;
 
     return 0;
 }

[PATCH 7 of 8] libxc: save: don't bother calculating stat's deltas unless we are going to print them

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID 5463bdc1d77942b50b28eea059eaba4d1ec7d2ac
# Parent  c6265f8b07948c5e5042b18f612e17f187921229
libxc: save: don't bother calculating stat's deltas unless we are going to print them

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r c6265f8b0794 -r 5463bdc1d779 tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -275,10 +275,6 @@ static int print_stats(xc_interface *xch
     static struct time_stats last;
     struct time_stats now;
 
-    long long wall_delta;
-    long long d0_cpu_delta;
-    long long d1_cpu_delta;
-
     gettimeofday(&now.wall, NULL);
 
     now.d0_cpu = xc_domain_get_cpu_usage(xch, 0, /* FIXME */ 0)/1000;
@@ -287,14 +283,19 @@ static int print_stats(xc_interface *xch
     if ( (now.d0_cpu == -1) || (now.d1_cpu == -1) )
         DPRINTF("ARRHHH!!\n");
 
-    wall_delta = tv_delta(&now.wall,&last.wall)/1000;
-    if ( wall_delta == 0 )
-        wall_delta = 1;
+    if ( print )
+    {
+        long long wall_delta;
+        long long d0_cpu_delta;
+        long long d1_cpu_delta;
 
-    d0_cpu_delta = (now.d0_cpu - last.d0_cpu)/1000;
-    d1_cpu_delta = (now.d1_cpu - last.d1_cpu)/1000;
+        wall_delta = tv_delta(&now.wall,&last.wall)/1000;
+        if ( wall_delta == 0 )
+            wall_delta = 1;
 
-    if ( print )
+        d0_cpu_delta = (now.d0_cpu - last.d0_cpu)/1000;
+        d1_cpu_delta = (now.d1_cpu - last.d1_cpu)/1000;
+
         DPRINTF("delta %lldms, dom0 %d%%, target %d%%, sent %dMb/s, "
                 "dirtied %dMb/s %" PRId32 " pages\n",
                 wall_delta,
@@ -303,6 +304,7 @@ static int print_stats(xc_interface *xch
                 (int)((pages_sent*PAGE_SIZE)/(wall_delta*(1000/8))),
                 (int)((stats->dirty_count*PAGE_SIZE)/(wall_delta*(1000/8))),
                 stats->dirty_count);
+    }
 
     last = now;

[PATCH 8 of 8] libxc: save: move static stats variable to stack variable

[Ian Campbell]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1306228450 -3600
# Node ID 32d62506e3be95124097775dc79c42304a18084c
# Parent  5463bdc1d77942b50b28eea059eaba4d1ec7d2ac
libxc: save: move static stats variable to stack variable.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 5463bdc1d779 -r 32d62506e3be tools/libxc/xc_domain_save.c
--- a/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
+++ b/tools/libxc/xc_domain_save.c	Tue May 24 10:14:10 2011 +0100
@@ -270,9 +270,9 @@ struct time_stats {
 };
 
 static int print_stats(xc_interface *xch, uint32_t domid, int pages_sent,
+                       struct time_stats *last,
                        xc_shadow_op_stats_t *stats, int print)
 {
-    static struct time_stats last;
     struct time_stats now;
 
     gettimeofday(&now.wall, NULL);
@@ -289,12 +289,12 @@ static int print_stats(xc_interface *xch
         long long d0_cpu_delta;
         long long d1_cpu_delta;
 
-        wall_delta = tv_delta(&now.wall,&last.wall)/1000;
+        wall_delta = tv_delta(&now.wall,&last->wall)/1000;
         if ( wall_delta == 0 )
             wall_delta = 1;
 
-        d0_cpu_delta = (now.d0_cpu - last.d0_cpu)/1000;
-        d1_cpu_delta = (now.d1_cpu - last.d1_cpu)/1000;
+        d0_cpu_delta = (now.d0_cpu - last->d0_cpu)/1000;
+        d1_cpu_delta = (now.d1_cpu - last->d1_cpu)/1000;
 
         DPRINTF("delta %lldms, dom0 %d%%, target %d%%, sent %dMb/s, "
                 "dirtied %dMb/s %" PRId32 " pages\n",
@@ -306,7 +306,7 @@ static int print_stats(xc_interface *xch
                 stats->dirty_count);
     }
 
-    last = now;
+    *last = now;
 
     return 0;
 }
@@ -843,7 +843,8 @@ int xc_domain_save(xc_interface *xch, in
     DECLARE_HYPERCALL_BUFFER(unsigned long, to_send);
     unsigned long *to_fix = NULL;
 
-    xc_shadow_op_stats_t stats;
+    struct time_stats time_stats;
+    xc_shadow_op_stats_t shadow_stats;
 
     unsigned long needed_to_fix = 0;
     unsigned long total_sent    = 0;
@@ -1053,7 +1054,7 @@ int xc_domain_save(xc_interface *xch, in
         DPRINTF("Had %d unexplained entries in p2m table\n", err);
     }
 
-    print_stats(xch, dom, 0, &stats, 0);
+    print_stats(xch, dom, 0, &time_stats, &shadow_stats, 0);
 
     tmem_saved = xc_tmem_save(xch, dom, io_fd, live, XC_SAVE_ID_TMEM);
     if ( tmem_saved == -1 )
@@ -1377,7 +1378,7 @@ int xc_domain_save(xc_interface *xch, in
 
         if ( last_iter )
         {
-            print_stats( xch, dom, sent_this_iter, &stats, 1);
+            print_stats( xch, dom, sent_this_iter, &time_stats, &shadow_stats, 1);
 
             DPRINTF("Total pages sent= %ld (%.2fx)\n",
                     total_sent, ((float)total_sent)/dinfo->p2m_size );
@@ -1439,7 +1440,7 @@ int xc_domain_save(xc_interface *xch, in
 
             if ( xc_shadow_control(xch, dom,
                                    XEN_DOMCTL_SHADOW_OP_CLEAN, HYPERCALL_BUFFER(to_send),
-                                   dinfo->p2m_size, NULL, 0, &stats) != dinfo->p2m_size )
+                                   dinfo->p2m_size, NULL, 0, &shadow_stats) != dinfo->p2m_size )
             {
                 PERROR("Error flushing shadow PT");
                 goto out;
@@ -1447,7 +1448,7 @@ int xc_domain_save(xc_interface *xch, in
 
             sent_last_iter = sent_this_iter;
 
-            print_stats(xch, dom, sent_this_iter, &stats, 1);
+            print_stats(xch, dom, sent_this_iter, &time_stats, &shadow_stats, 1);
 
         }
     } /* end of infinite for loop */
@@ -1810,7 +1811,7 @@ int xc_domain_save(xc_interface *xch, in
         callbacks->checkpoint(callbacks->data) > 0)
     {
         /* reset stats timer */
-        print_stats(xch, dom, 0, &stats, 0);
+        print_stats(xch, dom, 0, &time_stats, &shadow_stats, 0);
 
         rc = 1;
         /* last_iter = 1; */
@@ -1821,11 +1822,11 @@ int xc_domain_save(xc_interface *xch, in
             goto out;
         }
         DPRINTF("SUSPEND shinfo %08lx\n", info.shared_info_frame);
-        print_stats(xch, dom, 0, &stats, 1);
+        print_stats(xch, dom, 0, &time_stats, &shadow_stats, 1);
 
         if ( xc_shadow_control(xch, dom,
                                XEN_DOMCTL_SHADOW_OP_CLEAN, HYPERCALL_BUFFER(to_send),
-                               dinfo->p2m_size, NULL, 0, &stats) != dinfo->p2m_size )
+                               dinfo->p2m_size, NULL, 0, &shadow_stats) != dinfo->p2m_size )
         {
             PERROR("Error flushing shadow PT");
         }

Re: [PATCH 1 of 8] libxc: save/restore: remove static context variables

[Vincent Hanquez]

On 05/24/2011 10:14 AM, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1306228450 -3600
> # Node ID c43bf40e2f29eb02c7dcf1a2317f243f1af5f659
> # Parent  0727b2fcc33fd5cdf847fde0d2be261be3d6b279
> libxc: save/restore: remove static context variables
>
> 20544:ad9d75d74bd5 and 20545:cc7d66ba0dad seemingly intended to change these
> global static variables into stack variables but didn't remove the static
> qualifier.
>
> Also zero the entire struct once with memset rather than clearing fields
> piecemeal in two different places.

Good catch.
Acked-by-self.

-- 
Vincent

Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[Shriram Rajagopalan]

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

On Tue, May 24, 2011 at 5:14 AM, Ian Campbell <ian.campbell@citrix.com>wrote:

> Otherwise users which deal with multiple domains need to do their own
> locking and cannot save/migrate multiple domains in parallel (should
> they want to).
>
> Also made a bunch of cleanup along the way, mainly to make it easier
> to figure out what was going on with the twisty maze of macros
> redefining functions as macros and redefining the macros etc. I'm sure
> there is plenty more straightening out which could be done but I don't
> have the stomach for it this morning.
>
> I ll do it!!.. I have been waiting for this. Thanks a lot for cleaning up
this chaff!
I was under the impression that this was some arcane legacy code that
shouldnt
be touched. One particular thing that I would like to do is to factor out
the write functions
(outbuf_*, noncached_write, ratewrite*, etc) into a separate file and make
it sort of pluggable.

(selfish :P) I wanted to introduce a patch that would overlap outbuf flush
operation
and guest memory copy operation instead of the current model
<flush,copy,flush,copy..>.
This might be helpful for both Remus and live migration of large domains.

 Shriram, does this have any impact on Remus?
>
> From a cursory look, it doesnt look like it would impact Remus. I ll test
it
in my setup and revert soon.

shriram

[-- Attachment #1.2: Type: text/html, Size: 1847 bytes --]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 8 of 8] libxc: save: move static stats variable to stack variable

[Ian Jackson]

Ian Campbell writes ("[Xen-devel] [PATCH 8 of 8] libxc: save: move static stats variable to stack variable"):
> libxc: save: move static stats variable to stack variable.

Cor what a pile of poo you discovered.  Thanks for cleaning it up!

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[Ian Jackson]

Shriram Rajagopalan writes ("[Xen-devel] Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c"):
> I ll do it!!.. I have been waiting for this. Thanks a lot for
> cleaning up this chaff!  I was under the impression that this was
> some arcane legacy code that shouldnt be touched.

No, it's arcane legacy code that we have been gradually cleaning up
:-).

> One particular
> thing that I would like to do is to factor out the write functions
> (outbuf_*, noncached_write, ratewrite*, etc) into a separate file
> and make it sort of pluggable.

Do you have a particular use case fot that ?  Without a different set
of implementations I'm not sure that we need it to be pluggable.

> (selfish :P) I wanted to introduce a patch that would overlap outbuf
> flush operation and guest memory copy operation instead of the
> current model <flush,copy,flush,copy..>.  This might be helpful for
> both Remus and live migration of large domains.

But yes, if that produces a speedup, certainly.

>  Shriram, does this have any impact on Remus?

I think it should be OK but we should hear what Shriram has to say
(CC'd).

Ian.

Re: Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[Ian Campbell]

On Tue, 2011-05-24 at 18:02 +0100, Ian Jackson wrote:
> Shriram Rajagopalan writes ("[Xen-devel] Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c"):
> > I ll do it!!.. I have been waiting for this. Thanks a lot for
> > cleaning up this chaff!  I was under the impression that this was
> > some arcane legacy code that shouldnt be touched.
> 
> No, it's arcane legacy code that we have been gradually cleaning up
> :-).
> 
> > One particular
> > thing that I would like to do is to factor out the write functions
> > (outbuf_*, noncached_write, ratewrite*, etc) into a separate file
> > and make it sort of pluggable.
> 
> Do you have a particular use case fot that ?  Without a different set
> of implementations I'm not sure that we need it to be pluggable.
> 
> > (selfish :P) I wanted to introduce a patch that would overlap outbuf
> > flush operation and guest memory copy operation instead of the
> > current model <flush,copy,flush,copy..>.  This might be helpful for
> > both Remus and live migration of large domains.
> 
> But yes, if that produces a speedup, certainly.
> 
> >  Shriram, does this have any impact on Remus?
> 
> I think it should be OK but we should hear what Shriram has to say
> (CC'd).

You were replying to Shriram ;-)

Ian.

Re: Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c

[Shriram Rajagopalan]

On Tue, May 24, 2011 at 1:52 PM, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
>
> On Tue, 2011-05-24 at 18:02 +0100, Ian Jackson wrote:
> > Shriram Rajagopalan writes ("[Xen-devel] Re: [PATCH 0 of 8] Remove static variables from xc_domain_{save, restore}.c"):
> > > I ll do it!!.. I have been waiting for this. Thanks a lot for
> > > cleaning up this chaff!  I was under the impression that this was
> > > some arcane legacy code that shouldnt be touched.
> >
> > No, it's arcane legacy code that we have been gradually cleaning up
> > :-).
> >
> > > One particular
> > > thing that I would like to do is to factor out the write functions
> > > (outbuf_*, noncached_write, ratewrite*, etc) into a separate file
> > > and make it sort of pluggable.
> >
> > Do you have a particular use case fot that ?  Without a different set
> > of implementations I'm not sure that we need it to be pluggable.
> >
> > > (selfish :P) I wanted to introduce a patch that would overlap outbuf
> > > flush operation and guest memory copy operation instead of the
> > > current model <flush,copy,flush,copy..>.  This might be helpful for
> > > both Remus and live migration of large domains.
> >
> > But yes, if that produces a speedup, certainly.
> >
> > >  Shriram, does this have any impact on Remus?
> >
> > I think it should be OK but we should hear what Shriram has to say
> > (CC'd).
>
> You were replying to Shriram ;-)
>
> Ian.
>
>
>
Ok. I ve tested it with remus. works properly :).

Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

thanks
shriram