All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] fix migration of zero pages
@ 2013-06-06  8:36 Peter Lieven
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage" Peter Lieven
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages Peter Lieven
  0 siblings, 2 replies; 9+ messages in thread
From: Peter Lieven @ 2013-06-06  8:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: aik, pbonzini, Peter Lieven, quintela

There have been reports that migration is broken on pseries by Alexey Kardashevskiy.
It turned out that migration will fail in general as soon as a page is zero on the
source, but not on the destination. I thus reverted the skipping of zero pages
in bulk transfer phase and added a patch that does not (over)write zero pages
at the destination.

Peter Lieven (2):
  Revert "migration: do not sent zero pages in bulk stage"
  migration: do not overwrite zero pages

 arch_init.c                   |   38 ++++++++++++--------------------------
 hmp.c                         |    2 --
 include/migration/migration.h |    2 --
 migration.c                   |    3 +--
 qapi-schema.json              |    8 +++-----
 qmp-commands.hx               |    1 -
 6 files changed, 16 insertions(+), 38 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"
  2013-06-06  8:36 [Qemu-devel] [PATCH 0/2] fix migration of zero pages Peter Lieven
@ 2013-06-06  8:36 ` Peter Lieven
  2013-06-06 12:16   ` Eric Blake
  2013-06-19 10:10   ` Paolo Bonzini
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages Peter Lieven
  1 sibling, 2 replies; 9+ messages in thread
From: Peter Lieven @ 2013-06-06  8:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: aik, pbonzini, Peter Lieven, quintela

Not sending zero pages breaks migration if a page is zero
at the source but not at the destination. This can e.g. happen
if different BIOS versions are used at source and destination.
It has also been reported that migration on pseries is completely
broken with this patch.

This reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.

Conflicts:

	arch_init.c
	qmp-commands.hx

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 arch_init.c                   |   24 ++++--------------------
 hmp.c                         |    2 --
 include/migration/migration.h |    2 --
 migration.c                   |    3 +--
 qapi-schema.json              |    8 +++-----
 qmp-commands.hx               |    1 -
 6 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..283081d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -183,7 +183,6 @@ int64_t xbzrle_cache_resize(int64_t new_size)
 /* accounting for migration statistics */
 typedef struct AccountingInfo {
     uint64_t dup_pages;
-    uint64_t skipped_pages;
     uint64_t norm_pages;
     uint64_t iterations;
     uint64_t xbzrle_bytes;
@@ -209,16 +208,6 @@ uint64_t dup_mig_pages_transferred(void)
     return acct_info.dup_pages;
 }
 
-uint64_t skipped_mig_bytes_transferred(void)
-{
-    return acct_info.skipped_pages * TARGET_PAGE_SIZE;
-}
-
-uint64_t skipped_mig_pages_transferred(void)
-{
-    return acct_info.skipped_pages;
-}
-
 uint64_t norm_mig_bytes_transferred(void)
 {
     return acct_info.norm_pages * TARGET_PAGE_SIZE;
@@ -457,15 +446,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
             bytes_sent = -1;
             if (is_zero_page(p)) {
                 acct_info.dup_pages++;
-                if (!ram_bulk_stage) {
-                    bytes_sent = save_block_hdr(f, block, offset, cont,
-                                                RAM_SAVE_FLAG_COMPRESS);
-                    qemu_put_byte(f, 0);
-                    bytes_sent++;
-                } else {
-                    acct_info.skipped_pages++;
-                    bytes_sent = 0;
-                }
+                bytes_sent = save_block_hdr(f, block, offset, cont,
+                                            RAM_SAVE_FLAG_COMPRESS);
+                qemu_put_byte(f, 0);
+                bytes_sent++;
             } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
                 current_addr = block->offset + offset;
                 bytes_sent = save_xbzrle_page(f, p, current_addr, block,
diff --git a/hmp.c b/hmp.c
index 4fb76ec..55c4423 100644
--- a/hmp.c
+++ b/hmp.c
@@ -173,8 +173,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->ram->total >> 10);
         monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
                        info->ram->duplicate);
-        monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
-                       info->ram->skipped);
         monitor_printf(mon, "normal: %" PRIu64 " pages\n",
                        info->ram->normal);
         monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
diff --git a/include/migration/migration.h b/include/migration/migration.h
index e2acec6..bb617fd 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -96,8 +96,6 @@ extern SaveVMHandlers savevm_ram_handlers;
 
 uint64_t dup_mig_bytes_transferred(void);
 uint64_t dup_mig_pages_transferred(void);
-uint64_t skipped_mig_bytes_transferred(void);
-uint64_t skipped_mig_pages_transferred(void);
 uint64_t norm_mig_bytes_transferred(void);
 uint64_t norm_mig_pages_transferred(void);
 uint64_t xbzrle_mig_bytes_transferred(void);
diff --git a/migration.c b/migration.c
index 058f9e6..c305d3e 100644
--- a/migration.c
+++ b/migration.c
@@ -197,11 +197,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->ram->remaining = ram_bytes_remaining();
         info->ram->total = ram_bytes_total();
         info->ram->duplicate = dup_mig_pages_transferred();
-        info->ram->skipped = skipped_mig_pages_transferred();
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
         info->ram->dirty_pages_rate = s->dirty_pages_rate;
 
+
         if (blk_mig_active()) {
             info->has_disk = true;
             info->disk = g_malloc0(sizeof(*info->disk));
@@ -227,7 +227,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->ram->remaining = 0;
         info->ram->total = ram_bytes_total();
         info->ram->duplicate = dup_mig_pages_transferred();
-        info->ram->skipped = skipped_mig_pages_transferred();
         info->ram->normal = norm_mig_pages_transferred();
         info->ram->normal_bytes = norm_mig_bytes_transferred();
         break;
diff --git a/qapi-schema.json b/qapi-schema.json
index ef1f657..db0986c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -499,9 +499,7 @@
 #
 # @total: total amount of bytes involved in the migration process
 #
-# @duplicate: number of duplicate (zero) pages (since 1.2)
-#
-# @skipped: number of skipped zero pages (since 1.5)
+# @duplicate: number of duplicate pages (since 1.2)
 #
 # @normal : number of normal pages (since 1.2)
 #
@@ -514,8 +512,8 @@
 ##
 { 'type': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
-           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
-           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
+           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
+           'dirty-pages-rate' : 'int' } }
 
 ##
 # @XBZRLECacheStats
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ffd130e..fa5986f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2515,7 +2515,6 @@ The main json-object contains the following:
          - "duplicate": number of pages filled entirely with the same
             byte (json-int)
             These are sent over the wire much more efficiently.
-         - "skipped": number of skipped zero pages (json-int)
          - "normal" : number of whole pages transferred.  I.e. they
             were not sent as duplicate or xbzrle pages (json-int)
          - "normal-bytes" : number of bytes transferred in whole
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages
  2013-06-06  8:36 [Qemu-devel] [PATCH 0/2] fix migration of zero pages Peter Lieven
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage" Peter Lieven
@ 2013-06-06  8:36 ` Peter Lieven
  2013-06-19 10:10   ` Paolo Bonzini
  1 sibling, 1 reply; 9+ messages in thread
From: Peter Lieven @ 2013-06-06  8:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: aik, pbonzini, Peter Lieven, quintela

on incoming migration do not memset pages to zero if they already read as zero.
this will allocate a new zero page and consume memory unnecessarily. even
if we madvise a MADV_DONTNEED later this will only deallocate the memory
asynchronously.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 arch_init.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 283081d..642f241 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -821,14 +821,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
             }
 
             ch = qemu_get_byte(f);
-            memset(host, ch, TARGET_PAGE_SIZE);
+            if (ch != 0 || !is_zero_page(host)) {
+                memset(host, ch, TARGET_PAGE_SIZE);
 #ifndef _WIN32
-            if (ch == 0 &&
-                (!kvm_enabled() || kvm_has_sync_mmu()) &&
-                getpagesize() <= TARGET_PAGE_SIZE) {
-                qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
-            }
+                if (ch == 0 &&
+                    (!kvm_enabled() || kvm_has_sync_mmu()) &&
+                    getpagesize() <= TARGET_PAGE_SIZE) {
+                    qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
+                }
 #endif
+            }
         } else if (flags & RAM_SAVE_FLAG_PAGE) {
             void *host;
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage" Peter Lieven
@ 2013-06-06 12:16   ` Eric Blake
  2013-06-06 14:26     ` Peter Lieven
  2013-06-19 10:10   ` Paolo Bonzini
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Blake @ 2013-06-06 12:16 UTC (permalink / raw)
  To: Peter Lieven; +Cc: aik, pbonzini, qemu-devel, quintela

----- Original Message -----
> Not sending zero pages breaks migration if a page is zero
> at the source but not at the destination. This can e.g. happen
> if different BIOS versions are used at source and destination.
> It has also been reported that migration on pseries is completely
> broken with this patch.
> 
> This reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 

> +++ b/qapi-schema.json
> @@ -499,9 +499,7 @@
>  #
>  # @total: total amount of bytes involved in the migration process
>  #
> -# @duplicate: number of duplicate (zero) pages (since 1.2)
> -#
> -# @skipped: number of skipped zero pages (since 1.5)
> +# @duplicate: number of duplicate pages (since 1.2)
>  #
>  # @normal : number of normal pages (since 1.2)
>  #
> @@ -514,8 +512,8 @@
>  ##
>  { 'type': 'MigrationStats',
>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> -           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
> -           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
> +           'dirty-pages-rate' : 'int' } }

This hunk is questionable.  Removing something that we have previously sent
over the wire may break clients that are expecting this field to exist.
Rather than reverting the entire patch, you should consider keeping this
field present in QMP, even if you now always populate it with 0.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"
  2013-06-06 12:16   ` Eric Blake
@ 2013-06-06 14:26     ` Peter Lieven
  2013-06-15 15:28       ` Eric Blake
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Lieven @ 2013-06-06 14:26 UTC (permalink / raw)
  To: Eric Blake; +Cc: aik, pbonzini, Peter Lieven, qemu-devel, quintela

On 06.06.2013 14:16, Eric Blake wrote:
> ----- Original Message -----
>> Not sending zero pages breaks migration if a page is zero
>> at the source but not at the destination. This can e.g. happen
>> if different BIOS versions are used at source and destination.
>> It has also been reported that migration on pseries is completely
>> broken with this patch.
>>
>> This reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
>>
>> +++ b/qapi-schema.json
>> @@ -499,9 +499,7 @@
>>   #
>>   # @total: total amount of bytes involved in the migration process
>>   #
>> -# @duplicate: number of duplicate (zero) pages (since 1.2)
>> -#
>> -# @skipped: number of skipped zero pages (since 1.5)
>> +# @duplicate: number of duplicate pages (since 1.2)
>>   #
>>   # @normal : number of normal pages (since 1.2)
>>   #
>> @@ -514,8 +512,8 @@
>>   ##
>>   { 'type': 'MigrationStats',
>>     'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>> -           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
>> -           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
>> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
>> +           'dirty-pages-rate' : 'int' } }
> This hunk is questionable.  Removing something that we have previously sent
> over the wire may break clients that are expecting this field to exist.
> Rather than reverting the entire patch, you should consider keeping this
> field present in QMP, even if you now always populate it with 0.
>
You are right. I might still account zero pages in the bulk phase to give this
field a meaning. These pages are very likely not written on the destination except
for the cornercases. What do you think?

Peter

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"
  2013-06-06 14:26     ` Peter Lieven
@ 2013-06-15 15:28       ` Eric Blake
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2013-06-15 15:28 UTC (permalink / raw)
  To: Peter Lieven; +Cc: aik, pbonzini, Peter Lieven, qemu-devel, quintela

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

On 06/06/2013 03:26 PM, Peter Lieven wrote:

>>> @@ -514,8 +512,8 @@
>>>   ##
>>>   { 'type': 'MigrationStats',
>>>     'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>>> -           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
>>> -           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
>>> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
>>> +           'dirty-pages-rate' : 'int' } }
>> This hunk is questionable.  Removing something that we have previously
>> sent
>> over the wire may break clients that are expecting this field to exist.
>> Rather than reverting the entire patch, you should consider keeping this
>> field present in QMP, even if you now always populate it with 0.
>>
> You are right. I might still account zero pages in the bulk phase to
> give this
> field a meaning. These pages are very likely not written on the
> destination except
> for the cornercases. What do you think?

I don't care if you document a slightly different meaning for the
statistic; as long as it is documented.  Where I do care is removing
something that previous versions provided; I think your later versions
of this series adequately took care of this by reverting only a subset
of the original patch.  I'm okay with the current qemu.git behavior that
always displays returns "skipped":0, or if you post a later patch that
again makes it print a non-zero number that you find useful.

-- 
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: 621 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage" Peter Lieven
  2013-06-06 12:16   ` Eric Blake
@ 2013-06-19 10:10   ` Paolo Bonzini
  1 sibling, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2013-06-19 10:10 UTC (permalink / raw)
  To: Peter Lieven; +Cc: aik, qemu-devel, quintela

Il 06/06/2013 10:36, Peter Lieven ha scritto:
> Not sending zero pages breaks migration if a page is zero
> at the source but not at the destination. This can e.g. happen
> if different BIOS versions are used at source and destination.
> It has also been reported that migration on pseries is completely
> broken with this patch.
> 
> This reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 
> Conflicts:
> 
> 	arch_init.c
> 	qmp-commands.hx
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> ---
>  arch_init.c                   |   24 ++++--------------------
>  hmp.c                         |    2 --
>  include/migration/migration.h |    2 --
>  migration.c                   |    3 +--
>  qapi-schema.json              |    8 +++-----
>  qmp-commands.hx               |    1 -
>  6 files changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d32ecf..283081d 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -183,7 +183,6 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>  /* accounting for migration statistics */
>  typedef struct AccountingInfo {
>      uint64_t dup_pages;
> -    uint64_t skipped_pages;
>      uint64_t norm_pages;
>      uint64_t iterations;
>      uint64_t xbzrle_bytes;
> @@ -209,16 +208,6 @@ uint64_t dup_mig_pages_transferred(void)
>      return acct_info.dup_pages;
>  }
>  
> -uint64_t skipped_mig_bytes_transferred(void)
> -{
> -    return acct_info.skipped_pages * TARGET_PAGE_SIZE;
> -}
> -
> -uint64_t skipped_mig_pages_transferred(void)
> -{
> -    return acct_info.skipped_pages;
> -}
> -
>  uint64_t norm_mig_bytes_transferred(void)
>  {
>      return acct_info.norm_pages * TARGET_PAGE_SIZE;
> @@ -457,15 +446,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>              bytes_sent = -1;
>              if (is_zero_page(p)) {
>                  acct_info.dup_pages++;
> -                if (!ram_bulk_stage) {
> -                    bytes_sent = save_block_hdr(f, block, offset, cont,
> -                                                RAM_SAVE_FLAG_COMPRESS);
> -                    qemu_put_byte(f, 0);
> -                    bytes_sent++;
> -                } else {
> -                    acct_info.skipped_pages++;
> -                    bytes_sent = 0;
> -                }
> +                bytes_sent = save_block_hdr(f, block, offset, cont,
> +                                            RAM_SAVE_FLAG_COMPRESS);
> +                qemu_put_byte(f, 0);
> +                bytes_sent++;
>              } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
>                  current_addr = block->offset + offset;
>                  bytes_sent = save_xbzrle_page(f, p, current_addr, block,
> diff --git a/hmp.c b/hmp.c
> index 4fb76ec..55c4423 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -173,8 +173,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>                         info->ram->total >> 10);
>          monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
>                         info->ram->duplicate);
> -        monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
> -                       info->ram->skipped);
>          monitor_printf(mon, "normal: %" PRIu64 " pages\n",
>                         info->ram->normal);
>          monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e2acec6..bb617fd 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -96,8 +96,6 @@ extern SaveVMHandlers savevm_ram_handlers;
>  
>  uint64_t dup_mig_bytes_transferred(void);
>  uint64_t dup_mig_pages_transferred(void);
> -uint64_t skipped_mig_bytes_transferred(void);
> -uint64_t skipped_mig_pages_transferred(void);
>  uint64_t norm_mig_bytes_transferred(void);
>  uint64_t norm_mig_pages_transferred(void);
>  uint64_t xbzrle_mig_bytes_transferred(void);
> diff --git a/migration.c b/migration.c
> index 058f9e6..c305d3e 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -197,11 +197,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->ram->remaining = ram_bytes_remaining();
>          info->ram->total = ram_bytes_total();
>          info->ram->duplicate = dup_mig_pages_transferred();
> -        info->ram->skipped = skipped_mig_pages_transferred();
>          info->ram->normal = norm_mig_pages_transferred();
>          info->ram->normal_bytes = norm_mig_bytes_transferred();
>          info->ram->dirty_pages_rate = s->dirty_pages_rate;
>  
> +
>          if (blk_mig_active()) {
>              info->has_disk = true;
>              info->disk = g_malloc0(sizeof(*info->disk));
> @@ -227,7 +227,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->ram->remaining = 0;
>          info->ram->total = ram_bytes_total();
>          info->ram->duplicate = dup_mig_pages_transferred();
> -        info->ram->skipped = skipped_mig_pages_transferred();
>          info->ram->normal = norm_mig_pages_transferred();
>          info->ram->normal_bytes = norm_mig_bytes_transferred();
>          break;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ef1f657..db0986c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -499,9 +499,7 @@
>  #
>  # @total: total amount of bytes involved in the migration process
>  #
> -# @duplicate: number of duplicate (zero) pages (since 1.2)
> -#
> -# @skipped: number of skipped zero pages (since 1.5)
> +# @duplicate: number of duplicate pages (since 1.2)
>  #
>  # @normal : number of normal pages (since 1.2)
>  #
> @@ -514,8 +512,8 @@
>  ##
>  { 'type': 'MigrationStats',
>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> -           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
> -           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
> +           'dirty-pages-rate' : 'int' } }
>  
>  ##
>  # @XBZRLECacheStats
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ffd130e..fa5986f 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2515,7 +2515,6 @@ The main json-object contains the following:
>           - "duplicate": number of pages filled entirely with the same
>              byte (json-int)
>              These are sent over the wire much more efficiently.
> -         - "skipped": number of skipped zero pages (json-int)
>           - "normal" : number of whole pages transferred.  I.e. they
>              were not sent as duplicate or xbzrle pages (json-int)
>           - "normal-bytes" : number of bytes transferred in whole
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages
  2013-06-06  8:36 ` [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages Peter Lieven
@ 2013-06-19 10:10   ` Paolo Bonzini
  2013-06-24 14:59     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2013-06-19 10:10 UTC (permalink / raw)
  To: Peter Lieven; +Cc: aik, qemu-devel, quintela

Il 06/06/2013 10:36, Peter Lieven ha scritto:
> on incoming migration do not memset pages to zero if they already read as zero.
> this will allocate a new zero page and consume memory unnecessarily. even
> if we madvise a MADV_DONTNEED later this will only deallocate the memory
> asynchronously.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  arch_init.c |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 283081d..642f241 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -821,14 +821,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>              }
>  
>              ch = qemu_get_byte(f);
> -            memset(host, ch, TARGET_PAGE_SIZE);
> +            if (ch != 0 || !is_zero_page(host)) {
> +                memset(host, ch, TARGET_PAGE_SIZE);
>  #ifndef _WIN32
> -            if (ch == 0 &&
> -                (!kvm_enabled() || kvm_has_sync_mmu()) &&
> -                getpagesize() <= TARGET_PAGE_SIZE) {
> -                qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
> -            }
> +                if (ch == 0 &&
> +                    (!kvm_enabled() || kvm_has_sync_mmu()) &&
> +                    getpagesize() <= TARGET_PAGE_SIZE) {
> +                    qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
> +                }
>  #endif
> +            }
>          } else if (flags & RAM_SAVE_FLAG_PAGE) {
>              void *host;
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages
  2013-06-19 10:10   ` Paolo Bonzini
@ 2013-06-24 14:59     ` Alexey Kardashevskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2013-06-24 14:59 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Peter Lieven, qemu-devel, quintela

On 06/19/2013 08:10 PM, Paolo Bonzini wrote:
> Il 06/06/2013 10:36, Peter Lieven ha scritto:
>> on incoming migration do not memset pages to zero if they already read as zero.
>> this will allocate a new zero page and consume memory unnecessarily. even
>> if we madvise a MADV_DONTNEED later this will only deallocate the memory
>> asynchronously.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>  arch_init.c |   14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 283081d..642f241 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -821,14 +821,16 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>>              }
>>  
>>              ch = qemu_get_byte(f);
>> -            memset(host, ch, TARGET_PAGE_SIZE);
>> +            if (ch != 0 || !is_zero_page(host)) {
>> +                memset(host, ch, TARGET_PAGE_SIZE);
>>  #ifndef _WIN32
>> -            if (ch == 0 &&
>> -                (!kvm_enabled() || kvm_has_sync_mmu()) &&
>> -                getpagesize() <= TARGET_PAGE_SIZE) {
>> -                qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
>> -            }
>> +                if (ch == 0 &&
>> +                    (!kvm_enabled() || kvm_has_sync_mmu()) &&
>> +                    getpagesize() <= TARGET_PAGE_SIZE) {
>> +                    qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
>> +                }
>>  #endif
>> +            }
>>          } else if (flags & RAM_SAVE_FLAG_PAGE) {
>>              void *host;
>>  
>>
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Is it expected to reach upstream any time soon? Thanks.



-- 
Alexey

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-06-24 15:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-06  8:36 [Qemu-devel] [PATCH 0/2] fix migration of zero pages Peter Lieven
2013-06-06  8:36 ` [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage" Peter Lieven
2013-06-06 12:16   ` Eric Blake
2013-06-06 14:26     ` Peter Lieven
2013-06-15 15:28       ` Eric Blake
2013-06-19 10:10   ` Paolo Bonzini
2013-06-06  8:36 ` [Qemu-devel] [PATCH 2/2] migration: do not overwrite zero pages Peter Lieven
2013-06-19 10:10   ` Paolo Bonzini
2013-06-24 14:59     ` Alexey Kardashevskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.