[Qemu-devel] [PATCH] migration: drop MADVISE_DONT_NEED for incoming zero pages

[Qemu-devel] [PATCH] migration: drop MADVISE_DONT_NEED for incoming zero pages

[Peter Lieven]

The madvise for zeroed out pages was introduced when every transferred
zero page was memset to zero and thus allocated. Since commit
211ea740 we check for zeroness of a target page before we memset
it to zero. Additionally we memmap target memory so it is essentially
zero initalized (except for e.g. option roms and bios which are loaded
into target memory altough they shouldn't). 

It was reported recently that this madvise causes a performance degradation
in some situations. As the madvise should only be called rarely and if its called
it is likely on a busy page (it was non-zero and changed to zero during migration)
drop it completely.

Signed-off-by: Peter Lieven <pl@kamp.de>
Reported-By: Zhang Haoyu <haoyu.zhang@huawei.com>
---
 arch_init.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 7545d96..e0acbc5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -850,14 +850,6 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
 {
     if (ch != 0 || !is_zero_range(host, size)) {
         memset(host, ch, size);
-#ifndef _WIN32
-        if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu())) {
-            size = size & ~(getpagesize() - 1);
-            if (size > 0) {
-                qemu_madvise(host, size, QEMU_MADV_DONTNEED);
-            }
-        }
-#endif
     }
 }
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] migration: drop MADVISE_DONT_NEED for incoming zero pages

[Paolo Bonzini]

Il 07/10/2013 11:22, Peter Lieven ha scritto:
> The madvise for zeroed out pages was introduced when every transferred
> zero page was memset to zero and thus allocated. Since commit
> 211ea740 we check for zeroness of a target page before we memset
> it to zero. Additionally we memmap target memory so it is essentially
> zero initalized (except for e.g. option roms and bios which are loaded
> into target memory altough they shouldn't). 
> 
> It was reported recently that this madvise causes a performance degradation
> in some situations. As the madvise should only be called rarely and if its called
> it is likely on a busy page (it was non-zero and changed to zero during migration)
> drop it completely.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reported-By: Zhang Haoyu <haoyu.zhang@huawei.com>
> ---
>  arch_init.c |    8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 7545d96..e0acbc5 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -850,14 +850,6 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
>  {
>      if (ch != 0 || !is_zero_range(host, size)) {
>          memset(host, ch, size);
> -#ifndef _WIN32
> -        if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu())) {
> -            size = size & ~(getpagesize() - 1);
> -            if (size > 0) {
> -                qemu_madvise(host, size, QEMU_MADV_DONTNEED);
> -            }
> -        }
> -#endif
>      }
>  }
>  
> 

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

Re: [Qemu-devel] [PATCH] migration: drop MADVISE_DONT_NEED for incoming zero pages

[Andreas Färber]

Am 07.10.2013 11:22, schrieb Peter Lieven:
> The madvise for zeroed out pages was introduced when every transferred
> zero page was memset to zero and thus allocated. Since commit
> 211ea740 we check for zeroness of a target page before we memset
> it to zero. Additionally we memmap target memory so it is essentially
> zero initalized (except for e.g. option roms and bios which are loaded

"initialized"

> into target memory altough they shouldn't). 

"although"

> 
> It was reported recently that this madvise causes a performance degradation
> in some situations. As the madvise should only be called rarely and if its called

"it's"

> it is likely on a busy page (it was non-zero and changed to zero during migration)
> drop it completely.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reported-By: Zhang Haoyu <haoyu.zhang@huawei.com>

Please case "Reported-by" and place above Sob.

Juan, will you be picking this one up or do we need a respin?

Cheers,
Andreas

> ---
>  arch_init.c |    8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 7545d96..e0acbc5 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -850,14 +850,6 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
>  {
>      if (ch != 0 || !is_zero_range(host, size)) {
>          memset(host, ch, size);
> -#ifndef _WIN32
> -        if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu())) {
> -            size = size & ~(getpagesize() - 1);
> -            if (size > 0) {
> -                qemu_madvise(host, size, QEMU_MADV_DONTNEED);
> -            }
> -        }
> -#endif
>      }
>  }
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg