[Qemu-devel] [PATCH 00/10] migration: Optimization the xbzrle and fix two corruption issues

[Qemu-devel] [PATCH 00/10] migration: Optimization the xbzrle and fix two corruption issues

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

V1-->V2
* expose the counter that logs the times of updating the dirty bitmap to end user.

a. Optimization the xbzrle remarkable decrease the cache misses.
    The efficiency of compress increases more than fifty times.
    Before the patch set, the cache almost totally miss when the 
    number of cache item less than the dirty page number. Now the
    hot pages in the cache will not be replaced by other pages. 

b. Reducing the data copy

c. Fix two corruption issues.

ChenLiang (10):
  XBZRLE: Fix one XBZRLE corruption issues
  migration: Add counters of updating the dirty bitmap
  XBZRLE: optimize XBZRLE to decrease the cache missing
  XBZRLE: rebuild the cache_is_cached function
  migration: Fix the migrate auto converge process
  migraion: optimiztion xbzrle by reducing data copy
  migraion: clear the death code
  migration: s/uint64_t/int64_t the definitions of it_age
  migration: expose the bitmap_sync_cnt to the end user
  XBZRLE: update the doc of XBZRLE

 arch_init.c                    |  97 ++++++++++++++++++---------------------
 docs/xbzrle.txt                |   7 +++
 hmp.c                          |   2 +
 include/migration/migration.h  |   1 +
 include/migration/page_cache.h |  10 +++--
 migration.c                    |   2 +
 page_cache.c                   | 100 +++++++++++------------------------------
 qapi-schema.json               |   4 +-
 qmp-commands.hx                |   5 +++
 9 files changed, 98 insertions(+), 130 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 01/10] XBZRLE: Fix one XBZRLE corruption issues

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

The page may not be inserted into cache after executing save_xbzrle_page.
In case of failure to insert, the original page should be sent rather
than the page in the cache.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 arch_init.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 60c975d..2ac68c2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -340,7 +340,7 @@ static void xbzrle_cache_zero_page(ram_addr_t current_addr)
 
 #define ENCODING_FLAG_XBZRLE 0x1
 
-static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
+static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
                             ram_addr_t current_addr, RAMBlock *block,
                             ram_addr_t offset, int cont, bool last_stage)
 {
@@ -348,19 +348,23 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
     uint8_t *prev_cached_page;
 
     if (!cache_is_cached(XBZRLE.cache, current_addr)) {
+        acct_info.xbzrle_cache_miss++;
         if (!last_stage) {
-            if (cache_insert(XBZRLE.cache, current_addr, current_data) == -1) {
+            if (cache_insert(XBZRLE.cache, current_addr, *current_data) == -1) {
                 return -1;
+            } else {
+                /* update *current_data when the page has been
+                   inserted into cache */
+                *current_data = get_cached_data(XBZRLE.cache, current_addr);
             }
         }
-        acct_info.xbzrle_cache_miss++;
         return -1;
     }
 
     prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
 
     /* save current buffer into memory */
-    memcpy(XBZRLE.current_buf, current_data, TARGET_PAGE_SIZE);
+    memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE);
 
     /* XBZRLE encoding (if there is no overflow) */
     encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
@@ -373,7 +377,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
         DPRINTF("Overflow\n");
         acct_info.xbzrle_overflows++;
         /* update data in the cache */
-        memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
+        if (!last_stage) {
+            memcpy(prev_cached_page, *current_data, TARGET_PAGE_SIZE);
+            *current_data = prev_cached_page;
+        }
         return -1;
     }
 
@@ -598,15 +605,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
                  */
                 xbzrle_cache_zero_page(current_addr);
             } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
-                bytes_sent = save_xbzrle_page(f, p, current_addr, block,
+                bytes_sent = save_xbzrle_page(f, &p, current_addr, block,
                                               offset, cont, last_stage);
                 if (!last_stage) {
-                    /* We must send exactly what's in the xbzrle cache
-                     * even if the page wasn't xbzrle compressed, so that
-                     * it's right next time.
-                     */
-                    p = get_cached_data(XBZRLE.cache, current_addr);
-
                     /* Can't send this cached data async, since the cache page
                      * might get updated before it gets to the wire
                      */
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 01/10] XBZRLE: Fix one XBZRLE corruption issues

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> The page may not be inserted into cache after executing save_xbzrle_page.
> In case of failure to insert, the original page should be sent rather
> than the page in the cache.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH 02/10] migration: Add counters of updating the dirty bitmap

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Add counters to log the times of updating the dirty bitmap.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 arch_init.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch_init.c b/arch_init.c
index 2ac68c2..37e4aa5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -110,6 +110,23 @@ static bool mig_throttle_on;
 static int dirty_rate_high_cnt;
 static void check_guest_throttling(void);
 
+static uint64_t bitmap_sync_cnt;
+/* the functions *_bitmap_sync_cnt only run in migrate thread */
+static inline void reset_bitmap_sync_cnt(void)
+{
+    bitmap_sync_cnt = 0;
+}
+
+static inline void increase_bitmap_sync_cnt(void)
+{
+    bitmap_sync_cnt++;
+}
+
+static inline uint64_t get_bitmap_sync_cnt(void)
+{
+    return bitmap_sync_cnt;
+}
+
 /***********************************************************/
 /* ram save/restore */
 
@@ -487,6 +504,8 @@ static void migration_bitmap_sync(void)
     int64_t end_time;
     int64_t bytes_xfer_now;
 
+    increase_bitmap_sync_cnt();
+
     if (!bytes_xfer_prev) {
         bytes_xfer_prev = ram_bytes_transferred();
     }
@@ -734,6 +753,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     migration_dirty_pages = ram_pages;
     mig_throttle_on = false;
     dirty_rate_high_cnt = 0;
+    reset_bitmap_sync_cnt();
 
     if (migrate_use_xbzrle()) {
         qemu_mutex_lock_iothread();
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 02/10] migration: Add counters of updating the dirty bitmap

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1954 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 
> Add counters to log the times of updating the dirty bitmap.
> 
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Wait - how did my Reviewed-by get here?  There is no [PATCHv2] in the
subject line to indicate that I reviewed an earlier revision. and the
only other mail I see from me in the archives with the same subject line
is here:
https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05143.html

where I made a comment, but did NOT give my signature on the patch.

Please follow http://wiki.qemu.org/Contribute/SubmitAPatch when using
Reviewed-by: tags and subject lines.  Otherwise, you are making life
harder for yourself and for reviewers.

> ---
>  arch_init.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 2ac68c2..37e4aa5 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -110,6 +110,23 @@ static bool mig_throttle_on;
>  static int dirty_rate_high_cnt;
>  static void check_guest_throttling(void);
>  
> +static uint64_t bitmap_sync_cnt;
> +/* the functions *_bitmap_sync_cnt only run in migrate thread */
> +static inline void reset_bitmap_sync_cnt(void)

Isn't static inline a bit much in a .c file?  You generally want inline
functions to appear in .h.  For this particular file, I think these
functions are overkill...

>  
> +    increase_bitmap_sync_cnt();

and that you could just directly write 'bitmap_sync_cnt++' here.

When you send v3, do NOT add my Reviewed-by yet, because I still want to
make sure that the new version is correct.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 02/10] migration: Add counters of updating the dirty bitmap

[Gonglei (Arei)]

> On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> > From: ChenLiang <chenliang88@huawei.com>
> >
> > Add counters to log the times of updating the dirty bitmap.
> >
> > Signed-off-by: ChenLiang <chenliang88@huawei.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Wait - how did my Reviewed-by get here?  There is no [PATCHv2] in the
> subject line to indicate that I reviewed an earlier revision. and the
> only other mail I see from me in the archives with the same subject line
> is here:
> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05143.html
> 
> where I made a comment, but did NOT give my signature on the patch.
> 
> Please follow http://wiki.qemu.org/Contribute/SubmitAPatch when using
> Reviewed-by: tags and subject lines.  Otherwise, you are making life
> harder for yourself and for reviewers.
> 
I'm so sorry for those, and will post a new version.

> > ---
> >  arch_init.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 2ac68c2..37e4aa5 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -110,6 +110,23 @@ static bool mig_throttle_on;
> >  static int dirty_rate_high_cnt;
> >  static void check_guest_throttling(void);
> >
> > +static uint64_t bitmap_sync_cnt;
> > +/* the functions *_bitmap_sync_cnt only run in migrate thread */
> > +static inline void reset_bitmap_sync_cnt(void)
> 
> Isn't static inline a bit much in a .c file?  You generally want inline
> functions to appear in .h.  For this particular file, I think these
> functions are overkill...
> 
> >
> > +    increase_bitmap_sync_cnt();
> 
> and that you could just directly write 'bitmap_sync_cnt++' here.
> 
> When you send v3, do NOT add my Reviewed-by yet, because I still want to
> make sure that the new version is correct.
> 
I see.

> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 02/10] migration: Add counters of updating the dirty bitmap

[Juan Quintela]

"Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
>> On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
>> > From: ChenLiang <chenliang88@huawei.com>
>> >
>> > Add counters to log the times of updating the dirty bitmap.
>> >
>> > Signed-off-by: ChenLiang <chenliang88@huawei.com>
>> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> > Reviewed-by: Eric Blake <eblake@redhat.com>
>> 
>> Wait - how did my Reviewed-by get here?  There is no [PATCHv2] in the
>> subject line to indicate that I reviewed an earlier revision. and the
>> only other mail I see from me in the archives with the same subject line
>> is here:
>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05143.html
>> 
>> where I made a comment, but did NOT give my signature on the patch.
>> 
>> Please follow http://wiki.qemu.org/Contribute/SubmitAPatch when using
>> Reviewed-by: tags and subject lines.  Otherwise, you are making life
>> harder for yourself and for reviewers.
>> 
> I'm so sorry for those, and will post a new version.

>> > +static uint64_t bitmap_sync_cnt;

If you change, you can do s/_cnt/_count/?  As suggested in the output
string?

Thanks, Juan.

Later, Juan.

[Qemu-devel] [PATCH 03/10] XBZRLE: optimize XBZRLE to decrease the cache missing

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Avoid the hot pages being replaced by others to remarkable decrease cache
missing. The counter of updating dirty bitmap is used to indicate the cached
page age.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c                    |  8 +++++---
 include/migration/page_cache.h | 10 +++++++---
 page_cache.c                   | 23 +++++++++++++++++++----
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 37e4aa5..c6a70b8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -352,7 +352,8 @@ static void xbzrle_cache_zero_page(ram_addr_t current_addr)
 
     /* We don't care if this fails to allocate a new cache page
      * as long as it updated an old one */
-    cache_insert(XBZRLE.cache, current_addr, ZERO_TARGET_PAGE);
+    cache_insert(XBZRLE.cache, current_addr, ZERO_TARGET_PAGE,
+                 get_bitmap_sync_cnt());
 }
 
 #define ENCODING_FLAG_XBZRLE 0x1
@@ -364,10 +365,11 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
     int encoded_len = 0, bytes_sent = -1;
     uint8_t *prev_cached_page;
 
-    if (!cache_is_cached(XBZRLE.cache, current_addr)) {
+    if (!cache_is_cached(XBZRLE.cache, current_addr, get_bitmap_sync_cnt())) {
         acct_info.xbzrle_cache_miss++;
         if (!last_stage) {
-            if (cache_insert(XBZRLE.cache, current_addr, *current_data) == -1) {
+            if (cache_insert(XBZRLE.cache, current_addr, *current_data,
+                get_bitmap_sync_cnt()) == -1) {
                 return -1;
             } else {
                 /* update *current_data when the page has been
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index 2d5ce2d..dc0c6b5 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -43,8 +43,10 @@ void cache_fini(PageCache *cache);
  *
  * @cache pointer to the PageCache struct
  * @addr: page addr
+ * @current_age indicate the age of the page if cache hit
  */
-bool cache_is_cached(const PageCache *cache, uint64_t addr);
+bool cache_is_cached(const PageCache *cache, uint64_t addr,
+                     uint64_t current_age);
 
 /**
  * get_cached_data: Get the data cached for an addr
@@ -60,13 +62,15 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
  * cache_insert: insert the page into the cache. the page cache
  * will dup the data on insert. the previous value will be overwritten
  *
- * Returns -1 on error
+ * Returns -1 when the page isn't be inserted into cache
  *
  * @cache pointer to the PageCache struct
  * @addr: page address
  * @pdata: pointer to the page
+ * @current_age indicate the age of the page if the page is inserted into cache
  */
-int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata);
+int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
+                 uint64_t current_age);
 
 /**
  * cache_resize: resize the page cache. In case of size reduction the extra
diff --git a/page_cache.c b/page_cache.c
index b033681..74d9f8e 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -33,6 +33,9 @@
     do { } while (0)
 #endif
 
+/* the page in cache will not be replaced in two cycles */
+#define CACHED_PAGE_LIFETIME 2
+
 typedef struct CacheItem CacheItem;
 
 struct CacheItem {
@@ -121,7 +124,8 @@ static size_t cache_get_cache_pos(const PageCache *cache,
     return pos;
 }
 
-bool cache_is_cached(const PageCache *cache, uint64_t addr)
+bool cache_is_cached(const PageCache *cache, uint64_t addr,
+                     uint64_t current_age)
 {
     size_t pos;
 
@@ -130,7 +134,12 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr)
 
     pos = cache_get_cache_pos(cache, addr);
 
-    return (cache->page_cache[pos].it_addr == addr);
+    if (cache->page_cache[pos].it_addr == addr) {
+        /* update the it_age when the cache hit */
+        cache->page_cache[pos].it_age = current_age;
+        return true;
+    }
+    return false;
 }
 
 static CacheItem *cache_get_by_addr(const PageCache *cache, uint64_t addr)
@@ -150,7 +159,8 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr)
     return cache_get_by_addr(cache, addr)->it_data;
 }
 
-int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata)
+int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
+                 uint64_t current_age)
 {
 
     CacheItem *it = NULL;
@@ -161,6 +171,11 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata)
     /* actual update of entry */
     it = cache_get_by_addr(cache, addr);
 
+    if ((it->it_data != NULL) && (it->it_age +
+        CACHED_PAGE_LIFETIME > current_age)) {
+        /* the cache page is fresh, don't replace it */
+        return -1;
+    }
     /* allocate page */
     if (!it->it_data) {
         it->it_data = g_try_malloc(cache->page_size);
@@ -173,7 +188,7 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata)
 
     memcpy(it->it_data, pdata, cache->page_size);
 
-    it->it_age = ++cache->max_item_age;
+    it->it_age = current_age;
     it->it_addr = addr;
 
     return 0;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 03/10] XBZRLE: optimize XBZRLE to decrease the cache missing

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Avoid the hot pages being replaced by others to remarkable decrease cache
> missing. The counter of updating dirty bitmap is used to indicate the cached
> page age.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>


>  /**
>   * get_cached_data: Get the data cached for an addr
> @@ -60,13 +62,15 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
>   * cache_insert: insert the page into the cache. the page cache
>   * will dup the data on insert. the previous value will be overwritten
>   *
> - * Returns -1 on error
> + * Returns -1 when the page isn't be inserted into cache


s/be//?

>   *
>   * @cache pointer to the PageCache struct
>   * @addr: page address
>   * @pdata: pointer to the page
> + * @current_age indicate the age of the page if the page is inserted into cache

missing colon.

   * @current_age: current bitmap generation



> @@ -161,6 +171,11 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata)
>      /* actual update of entry */
>      it = cache_get_by_addr(cache, addr);
>  
> +    if ((it->it_data != NULL) && (it->it_age +
> +        CACHED_PAGE_LIFETIME > current_age)) {
> +        /* the cache page is fresh, don't replace it */

Should we add a counter for this "misses"?  It is a question to know how
much it happens.

BTW, do you have any benchmark/numbers showing that this is a good idea?

Thanks, Juan.

Re: [Qemu-devel] [PATCH 03/10] XBZRLE: optimize XBZRLE to decrease the cache missing

[Gonglei (Arei)]

Sorry for late.
 
> 
> > @@ -161,6 +171,11 @@ int cache_insert(PageCache *cache, uint64_t addr,
> const uint8_t *pdata)
> >      /* actual update of entry */
> >      it = cache_get_by_addr(cache, addr);
> >
> > +    if ((it->it_data != NULL) && (it->it_age +
> > +        CACHED_PAGE_LIFETIME > current_age)) {
> > +        /* the cache page is fresh, don't replace it */
> 
> Should we add a counter for this "misses"?  It is a question to know how
> much it happens.
expose the rate of xbzrle cache miss to end user.

> 
> BTW, do you have any benchmark/numbers showing that this is a good idea?
> 
> Thanks, Juan.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 03/10] XBZRLE: optimize XBZRLE to decrease the cache missing

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2763 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 
> Avoid the hot pages being replaced by others to remarkable decrease cache

s/the hot/hot/
s/remarkable/remarkably/

> missing. The counter of updating dirty bitmap is used to indicate the cached

s/missing/misses/

> page age.

Please give more rationale - when claiming that something gave a speed
improvement, it helps to back up that claim with the sample program you
ran in the guest along with before and after numbers to back up your
point.  A "remarkable decrease" is subjective, but pre- and post-patch
numbers is objective.

This may mean rebasing your series to re-order patches, and FIRST export
the counters that you then use to justify this patch (the current order
of applying the optimization, and only THEN exposing the numbers through
QMP, makes it harder to prove that this patch helped).

> 
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  arch_init.c                    |  8 +++++---
>  include/migration/page_cache.h | 10 +++++++---
>  page_cache.c                   | 23 +++++++++++++++++++----
>  3 files changed, 31 insertions(+), 10 deletions(-)
> 

>          acct_info.xbzrle_cache_miss++;
>          if (!last_stage) {
> -            if (cache_insert(XBZRLE.cache, current_addr, *current_data) == -1) {
> +            if (cache_insert(XBZRLE.cache, current_addr, *current_data,
> +                get_bitmap_sync_cnt()) == -1) {

Indentation - I'd write:

           if (cache_insert(XBZRLE.cache, current_addr, *current_data,
                            get_bitmap_sync_cnt()) == -1) {

This patch needs to be rebased once you fix 2/10 to just directly use
the variable instead of wrapping it inside pointless static inline
accessor functions.


> @@ -161,6 +171,11 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata)
>      /* actual update of entry */
>      it = cache_get_by_addr(cache, addr);
>  
> +    if ((it->it_data != NULL) && (it->it_age +
> +        CACHED_PAGE_LIFETIME > current_age)) {

Unnecessary (), and odd choice of line break location coupled with poor
indentation.  It fits fine on one 80-column line (hmm, my email may wrap
because I write email in fewer columns than 80):

    if (it->it_data && it->it_age + CACHED_PAGE_LIFETIME > current_age) {

and if you MUST break lines, breaking on the lower-priority operators
makes it easier to read the grouping:

    if (it->it_data &&
        it->it_age + CACHED_PAGE_LIFETIME > current_age) {

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 04/10] XBZRLE: rebuild the cache_is_cached function

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Rebuild the cache_is_cached function.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 page_cache.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/page_cache.c b/page_cache.c
index 74d9f8e..6fc4334 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -124,24 +124,6 @@ static size_t cache_get_cache_pos(const PageCache *cache,
     return pos;
 }
 
-bool cache_is_cached(const PageCache *cache, uint64_t addr,
-                     uint64_t current_age)
-{
-    size_t pos;
-
-    g_assert(cache);
-    g_assert(cache->page_cache);
-
-    pos = cache_get_cache_pos(cache, addr);
-
-    if (cache->page_cache[pos].it_addr == addr) {
-        /* update the it_age when the cache hit */
-        cache->page_cache[pos].it_age = current_age;
-        return true;
-    }
-    return false;
-}
-
 static CacheItem *cache_get_by_addr(const PageCache *cache, uint64_t addr)
 {
     size_t pos;
@@ -159,6 +141,21 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr)
     return cache_get_by_addr(cache, addr)->it_data;
 }
 
+bool cache_is_cached(const PageCache *cache, uint64_t addr,
+                     uint64_t current_age)
+{
+    CacheItem *it = NULL;
+
+    it = cache_get_by_addr(cache, addr);
+
+    if (it->it_addr == addr) {
+        /* updata the it_age when the cache hit */
+        it->it_age = current_age;
+        return true;
+    }
+    return false;
+}
+
 int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
                  uint64_t current_age)
 {
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 04/10] XBZRLE: rebuild the cache_is_cached function

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 
> Rebuild the cache_is_cached function.
> 
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Once again, I did NOT give R-b in v1.

> ---
>  page_cache.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
> 

> +    if (it->it_addr == addr) {
> +        /* updata the it_age when the cache hit */

Furthermore, I made a review comment in v1 that you had a typo, but it
went unchanged in this version:

s/updata/update/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 04/10] XBZRLE: rebuild the cache_is_cached function

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Rebuild the cache_is_cached function.

Why?

> +bool cache_is_cached(const PageCache *cache, uint64_t addr,
> +                     uint64_t current_age)
> +{
> +    CacheItem *it = NULL;

s/= NULL//

Thanks, Juan.

> +
> +    it = cache_get_by_addr(cache, addr);
> +
> +    if (it->it_addr == addr) {
> +        /* updata the it_age when the cache hit */
> +        it->it_age = current_age;
> +        return true;
> +    }
> +    return false;
> +}
> +
>  int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
>                   uint64_t current_age)
>  {

[Qemu-devel] [PATCH 05/10] migration: Fix the migrate auto converge process

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

It is inaccuracy and complex that using the transfer speed of
migration thread to determine whether the convergence migration.
The dirty page may be compressed by XBZRLE or ZERO_PAGE.The counter
of updating dirty bitmap will be increasing continuously if the
migration can't convergence.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index c6a70b8..d79535c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -107,7 +107,6 @@ int graphic_depth = 32;
 
 const uint32_t arch_type = QEMU_ARCH;
 static bool mig_throttle_on;
-static int dirty_rate_high_cnt;
 static void check_guest_throttling(void);
 
 static uint64_t bitmap_sync_cnt;
@@ -501,17 +500,11 @@ static void migration_bitmap_sync(void)
     uint64_t num_dirty_pages_init = migration_dirty_pages;
     MigrationState *s = migrate_get_current();
     static int64_t start_time;
-    static int64_t bytes_xfer_prev;
     static int64_t num_dirty_pages_period;
     int64_t end_time;
-    int64_t bytes_xfer_now;
 
     increase_bitmap_sync_cnt();
 
-    if (!bytes_xfer_prev) {
-        bytes_xfer_prev = ram_bytes_transferred();
-    }
-
     if (!start_time) {
         start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
     }
@@ -530,21 +523,11 @@ static void migration_bitmap_sync(void)
     /* more than 1 second = 1000 millisecons */
     if (end_time > start_time + 1000) {
         if (migrate_auto_converge()) {
-            /* The following detection logic can be refined later. For now:
-               Check to see if the dirtied bytes is 50% more than the approx.
-               amount of bytes that just got transferred since the last time we
-               were in this routine. If that happens >N times (for now N==4)
-               we turn on the throttle down logic */
-            bytes_xfer_now = ram_bytes_transferred();
-            if (s->dirty_pages_rate &&
-               (num_dirty_pages_period * TARGET_PAGE_SIZE >
-                   (bytes_xfer_now - bytes_xfer_prev)/2) &&
-               (dirty_rate_high_cnt++ > 4)) {
-                    trace_migration_throttle();
-                    mig_throttle_on = true;
-                    dirty_rate_high_cnt = 0;
-             }
-             bytes_xfer_prev = bytes_xfer_now;
+            if (get_bitmap_sync_cnt() > 15) {
+                /* It indicates that migration can't converge when the counter
+                is larger than fifteen. Enable the feature of auto converge */
+                mig_throttle_on = true;
+            }
         } else {
              mig_throttle_on = false;
         }
@@ -754,7 +737,6 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     bitmap_set(migration_bitmap, 0, ram_pages);
     migration_dirty_pages = ram_pages;
     mig_throttle_on = false;
-    dirty_rate_high_cnt = 0;
     reset_bitmap_sync_cnt();
 
     if (migrate_use_xbzrle()) {
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 05/10] migration: Fix the migrate auto converge process

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> It is inaccuracy and complex that using the transfer speed of
> migration thread to determine whether the convergence migration.
> The dirty page may be compressed by XBZRLE or ZERO_PAGE.The counter
> of updating dirty bitmap will be increasing continuously if the
> migration can't convergence.

"It is inexact and complex to use the migration transfer speed to
dectermine weather the convergence of migration."

> @@ -530,21 +523,11 @@ static void migration_bitmap_sync(void)
>      /* more than 1 second = 1000 millisecons */
>      if (end_time > start_time + 1000) {
>          if (migrate_auto_converge()) {
> -            /* The following detection logic can be refined later. For now:
> -               Check to see if the dirtied bytes is 50% more than the approx.
> -               amount of bytes that just got transferred since the last time we
> -               were in this routine. If that happens >N times (for now N==4)
> -               we turn on the throttle down logic */
> -            bytes_xfer_now = ram_bytes_transferred();
> -            if (s->dirty_pages_rate &&
> -               (num_dirty_pages_period * TARGET_PAGE_SIZE >
> -                   (bytes_xfer_now - bytes_xfer_prev)/2) &&
> -               (dirty_rate_high_cnt++ > 4)) {
> -                    trace_migration_throttle();
> -                    mig_throttle_on = true;
> -                    dirty_rate_high_cnt = 0;
> -             }
> -             bytes_xfer_prev = bytes_xfer_now;
> +            if (get_bitmap_sync_cnt() > 15) {
> +                /* It indicates that migration can't converge when the counter
> +                is larger than fifteen. Enable the feature of auto
>      converge */

Comment is not needed, it says excatly what the code does.

But why 15?  It is not that I think that the older code is better or
worse than yours.  Just that we move from one magic number to another
(that is even bigger).

Shouldn't it be easier jut just change mig_sleep_cpu()

to do something like:


static void mig_sleep_cpu(void *opq)
{
    qemu_mutex_unlock_iothread();
    g_usleep((2*get_bitmap_sync_cnt()*1000);
    qemu_mutex_lock_iothread();
}

This would get the 30ms on the 15th iteration.  I am open to change that
formula to anything different, but what I want is changing this to
something that makes the less convergence -> the more throotling.

BTW, you are testing this with any workload to see that it improves?


> +                mig_throttle_on = true;
> +            }

Vinod, what do you think?

Do you have a workload to test this?

Thanks, Juan.

Re: [Qemu-devel] [PATCH 05/10] migration: Fix the migrate auto converge process

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On 03/11/2014 02:48 PM, Juan Quintela wrote:
> <arei.gonglei@huawei.com> wrote:
>> From: ChenLiang <chenliang88@huawei.com>
>>
>> It is inaccuracy and complex that using the transfer speed of
>> migration thread to determine whether the convergence migration.
>> The dirty page may be compressed by XBZRLE or ZERO_PAGE.The counter
>> of updating dirty bitmap will be increasing continuously if the
>> migration can't convergence.
> 
> "It is inexact and complex to use the migration transfer speed to
> dectermine weather the convergence of migration."

or even better (plus spelling fixes):

determine whether migration is converging

> 
>> @@ -530,21 +523,11 @@ static void migration_bitmap_sync(void)
>>      /* more than 1 second = 1000 millisecons */

Pre-existing, but as long as you are touching this area of code:
s/millisecons/milliseconds/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 05/10] migration: Fix the migrate auto converge process

[Chegu Vinod]

On 3/11/2014 1:48 PM, Juan Quintela wrote:
> <arei.gonglei@huawei.com> wrote:
>> From: ChenLiang <chenliang88@huawei.com>
>>
>> It is inaccuracy and complex that using the transfer speed of
>> migration thread to determine whether the convergence migration.
>> The dirty page may be compressed by XBZRLE or ZERO_PAGE.The counter
>> of updating dirty bitmap will be increasing continuously if the
>> migration can't convergence.
> "It is inexact and complex to use the migration transfer speed to
> dectermine weather the convergence of migration."
>
>> @@ -530,21 +523,11 @@ static void migration_bitmap_sync(void)
>>       /* more than 1 second = 1000 millisecons */
>>       if (end_time > start_time + 1000) {
>>           if (migrate_auto_converge()) {
>> -            /* The following detection logic can be refined later. For now:
>> -               Check to see if the dirtied bytes is 50% more than the approx.
>> -               amount of bytes that just got transferred since the last time we
>> -               were in this routine. If that happens >N times (for now N==4)
>> -               we turn on the throttle down logic */
>> -            bytes_xfer_now = ram_bytes_transferred();
>> -            if (s->dirty_pages_rate &&
>> -               (num_dirty_pages_period * TARGET_PAGE_SIZE >
>> -                   (bytes_xfer_now - bytes_xfer_prev)/2) &&
>> -               (dirty_rate_high_cnt++ > 4)) {
>> -                    trace_migration_throttle();
>> -                    mig_throttle_on = true;
>> -                    dirty_rate_high_cnt = 0;
>> -             }
>> -             bytes_xfer_prev = bytes_xfer_now;
>> +            if (get_bitmap_sync_cnt() > 15) {
>> +                /* It indicates that migration can't converge when the counter
>> +                is larger than fifteen. Enable the feature of auto
>>       converge */
> Comment is not needed, it says excatly what the code does.
>
> But why 15?  It is not that I think that the older code is better or
> worse than yours.  Just that we move from one magic number to another
> (that is even bigger).
>
> Shouldn't it be easier jut just change mig_sleep_cpu()
>
> to do something like:
>
>
> static void mig_sleep_cpu(void *opq)
> {
>      qemu_mutex_unlock_iothread();
>      g_usleep((2*get_bitmap_sync_cnt()*1000);
>      qemu_mutex_lock_iothread();
> }
>
> This would get the 30ms on the 15th iteration.  I am open to change that
> formula to anything different, but what I want is changing this to
> something that makes the less convergence -> the more throotling.

<  'already got some feedback earlier on this and had this task in the 
list of things
     to work on... :)   >

Having the throttling start with some per-defined "degree" and then have 
that "degree" gradually increase ...either

a) automatically as shown in Juan's example above (or)

b) via some TBD user level interface...

...is one way to help with ensuring convergence for all cases.

The issue of continuing to increase this "degree" of throttling is an 
obvious area of concern for the workload ( that is still trying to run 
in the VM).   Would it it better to force the live migration to switch 
from the iterative pre-copy phase to the "downtime" phase ...if it fails 
to converge even after throttling it for a couple of iterations ?  Doing 
so could result in a longer actual downtime. Hope to try this and 
see...  but if anyone has inputs(other than doing post-copy etc) pl. do 
share.


>
> BTW, you are testing this with any workload to see that it improves?

Yes. Please do share some data.

>
>
>> +                mig_throttle_on = true;
>> +            }
> Vinod, what do you think?
As is noted in the current code...the "logic" to detect the lack of 
convergence needs to be refined. If there is a better way to help detect 
same (and which covers these other cases like XBZRLE etc) then I am all 
for it.  I do agree with Juan about the choice of magic numbers (i.e. 
one may not be better than the other).

BTW, on a related note...

I haven't used XBZRLE in the recent past (after having tried it in the 
early days). Does it now perform well with larger sized VMs running real 
world workloads  ?   Assume that is where you found that there was still 
need for forcing convergence ?

Pl. do consider sharing some results about the type of workload and also 
the size of the VMs etc that you have tried with XBZRLE.

> Do you have a workload to test this?

Hmm... One can test this with memory intensive Java warehouse type of 
workloads (besides using synthetic workloads).

Vinod

> Thanks, Juan.
> .
>

Re: [Qemu-devel] [PATCH 05/10] migration: Fix the migrate auto converge process

[Gonglei (Arei)]

> <  'already got some feedback earlier on this and had this task in the
> list of things
>      to work on... :)   >
> 
> Having the throttling start with some per-defined "degree" and then have
> that "degree" gradually increase ...either
> 
> a) automatically as shown in Juan's example above (or)
> 
> b) via some TBD user level interface...
> 
> ...is one way to help with ensuring convergence for all cases.
> 
> The issue of continuing to increase this "degree" of throttling is an
> obvious area of concern for the workload ( that is still trying to run
> in the VM).   Would it it better to force the live migration to switch
> from the iterative pre-copy phase to the "downtime" phase ...if it fails
> to converge even after throttling it for a couple of iterations ?  Doing
> so could result in a longer actual downtime. Hope to try this and
> see...  but if anyone has inputs(other than doing post-copy etc) pl. do
> share.
> 
> 
> >
> > BTW, you are testing this with any workload to see that it improves?
> 
> Yes. Please do share some data.
> 
> >
> >
> >> +                mig_throttle_on = true;
> >> +            }
> > Vinod, what do you think?
> As is noted in the current code...the "logic" to detect the lack of
> convergence needs to be refined. If there is a better way to help detect
> same (and which covers these other cases like XBZRLE etc) then I am all
> for it.  I do agree with Juan about the choice of magic numbers (i.e.
> one may not be better than the other).
> 
> BTW, on a related note...
> 
> I haven't used XBZRLE in the recent past (after having tried it in the
> early days). Does it now perform well with larger sized VMs running real
> world workloads  ?   Assume that is where you found that there was still
> need for forcing convergence ?
> 
> Pl. do consider sharing some results about the type of workload and also
> the size of the VMs etc that you have tried with XBZRLE.
> 
> > Do you have a workload to test this?
> 
> Hmm... One can test this with memory intensive Java warehouse type of
> workloads (besides using synthetic workloads).
> 
> Vinod
> 
It will be more robust no matter whether xbzrle is enable. This patch don't aim at performance.
BTW, the migration transfer speed which used in migration_thread also has same problem. 
I want to fix it, do you have any suggestion? Thanks.

> > Thanks, Juan.
> > .
> >

Best regards,
-Gonglei

[Qemu-devel] [PATCH 06/10] migraion: optimiztion xbzrle by reducing data copy

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Reducing data copy can reduce cpu overhead.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index d79535c..7ef3ebb 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -381,11 +381,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
 
     prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
 
-    /* save current buffer into memory */
-    memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE);
-
     /* XBZRLE encoding (if there is no overflow) */
-    encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
+    encoded_len = xbzrle_encode_buffer(prev_cached_page, *current_data,
                                        TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
                                        TARGET_PAGE_SIZE);
     if (encoded_len == 0) {
@@ -404,7 +401,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
 
     /* we need to update the data in the cache, in order to get the same data */
     if (!last_stage) {
-        memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+        xbzrle_decode_buffer(XBZRLE.encoded_buf, encoded_len, prev_cached_page,
+                             TARGET_PAGE_SIZE);
     }
 
     /* Send XBZRLE based compressed page */
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 06/10] migraion: optimiztion xbzrle by reducing data copy

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Reducing data copy can reduce cpu overhead.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>


Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH 06/10] migraion: optimiztion xbzrle by reducing data copy

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Reducing data copy can reduce cpu overhead.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH 06/10] migraion: optimiztion xbzrle by reducing data copy

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1615 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>

s/migraion/migration/ in the subject (here and in 7/10)

s/optimiztion/optimize/ in the subject

> +++ b/arch_init.c
> @@ -381,11 +381,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
>  
>      prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
>  
> -    /* save current buffer into memory */
> -    memcpy(XBZRLE.current_buf, *current_data, TARGET_PAGE_SIZE);
> -
>      /* XBZRLE encoding (if there is no overflow) */
> -    encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
> +    encoded_len = xbzrle_encode_buffer(prev_cached_page, *current_data,
>                                         TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
>                                         TARGET_PAGE_SIZE);
>      if (encoded_len == 0) {
> @@ -404,7 +401,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
>  
>      /* we need to update the data in the cache, in order to get the same data */
>      if (!last_stage) {
> -        memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
> +        xbzrle_decode_buffer(XBZRLE.encoded_buf, encoded_len, prev_cached_page,
> +                             TARGET_PAGE_SIZE);

So, is XBZRLE.current_buf even needed after this patch?  Oh, patch 7
kills it.  I guess splitting in two patches is okay, although I
personally would have squashed them into one.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 07/10] migraion: clear the death code

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Clear the death code

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c  | 13 -------------
 page_cache.c | 58 ----------------------------------------------------------
 2 files changed, 71 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 7ef3ebb..461a10a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -178,14 +178,11 @@ static inline bool is_zero_range(uint8_t *p, uint64_t size)
 static struct {
     /* buffer used for XBZRLE encoding */
     uint8_t *encoded_buf;
-    /* buffer for storing page content */
-    uint8_t *current_buf;
     /* Cache for XBZRLE, Protected by lock. */
     PageCache *cache;
     QemuMutex lock;
 } XBZRLE = {
     .encoded_buf = NULL,
-    .current_buf = NULL,
     .cache = NULL,
 };
 /* buffer used for XBZRLE decoding */
@@ -702,10 +699,8 @@ static void migration_end(void)
         cache_fini(XBZRLE.cache);
         g_free(XBZRLE.cache);
         g_free(XBZRLE.encoded_buf);
-        g_free(XBZRLE.current_buf);
         XBZRLE.cache = NULL;
         XBZRLE.encoded_buf = NULL;
-        XBZRLE.current_buf = NULL;
     }
     XBZRLE_cache_unlock();
 }
@@ -757,14 +752,6 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
             return -1;
         }
 
-        XBZRLE.current_buf = g_try_malloc(TARGET_PAGE_SIZE);
-        if (!XBZRLE.current_buf) {
-            DPRINTF("Error allocating current_buf\n");
-            g_free(XBZRLE.encoded_buf);
-            XBZRLE.encoded_buf = NULL;
-            return -1;
-        }
-
         acct_clear();
     }
 
diff --git a/page_cache.c b/page_cache.c
index 6fc4334..579330c 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -48,7 +48,6 @@ struct PageCache {
     CacheItem *page_cache;
     unsigned int page_size;
     int64_t max_num_items;
-    uint64_t max_item_age;
     int64_t num_items;
 };
 
@@ -76,7 +75,6 @@ PageCache *cache_init(int64_t num_pages, unsigned int page_size)
     }
     cache->page_size = page_size;
     cache->num_items = 0;
-    cache->max_item_age = 0;
     cache->max_num_items = num_pages;
 
     DPRINTF("Setting cache buckets to %" PRId64 "\n", cache->max_num_items);
@@ -190,59 +188,3 @@ int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
 
     return 0;
 }
-
-int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
-{
-    PageCache *new_cache;
-    int64_t i;
-
-    CacheItem *old_it, *new_it;
-
-    g_assert(cache);
-
-    /* cache was not inited */
-    if (cache->page_cache == NULL) {
-        return -1;
-    }
-
-    /* same size */
-    if (pow2floor(new_num_pages) == cache->max_num_items) {
-        return cache->max_num_items;
-    }
-
-    new_cache = cache_init(new_num_pages, cache->page_size);
-    if (!(new_cache)) {
-        DPRINTF("Error creating new cache\n");
-        return -1;
-    }
-
-    /* move all data from old cache */
-    for (i = 0; i < cache->max_num_items; i++) {
-        old_it = &cache->page_cache[i];
-        if (old_it->it_addr != -1) {
-            /* check for collision, if there is, keep MRU page */
-            new_it = cache_get_by_addr(new_cache, old_it->it_addr);
-            if (new_it->it_data && new_it->it_age >= old_it->it_age) {
-                /* keep the MRU page */
-                g_free(old_it->it_data);
-            } else {
-                if (!new_it->it_data) {
-                    new_cache->num_items++;
-                }
-                g_free(new_it->it_data);
-                new_it->it_data = old_it->it_data;
-                new_it->it_age = old_it->it_age;
-                new_it->it_addr = old_it->it_addr;
-            }
-        }
-    }
-
-    g_free(cache->page_cache);
-    cache->page_cache = new_cache->page_cache;
-    cache->max_num_items = new_cache->max_num_items;
-    cache->num_items = new_cache->num_items;
-
-    g_free(new_cache);
-
-    return cache->max_num_items;
-}
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 07/10] migraion: clear the death code

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Clear the death code
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH 07/10] migraion: clear the death code

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 
> Clear the death code

s/death/dead/ (twice)

> 
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  arch_init.c  | 13 -------------
>  page_cache.c | 58 ----------------------------------------------------------
>  2 files changed, 71 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 08/10] migration: s/uint64_t/int64_t the definitions of it_age

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c                    | 4 ++--
 include/migration/page_cache.h | 4 ++--
 page_cache.c                   | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 461a10a..1c1488a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -109,7 +109,7 @@ const uint32_t arch_type = QEMU_ARCH;
 static bool mig_throttle_on;
 static void check_guest_throttling(void);
 
-static uint64_t bitmap_sync_cnt;
+static int64_t bitmap_sync_cnt;
 /* the functions *_bitmap_sync_cnt only run in migrate thread */
 static inline void reset_bitmap_sync_cnt(void)
 {
@@ -121,7 +121,7 @@ static inline void increase_bitmap_sync_cnt(void)
     bitmap_sync_cnt++;
 }
 
-static inline uint64_t get_bitmap_sync_cnt(void)
+static inline int64_t get_bitmap_sync_cnt(void)
 {
     return bitmap_sync_cnt;
 }
diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
index dc0c6b5..34518ba 100644
--- a/include/migration/page_cache.h
+++ b/include/migration/page_cache.h
@@ -46,7 +46,7 @@ void cache_fini(PageCache *cache);
  * @current_age indicate the age of the page if cache hit
  */
 bool cache_is_cached(const PageCache *cache, uint64_t addr,
-                     uint64_t current_age);
+                     int64_t current_age);
 
 /**
  * get_cached_data: Get the data cached for an addr
@@ -70,7 +70,7 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
  * @current_age indicate the age of the page if the page is inserted into cache
  */
 int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
-                 uint64_t current_age);
+                 int64_t current_age);
 
 /**
  * cache_resize: resize the page cache. In case of size reduction the extra
diff --git a/page_cache.c b/page_cache.c
index 579330c..b32afdc 100644
--- a/page_cache.c
+++ b/page_cache.c
@@ -40,7 +40,7 @@ typedef struct CacheItem CacheItem;
 
 struct CacheItem {
     uint64_t it_addr;
-    uint64_t it_age;
+    int64_t it_age;
     uint8_t *it_data;
 };
 
@@ -140,7 +140,7 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr)
 }
 
 bool cache_is_cached(const PageCache *cache, uint64_t addr,
-                     uint64_t current_age)
+                     int64_t current_age)
 {
     CacheItem *it = NULL;
 
@@ -155,7 +155,7 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr,
 }
 
 int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
-                 uint64_t current_age)
+                 int64_t current_age)
 {
 
     CacheItem *it = NULL;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 08/10] migration: s/uint64_t/int64_t the definitions of it_age

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 

The subject line says what, but there is no WHY.  The commit message
should explain why you want a signed type.

> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  arch_init.c                    | 4 ++--
>  include/migration/page_cache.h | 4 ++--
>  page_cache.c                   | 6 +++---
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 461a10a..1c1488a 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -109,7 +109,7 @@ const uint32_t arch_type = QEMU_ARCH;
>  static bool mig_throttle_on;
>  static void check_guest_throttling(void);
>  
> -static uint64_t bitmap_sync_cnt;
> +static int64_t bitmap_sync_cnt;

Wait.  Patch 2/10 introduced this variable.  Why not introduce it with
the correct type to begin with?  Please reorder your series so that you
aren't churning on new code.


> -static inline uint64_t get_bitmap_sync_cnt(void)
> +static inline int64_t get_bitmap_sync_cnt(void)
>  {
>      return bitmap_sync_cnt;

Not to mention that my argument in patch 2 still holds - unless you are
going to export this function, it is pointless compared to just using
the variable directly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 08/10] migration: s/uint64_t/int64_t the definitions of it_age

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>


You are changing teh types introduced in patch2, please fix them there?
Anyways, why are you changing the age to int64_t?  Not that I expect it
to be so big, but I would expect it not to be negative?

Later, Juan.

> ---
>  arch_init.c                    | 4 ++--
>  include/migration/page_cache.h | 4 ++--
>  page_cache.c                   | 6 +++---
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 461a10a..1c1488a 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -109,7 +109,7 @@ const uint32_t arch_type = QEMU_ARCH;
>  static bool mig_throttle_on;
>  static void check_guest_throttling(void);
>  
> -static uint64_t bitmap_sync_cnt;
> +static int64_t bitmap_sync_cnt;
>  /* the functions *_bitmap_sync_cnt only run in migrate thread */
>  static inline void reset_bitmap_sync_cnt(void)
>  {
> @@ -121,7 +121,7 @@ static inline void increase_bitmap_sync_cnt(void)
>      bitmap_sync_cnt++;
>  }
>  
> -static inline uint64_t get_bitmap_sync_cnt(void)
> +static inline int64_t get_bitmap_sync_cnt(void)
>  {
>      return bitmap_sync_cnt;
>  }
> diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h
> index dc0c6b5..34518ba 100644
> --- a/include/migration/page_cache.h
> +++ b/include/migration/page_cache.h
> @@ -46,7 +46,7 @@ void cache_fini(PageCache *cache);
>   * @current_age indicate the age of the page if cache hit
>   */
>  bool cache_is_cached(const PageCache *cache, uint64_t addr,
> -                     uint64_t current_age);
> +                     int64_t current_age);
>  
>  /**
>   * get_cached_data: Get the data cached for an addr
> @@ -70,7 +70,7 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
>   * @current_age indicate the age of the page if the page is inserted into cache
>   */
>  int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
> -                 uint64_t current_age);
> +                 int64_t current_age);
>  
>  /**
>   * cache_resize: resize the page cache. In case of size reduction the extra
> diff --git a/page_cache.c b/page_cache.c
> index 579330c..b32afdc 100644
> --- a/page_cache.c
> +++ b/page_cache.c
> @@ -40,7 +40,7 @@ typedef struct CacheItem CacheItem;
>  
>  struct CacheItem {
>      uint64_t it_addr;
> -    uint64_t it_age;
> +    int64_t it_age;
>      uint8_t *it_data;
>  };
>  
> @@ -140,7 +140,7 @@ uint8_t *get_cached_data(const PageCache *cache, uint64_t addr)
>  }
>  
>  bool cache_is_cached(const PageCache *cache, uint64_t addr,
> -                     uint64_t current_age)
> +                     int64_t current_age)
>  {
>      CacheItem *it = NULL;
>  
> @@ -155,7 +155,7 @@ bool cache_is_cached(const PageCache *cache, uint64_t addr,
>  }
>  
>  int cache_insert(PageCache *cache, uint64_t addr, const uint8_t *pdata,
> -                 uint64_t current_age)
> +                 int64_t current_age)
>  {
>  
>      CacheItem *it = NULL;

[Qemu-devel] [PATCH 09/10] migration: expose the bitmap_sync_cnt to the end user

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

expose the counter that log the times of updating the dirty bitmap to
end user.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c                   | 1 +
 hmp.c                         | 2 ++
 include/migration/migration.h | 1 +
 migration.c                   | 2 ++
 qapi-schema.json              | 4 +++-
 qmp-commands.hx               | 5 +++++
 6 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch_init.c b/arch_init.c
index 1c1488a..77fb168 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -531,6 +531,7 @@ static void migration_bitmap_sync(void)
         s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE;
         start_time = end_time;
         num_dirty_pages_period = 0;
+        s->dirty_sync_cnt = get_bitmap_sync_cnt();
     }
 }
 
diff --git a/hmp.c b/hmp.c
index 2f279c4..843174d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -188,6 +188,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->ram->normal);
         monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
                        info->ram->normal_bytes >> 10);
+        monitor_printf(mon, "dirty sync cnt: %" PRIu64 "\n",
+                       info->ram->dirty_sync_cnt);
         if (info->ram->dirty_pages_rate) {
             monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
                            info->ram->dirty_pages_rate);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 3e1e6c7..a45affd 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -61,6 +61,7 @@ struct MigrationState
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
     int64_t setup_time;
+    int64_t dirty_sync_cnt;
 };
 
 void process_incoming_migration(QEMUFile *f);
diff --git a/migration.c b/migration.c
index 14235b2..4c9bc89 100644
--- a/migration.c
+++ b/migration.c
@@ -220,6 +220,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->ram->normal_bytes = norm_mig_bytes_transferred();
         info->ram->dirty_pages_rate = s->dirty_pages_rate;
         info->ram->mbps = s->mbps;
+        info->ram->dirty_sync_cnt = s->dirty_sync_cnt;
 
         if (blk_mig_active()) {
             info->has_disk = true;
@@ -253,6 +254,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
         info->ram->mbps = s->mbps;
+        info->ram->dirty_sync_cnt = s->dirty_sync_cnt;
         break;
     case MIG_STATE_ERROR:
         info->has_status = true;
diff --git a/qapi-schema.json b/qapi-schema.json
index 6c381b7..0bcad54 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -651,13 +651,15 @@
 #
 # @mbps: throughput in megabits/sec. (since 1.6)
 #
+# @dirty-sync-cnt: the times of ram dirty sync (since 1.7)
+#
 # Since: 0.14.0
 ##
 { 'type': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
            'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
            'normal-bytes': 'int', 'dirty-pages-rate' : 'int',
-           'mbps' : 'number' } }
+           'mbps' : 'number', 'dirty-sync-cnt' : 'int' } }
 
 ##
 # @XBZRLECacheStats
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d982cd6..47fc900 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2928,6 +2928,7 @@ The main json-object contains the following:
             pages. This is just normal pages times size of one page,
             but this way upper levels don't need to care about page
             size (json-int)
+         - "dirty-sync-cnt": the times of ram dirty sync (json-int)
 - "disk": only present if "status" is "active" and it is a block migration,
   it is a json-object with the following disk information:
          - "transferred": amount transferred in bytes (json-int)
@@ -2966,6 +2967,7 @@ Examples:
           "duplicate":123,
           "normal":123,
           "normal-bytes":123456
+          "dirty-sync-cnt":15
         }
      }
    }
@@ -2991,6 +2993,7 @@ Examples:
             "duplicate":123,
             "normal":123,
             "normal-bytes":123456
+            "dirty-sync-cnt":15
          }
       }
    }
@@ -3011,6 +3014,7 @@ Examples:
             "duplicate":123,
             "normal":123,
             "normal-bytes":123456
+            "dirty-sync-cnt":15
          },
          "disk":{
             "total":20971520,
@@ -3037,6 +3041,7 @@ Examples:
             "duplicate":10,
             "normal":3333,
             "normal-bytes":3412992
+            "dirty-sync-cnt":15
          },
          "xbzrle-cache":{
             "cache-size":67108864,
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 09/10] migration: expose the bitmap_sync_cnt to the end user

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1389 bytes --]

On 03/11/2014 06:53 AM, arei.gonglei@huawei.com wrote:
> From: ChenLiang <chenliang88@huawei.com>
> 
> expose the counter that log the times of updating the dirty bitmap to
> end user.
> 
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---

> +++ b/qapi-schema.json
> @@ -651,13 +651,15 @@
>  #
>  # @mbps: throughput in megabits/sec. (since 1.6)
>  #
> +# @dirty-sync-cnt: the times of ram dirty sync (since 1.7)

The earliest this will make it in is 2.0 - and that's if it is treated
as a bug fix.  Otherwise, this needs to be '(since 2.1)'.

'cnt' is an abbreviation; it doesn't hurt to spell things out to say
'dirty-sync-count'.


> @@ -2966,6 +2967,7 @@ Examples:
>            "duplicate":123,
>            "normal":123,
>            "normal-bytes":123456
> +          "dirty-sync-cnt":15

Invalid JSON.  If dirty-sync-cnt is present, "normal-bytes":123456 must
have a comma separating the two elements.

>          }
>       }
>     }
> @@ -2991,6 +2993,7 @@ Examples:
>              "duplicate":123,
>              "normal":123,
>              "normal-bytes":123456
> +            "dirty-sync-cnt":15

And again.  All of your insertions to this example need correction.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 09/10] migration: expose the bitmap_sync_cnt to the end user

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> expose the counter that log the times of updating the dirty bitmap to
> end user.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Once Eric problems are fixed, rest of the patch looks ok.

Later, Juan.

[Qemu-devel] [PATCH 10/10] XBZRLE: update the doc of XBZRLE

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 docs/xbzrle.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
index cc3a26a..cdf1e3e 100644
--- a/docs/xbzrle.txt
+++ b/docs/xbzrle.txt
@@ -71,6 +71,13 @@ encoded buffer:
 encoded length 24
 e9 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 03 01 67 01 01 69
 
+The strategy of updating cache
+=================================
+Keeping the hot page in cache is effective to decrease cache missing.
+XBZRLE use a counter as the age of page. The counter will increase
+after the ram dirty bitmap syncing. When cache conflicts XBZRLE only
+replace the old page in cache.
+
 Usage
 ======================
 1. Verify the destination QEMU version is able to decode the new format.
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 10/10] XBZRLE: update the doc of XBZRLE

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  docs/xbzrle.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
> index cc3a26a..cdf1e3e 100644
> --- a/docs/xbzrle.txt
> +++ b/docs/xbzrle.txt
> @@ -71,6 +71,13 @@ encoded buffer:
>  encoded length 24
>  e9 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 03 01 67 01 01 69
>  
> +The strategy of updating cache
> +=================================
> +Keeping the hot page in cache is effective to decrease cache missing.
> +XBZRLE use a counter as the age of page. The counter will increase
> +after the ram dirty bitmap syncing. When cache conflicts XBZRLE only
> +replace the old page in cache.
> +
>  Usage
>  ======================
>  1. Verify the destination QEMU version is able to decode the new format.

You can merge this one with patch3, but not important.

Thanks, Juan.