Message ID | 20190604061727.6857-1-richardw.yang@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3] migratioin/ram: leave RAMBlock->bmap blank on allocating | expand |
On Tue, Jun 04, 2019 at 02:17:27PM +0800, Wei Yang wrote: > During migration, we would sync bitmap from ram_list.dirty_memory to > RAMBlock.bmap in cpu_physical_memory_sync_dirty_bitmap(). > > Since we set RAMBlock.bmap and ram_list.dirty_memory both to all 1, this > means at the first round this sync is meaningless and is a duplicated > work. > > Leaving RAMBlock->bmap blank on allocating would have a side effect on > migration_dirty_pages, since it is calculated from the result of > cpu_physical_memory_sync_dirty_bitmap(). To keep it right, we need to > set migration_dirty_pages to 0 in ram_state_init(). > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thanks for updating the comments. Acked-by: Peter Xu <peterx@redhat.com>
On Tue, Jun 04, 2019 at 03:06:14PM +0800, Peter Xu wrote: >On Tue, Jun 04, 2019 at 02:17:27PM +0800, Wei Yang wrote: >> During migration, we would sync bitmap from ram_list.dirty_memory to >> RAMBlock.bmap in cpu_physical_memory_sync_dirty_bitmap(). >> >> Since we set RAMBlock.bmap and ram_list.dirty_memory both to all 1, this >> means at the first round this sync is meaningless and is a duplicated >> work. >> >> Leaving RAMBlock->bmap blank on allocating would have a side effect on >> migration_dirty_pages, since it is calculated from the result of >> cpu_physical_memory_sync_dirty_bitmap(). To keep it right, we need to >> set migration_dirty_pages to 0 in ram_state_init(). >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >Thanks for updating the comments. > My pleasure :-) >Acked-by: Peter Xu <peterx@redhat.com> > >-- >Peter Xu
diff --git a/migration/ram.c b/migration/ram.c index 4c60869226..c3ece382ae 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3182,11 +3182,11 @@ static int ram_state_init(RAMState **rsp) QSIMPLEQ_INIT(&(*rsp)->src_page_requests); /* - * Count the total number of pages used by ram blocks not including any - * gaps due to alignment or unplugs. + * This must match with the initial values of dirty bitmap. + * Currently we initialize the dirty bitmap to all zeros so + * here the total dirty page count is zero. */ - (*rsp)->migration_dirty_pages = ram_bytes_total() >> TARGET_PAGE_BITS; - + (*rsp)->migration_dirty_pages = 0; ram_state_reset(*rsp); return 0; @@ -3201,8 +3201,16 @@ static void ram_list_init_bitmaps(void) if (ram_bytes_total()) { RAMBLOCK_FOREACH_NOT_IGNORED(block) { pages = block->max_length >> TARGET_PAGE_BITS; + /* + * The initial dirty bitmap for migration must be set with all + * ones to make sure we'll migrate every guest RAM page to + * destination. + * Here we didn't set RAMBlock.bmap simply because it is already + * set in ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] in + * ram_block_add, and that's where we'll sync the dirty bitmaps. + * Here setting RAMBlock.bmap would be fine too but not necessary. + */ block->bmap = bitmap_new(pages); - bitmap_set(block->bmap, 0, pages); if (migrate_postcopy_ram()) { block->unsentmap = bitmap_new(pages); bitmap_set(block->unsentmap, 0, pages);