[Qemu-devel] [PATCH 16/51] ram: Move iterations into RAMState

Juan Quintela posted 51 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 16/51] ram: Move iterations into RAMState
Posted by Juan Quintela 8 years, 10 months ago
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 9fa3bd7..690ca8f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -170,6 +170,8 @@ struct RAMState {
     uint64_t zero_pages;
     /* number of normal transferred pages */
     uint64_t norm_pages;
+    /* Iterations since start */
+    uint64_t iterations;
 };
 typedef struct RAMState RAMState;
 
@@ -177,7 +179,6 @@ static RAMState ram_state;
 
 /* accounting for migration statistics */
 typedef struct AccountingInfo {
-    uint64_t iterations;
     uint64_t xbzrle_bytes;
     uint64_t xbzrle_pages;
     uint64_t xbzrle_cache_miss;
@@ -693,13 +694,13 @@ static void migration_bitmap_sync(RAMState *rs)
         }
 
         if (migrate_use_xbzrle()) {
-            if (rs->iterations_prev != acct_info.iterations) {
+            if (rs->iterations_prev != rs->iterations) {
                 acct_info.xbzrle_cache_miss_rate =
                    (double)(acct_info.xbzrle_cache_miss -
                             rs->xbzrle_cache_miss_prev) /
-                   (acct_info.iterations - rs->iterations_prev);
+                   (rs->iterations - rs->iterations_prev);
             }
-            rs->iterations_prev = acct_info.iterations;
+            rs->iterations_prev = rs->iterations;
             rs->xbzrle_cache_miss_prev = acct_info.xbzrle_cache_miss;
         }
         s->dirty_pages_rate = rs->num_dirty_pages_period * 1000
@@ -1993,6 +1994,7 @@ static int ram_save_init_globals(RAMState *rs)
     rs->bitmap_sync_count = 0;
     rs->zero_pages = 0;
     rs->norm_pages = 0;
+    rs->iterations = 0;
     migration_bitmap_sync_init(rs);
     qemu_mutex_init(&migration_bitmap_mutex);
 
@@ -2150,7 +2152,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
             done = 1;
             break;
         }
-        acct_info.iterations++;
+        rs->iterations++;
 
         /* we want to check in the 1st loop, just in case it was the 1st time
            and we had to sync the dirty bitmap.
-- 
2.9.3


Re: [Qemu-devel] [PATCH 16/51] ram: Move iterations into RAMState
Posted by Peter Xu 8 years, 10 months ago
On Thu, Mar 23, 2017 at 09:45:09PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Another comment not directly related to this patch...

> ---
>  migration/ram.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 9fa3bd7..690ca8f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -170,6 +170,8 @@ struct RAMState {
>      uint64_t zero_pages;
>      /* number of normal transferred pages */
>      uint64_t norm_pages;
> +    /* Iterations since start */
> +    uint64_t iterations;
>  };
>  typedef struct RAMState RAMState;
>  
> @@ -177,7 +179,6 @@ static RAMState ram_state;
>  
>  /* accounting for migration statistics */
>  typedef struct AccountingInfo {
> -    uint64_t iterations;
>      uint64_t xbzrle_bytes;
>      uint64_t xbzrle_pages;
>      uint64_t xbzrle_cache_miss;
> @@ -693,13 +694,13 @@ static void migration_bitmap_sync(RAMState *rs)
>          }
>  
>          if (migrate_use_xbzrle()) {
> -            if (rs->iterations_prev != acct_info.iterations) {
> +            if (rs->iterations_prev != rs->iterations) {
>                  acct_info.xbzrle_cache_miss_rate =
>                     (double)(acct_info.xbzrle_cache_miss -
>                              rs->xbzrle_cache_miss_prev) /
> -                   (acct_info.iterations - rs->iterations_prev);
> +                   (rs->iterations - rs->iterations_prev);

Here we are calculating cache miss rate by xbzrle_cache_miss and
iterations. However looks like xbzrle_cache_miss is counted per guest
page (in save_xbzrle_page()) while the iteration count is per host
page (in ram_save_iterate()). Then, what if host page size not equals
to guest page size? E.g., when host uses 2M huge pages, host page size
is 2M, while guest page size can be 4K?

Thanks,

-- peterx