Message ID | 63cb8b20f633a6abe56d384e3c3a5151ee6f422d.1379498467.git.yamahata@private.email.ne.jp |
---|---|
State | New |
Headers | show |
Isaku Yamahata <yamahata@private.email.ne.jp> wrote: > ram_handle_compressed() should be aware size > TARGET_PAGE_SIZE > migration-rdma can call it with larger size. > > Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp> > --- > arch_init.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index e47e139..64c81b0 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -844,15 +844,22 @@ static inline void *host_from_stream_offset(QEMUFile *f, > */ > void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) > { > - if (ch != 0 || !is_zero_page(host)) { > - memset(host, ch, size); I assume that size is always a multiple of pagesize, right? > + uint64_t pagesize = getpagesize(); > + while (size > 0) { > + uint64_t length = MIN(pagesize, size); > + > + if (ch !=0 || buffer_find_nonzero_offset(host, length) != length) { This is a different optimization, we were't doing this before. > + memset(host, ch, length); > #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()) && pagesize <= length) { > + qemu_madvise(host, size, QEMU_MADV_DONTNEED); Here don't we need s/size/length/? > + } > #endif > + } > + > + size -= length; > + host += length; > } > }
On 09/18/2013 06:07 AM, Isaku Yamahata wrote: > ram_handle_compressed() should be aware size > TARGET_PAGE_SIZE > migration-rdma can call it with larger size. > > Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp> > --- > arch_init.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index e47e139..64c81b0 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -844,15 +844,22 @@ static inline void *host_from_stream_offset(QEMUFile *f, > */ > void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) > { > - if (ch != 0 || !is_zero_page(host)) { > - memset(host, ch, size); > + uint64_t pagesize = getpagesize(); > + while (size > 0) { > + uint64_t length = MIN(pagesize, size); > + > + if (ch !=0 || buffer_find_nonzero_offset(host, length) != length) { > + memset(host, ch, length); > #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()) && pagesize <= length) { > + qemu_madvise(host, size, QEMU_MADV_DONTNEED); > + } > #endif > + } > + > + size -= length; > + host += length; > } > } > What is the reason for breaking the madvise() into smaller pieces? - Michael
On Wed, Sep 18, 2013 at 02:08:40PM +0200, Juan Quintela wrote: > Isaku Yamahata <yamahata@private.email.ne.jp> wrote: > > ram_handle_compressed() should be aware size > TARGET_PAGE_SIZE > > migration-rdma can call it with larger size. > > > > Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp> > > --- > > arch_init.c | 21 ++++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/arch_init.c b/arch_init.c > > index e47e139..64c81b0 100644 > > --- a/arch_init.c > > +++ b/arch_init.c > > @@ -844,15 +844,22 @@ static inline void *host_from_stream_offset(QEMUFile *f, > > */ > > void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) > > { > > - if (ch != 0 || !is_zero_page(host)) { > > - memset(host, ch, size); > > I assume that size is always a multiple of pagesize, right? Right. The issues is that is_zero_page() checks only first page. The following page needs to be checked. > > > + uint64_t pagesize = getpagesize(); > > + while (size > 0) { > > + uint64_t length = MIN(pagesize, size); > > + > > + if (ch !=0 || buffer_find_nonzero_offset(host, length) != length) { > > This is a different optimization, we were't doing this before. Okay, I'll drop loop and use size. thanks, > > + memset(host, ch, length); > > #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()) && pagesize <= length) { > > + qemu_madvise(host, size, QEMU_MADV_DONTNEED); > > Here don't we need s/size/length/? > > > + } > > #endif > > + } > > + > > + size -= length; > > + host += length; > > } > > } >
diff --git a/arch_init.c b/arch_init.c index e47e139..64c81b0 100644 --- a/arch_init.c +++ b/arch_init.c @@ -844,15 +844,22 @@ static inline void *host_from_stream_offset(QEMUFile *f, */ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size) { - if (ch != 0 || !is_zero_page(host)) { - memset(host, ch, size); + uint64_t pagesize = getpagesize(); + while (size > 0) { + uint64_t length = MIN(pagesize, size); + + if (ch !=0 || buffer_find_nonzero_offset(host, length) != length) { + memset(host, ch, length); #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()) && pagesize <= length) { + qemu_madvise(host, size, QEMU_MADV_DONTNEED); + } #endif + } + + size -= length; + host += length; } }
ram_handle_compressed() should be aware size > TARGET_PAGE_SIZE migration-rdma can call it with larger size. Signed-off-by: Isaku Yamahata <yamahata@private.email.ne.jp> --- arch_init.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)