On Monday, February 26, 2018 1:07 PM, Wei Wang wrote: > On 02/09/2018 07:50 PM, Dr. David Alan Gilbert wrote: > > * Wei Wang (wei.w.w...@intel.com) wrote: > >> Use the free page reporting feature from the balloon device to clear > >> the bits corresponding to guest free pages from the dirty bitmap, so > >> that the free memory are not sent. > >> > >> Signed-off-by: Wei Wang <wei.w.w...@intel.com> > >> CC: Michael S. Tsirkin <m...@redhat.com> > >> CC: Juan Quintela <quint...@redhat.com> > >> --- > >> migration/ram.c | 24 ++++++++++++++++++++---- > >> 1 file changed, 20 insertions(+), 4 deletions(-) > >> > >> diff --git a/migration/ram.c b/migration/ram.c index d6f462c..4fe16d2 > >> 100644 > >> --- a/migration/ram.c > >> +++ b/migration/ram.c > >> @@ -49,6 +49,7 @@ > >> #include "qemu/rcu_queue.h" > >> #include "migration/colo.h" > >> #include "migration/block.h" > >> +#include "sysemu/balloon.h" > >> > >> /***********************************************************/ > >> /* ram save/restore */ > >> @@ -206,6 +207,10 @@ struct RAMState { > >> uint32_t last_version; > >> /* We are in the first round */ > >> bool ram_bulk_stage; > >> + /* The feature, skipping the transfer of free pages, is supported */ > >> + bool free_page_support; > >> + /* Skip the transfer of free pages in the bulk stage */ > >> + bool free_page_done; > >> /* How many times we have dirty too many pages */ > >> int dirty_rate_high_cnt; > >> /* these variables are used for bitmap sync */ @@ -773,7 +778,7 > >> @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock > *rb, > >> unsigned long *bitmap = rb->bmap; > >> unsigned long next; > >> > >> - if (rs->ram_bulk_stage && start > 0) { > >> + if (rs->ram_bulk_stage && start > 0 && !rs->free_page_support) { > >> next = start + 1; > >> } else { > >> next = find_next_bit(bitmap, size, start); @@ -1653,6 > >> +1658,8 @@ static void ram_state_reset(RAMState *rs) > >> rs->last_page = 0; > >> rs->last_version = ram_list.version; > >> rs->ram_bulk_stage = true; > >> + rs->free_page_support = balloon_free_page_support(); > >> + rs->free_page_done = false; > >> } > >> > >> #define MAX_WAIT 50 /* ms, half buffered_file limit */ @@ -2135,7 > >> +2142,7 @@ static int ram_state_init(RAMState **rsp) > >> return 0; > >> } > >> > >> -static void ram_list_init_bitmaps(void) > >> +static void ram_list_init_bitmaps(RAMState *rs) > >> { > >> RAMBlock *block; > >> unsigned long pages; > >> @@ -2145,7 +2152,11 @@ static void ram_list_init_bitmaps(void) > >> QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > >> pages = block->max_length >> TARGET_PAGE_BITS; > >> block->bmap = bitmap_new(pages); > >> - bitmap_set(block->bmap, 0, pages); > >> + if (rs->free_page_support) { > >> + bitmap_set(block->bmap, 1, pages); > > I don't understand how it makes sense to do that here; ignoring > > anything ese it means that migration_dirty_pages is wrong which could > > end up with migration finishing before all real pages are sent. > > > > The bulk stage treats all the pages as dirty pages, so we set all the bits to > "1", > this is needed by this optimization feature, because the free pages reported > from the guest can then be directly cleared from the bitmap (we don't need > any more bitmaps to record free pages). >
Sorry, there was a misunderstanding of the bitmap_set API (thought it was used to set all the bits to 1 or 0). So the above change isn't needed actually. Btw, this doesn't affect the results I reported. Best, Wei