| Message ID | 1363362619-3190-8-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 03/15/2013 09:50 AM, Peter Lieven wrote: > during bulk stage of ram migration if a page is a > zero page do not send it at all. > the memory at the destination reads as zero anyway. > > even if there is an madvise with QEMU_MADV_DONTNEED > at the target upon receival of a zero page I have observed s/receival/receipt/ > that the target starts swapping if the memory is overcommitted. > it seems that the pages are dropped asynchronously. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > arch_init.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > +++ b/arch_init.c > @@ -432,9 +432,11 @@ static int ram_save_block(QEMUFile *f, bool last_stage) > bytes_sent = -1; > if (buffer_is_zero(p, TARGET_PAGE_SIZE)) { > acct_info.dup_pages++; > - bytes_sent = save_block_hdr(f, block, offset, cont, > - RAM_SAVE_FLAG_COMPRESS); > - qemu_put_byte(f, *p); > + if (!ram_bulk_stage) { > + bytes_sent = save_block_hdr(f, block, offset, cont, > + RAM_SAVE_FLAG_COMPRESS); > + qemu_put_byte(f, *p); Hmm, in patch 5/9, why not 'qemu_put_byte(f, 0)' instead of spending time dereferencing *p, since you already know the byte is 0? > + } > bytes_sent += 1; Your accounting is now off. This increments bytes_sent even when nothing was sent in the bulk phase. When touching this line, consider using ++ instead of += 1. > } else if (migrate_use_xbzrle()) { > current_addr = block->offset + offset; >
Am 19.03.2013 um 18:36 schrieb Eric Blake <eblake@redhat.com>: > On 03/15/2013 09:50 AM, Peter Lieven wrote: >> during bulk stage of ram migration if a page is a >> zero page do not send it at all. >> the memory at the destination reads as zero anyway. >> >> even if there is an madvise with QEMU_MADV_DONTNEED >> at the target upon receival of a zero page I have observed > > s/receival/receipt/ > >> that the target starts swapping if the memory is overcommitted. >> it seems that the pages are dropped asynchronously. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> arch_init.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> +++ b/arch_init.c >> @@ -432,9 +432,11 @@ static int ram_save_block(QEMUFile *f, bool last_stage) >> bytes_sent = -1; >> if (buffer_is_zero(p, TARGET_PAGE_SIZE)) { >> acct_info.dup_pages++; >> - bytes_sent = save_block_hdr(f, block, offset, cont, >> - RAM_SAVE_FLAG_COMPRESS); >> - qemu_put_byte(f, *p); >> + if (!ram_bulk_stage) { >> + bytes_sent = save_block_hdr(f, block, offset, cont, >> + RAM_SAVE_FLAG_COMPRESS); >> + qemu_put_byte(f, *p); > > Hmm, in patch 5/9, why not 'qemu_put_byte(f, 0)' instead of spending > time dereferencing *p, since you already know the byte is 0? Good idea. Actually to avoid unnecessary checks in buffer_is_zero I think I would like to change "if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {..} " to bool inline is_zero_page(uint8_t *p) {return buffer_find_nonzero_offset(p, TARGET_PAGE_SIZE) == TARGET_PAGE_SIZE);} ... if (is_zero_page(p)) { … } also for better readability. For pages we know that they satisfy can_use_buffer_find_nonzero_offset(). Peter > >> + } >> bytes_sent += 1; > > Your accounting is now off. This increments bytes_sent even when > nothing was sent in the bulk phase. When touching this line, consider > using ++ instead of += 1. > >> } else if (migrate_use_xbzrle()) { >> current_addr = block->offset + offset; >> > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
diff --git a/arch_init.c b/arch_init.c index e5531e8..a3dc20d 100644 --- a/arch_init.c +++ b/arch_init.c @@ -432,9 +432,11 @@ static int ram_save_block(QEMUFile *f, bool last_stage) bytes_sent = -1; if (buffer_is_zero(p, TARGET_PAGE_SIZE)) { acct_info.dup_pages++; - bytes_sent = save_block_hdr(f, block, offset, cont, - RAM_SAVE_FLAG_COMPRESS); - qemu_put_byte(f, *p); + if (!ram_bulk_stage) { + bytes_sent = save_block_hdr(f, block, offset, cont, + RAM_SAVE_FLAG_COMPRESS); + qemu_put_byte(f, *p); + } bytes_sent += 1; } else if (migrate_use_xbzrle()) { current_addr = block->offset + offset;
during bulk stage of ram migration if a page is a zero page do not send it at all. the memory at the destination reads as zero anyway. even if there is an madvise with QEMU_MADV_DONTNEED at the target upon receival of a zero page I have observed that the target starts swapping if the memory is overcommitted. it seems that the pages are dropped asynchronously. Signed-off-by: Peter Lieven <pl@kamp.de> --- arch_init.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)