Hyman Huang <yong.hu...@smartx.com> writes: > shadow_bmap, iter_bmap and iter_dirty_pages are introduced > to satisfy the need for background sync. > > Meanwhile, introduce enumeration of sync method. > > Signed-off-by: Hyman Huang <yong.hu...@smartx.com> > --- > include/exec/ramblock.h | 45 +++++++++++++++++++++++++++++++++++++++++ > migration/ram.c | 6 ++++++ > 2 files changed, 51 insertions(+) > > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h > index 0babd105c0..0e327bc0ae 100644 > --- a/include/exec/ramblock.h > +++ b/include/exec/ramblock.h > @@ -24,6 +24,30 @@ > #include "qemu/rcu.h" > #include "exec/ramlist.h" > > +/* Possible bits for cpu_physical_memory_sync_dirty_bitmap */ > + > +/* > + * The old-fashioned sync, which is, in turn, used for CPU > + * throttle and memory transfer.
I'm not sure I follow what "in turn" is supposed to mean in this sentence. Could you clarify? > + */ > +#define RAMBLOCK_SYN_LEGACY_ITER (1U << 0) So ITER is as opposed to background? I'm a bit confused with the terms. > + > +/* > + * The modern sync, which is, in turn, used for CPU throttle > + * and memory transfer. > + */ > +#define RAMBLOCK_SYN_MODERN_ITER (1U << 1) > + > +/* The modern sync, which is used for CPU throttle only */ > +#define RAMBLOCK_SYN_MODERN_BACKGROUND (1U << 2) What's the plan for the "legacy" part? To be removed soon? Do we want to remove it now? Maybe better to not use the modern/legacy terms unless we want to give the impression that the legacy one is discontinued. > + > +#define RAMBLOCK_SYN_MASK (0x7) > + > +typedef enum RAMBlockSynMode { > + RAMBLOCK_SYN_LEGACY, /* Old-fashined mode */ > + RAMBLOCK_SYN_MODERN, /* Background-sync-supported mode */ > +} RAMBlockSynMode; I'm also wondering wheter we need this enum + the flags or one of them would suffice. I'm looking at code like this in the following patches, for instance: + if (sync_mode == RAMBLOCK_SYN_MODERN) { + if (background) { + flag = RAMBLOCK_SYN_MODERN_BACKGROUND; + } else { + flag = RAMBLOCK_SYN_MODERN_ITER; + } + } Couldn't we use LEGACY/BG/ITER? > + > struct RAMBlock { > struct rcu_head rcu; > struct MemoryRegion *mr; > @@ -89,6 +113,27 @@ struct RAMBlock { > * could not have been valid on the source. > */ > ram_addr_t postcopy_length; > + > + /* > + * Used to backup the bmap during background sync to see whether any > dirty > + * pages were sent during that time. > + */ > + unsigned long *shadow_bmap; > + > + /* > + * The bitmap "bmap," which was initially used for both sync and memory > + * transfer, will be replaced by two bitmaps: the previously used "bmap" > + * and the recently added "iter_bmap." Only the memory transfer is > + * conducted with the previously used "bmap"; the recently added > + * "iter_bmap" is utilized for dirty bitmap sync. > + */ > + unsigned long *iter_bmap; > + > + /* Number of new dirty pages during iteration */ > + uint64_t iter_dirty_pages; > + > + /* If background sync has shown up during iteration */ > + bool background_sync_shown_up; > }; > #endif > #endif > diff --git a/migration/ram.c b/migration/ram.c > index 67ca3d5d51..f29faa82d6 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2362,6 +2362,10 @@ static void ram_bitmaps_destroy(void) > block->bmap = NULL; > g_free(block->file_bmap); > block->file_bmap = NULL; > + g_free(block->shadow_bmap); > + block->shadow_bmap = NULL; > + g_free(block->iter_bmap); > + block->iter_bmap = NULL; > } > } > > @@ -2753,6 +2757,8 @@ static void ram_list_init_bitmaps(void) > } > block->clear_bmap_shift = shift; > block->clear_bmap = bitmap_new(clear_bmap_size(pages, shift)); > + block->shadow_bmap = bitmap_new(pages); > + block->iter_bmap = bitmap_new(pages); > } > } > }