On Mon, Jun 07, 2021 at 09:13:12AM +0800, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) <huang...@chinatelecom.cn> > > dirty rate measurement may start or stop dirty logging during > calculation. this conflict with migration because stop dirty > log make migration leave dirty pages out then that'll be a problem. > > make global_dirty_log a bitmask can let both migration and dirty > rate measurement work fine. introduce GLOBAL_DIRTY_MIGRATION and > GLOBAL_DIRTY_DIRTY_RATE to distinguish what current dirty log aims > for, migration or dirty rate. > > all references to global_dirty_log should be untouched because any bit > set there should justify that global dirty logging is enabled. > > Signed-off-by: Hyman Huang(黄勇) <huang...@chinatelecom.cn> > --- > include/exec/memory.h | 13 ++++++++++--- > migration/ram.c | 8 ++++---- > softmmu/memory.c | 36 +++++++++++++++++++++++++++--------- > 3 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c158fd7084..94c7088299 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -55,7 +55,10 @@ static inline void fuzz_dma_read_cb(size_t addr, > } > #endif > > -extern bool global_dirty_log; > +#define GLOBAL_DIRTY_MIGRATION (1U<<0) > +#define GLOBAL_DIRTY_DIRTY_RATE (1U<<1)
Add some comment for each dirty log reason would be nice. > diff --git a/softmmu/memory.c b/softmmu/memory.c > index c19b0be6b1..b93baba82d 100644 > --- a/softmmu/memory.c > +++ b/softmmu/memory.c > @@ -39,7 +39,7 @@ > static unsigned memory_region_transaction_depth; > static bool memory_region_update_pending; > static bool ioeventfd_update_pending; > -bool global_dirty_log; > +int global_dirty_log; Maybe unsigned int would make more sense for a bitmask? > > static QTAILQ_HEAD(, MemoryListener) memory_listeners > = QTAILQ_HEAD_INITIALIZER(memory_listeners); > @@ -2659,14 +2659,20 @@ void memory_global_after_dirty_log_sync(void) > > static VMChangeStateEntry *vmstate_change; > > -void memory_global_dirty_log_start(void) > +void memory_global_dirty_log_start(int flags) > { > if (vmstate_change) { > qemu_del_vm_change_state_handler(vmstate_change); > vmstate_change = NULL; > } > > - global_dirty_log = true; > + if (flags & GLOBAL_DIRTY_MIGRATION) { > + global_dirty_log |= GLOBAL_DIRTY_MIGRATION; > + } > + > + if (flags & GLOBAL_DIRTY_DIRTY_RATE) { > + global_dirty_log |= GLOBAL_DIRTY_DIRTY_RATE; > + } Instead of separate "if"s, perhaps something like this? #define GLOBAL_DIRTY_MASK (0x3) assert(!(flags & (~GLOBAL_DIRTY_MASK))); assert(!(global_dirty_log ^ flags)); global_dirty_log |= flags; > > MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward); > > @@ -2676,9 +2682,15 @@ void memory_global_dirty_log_start(void) > memory_region_transaction_commit(); > } > > -static void memory_global_dirty_log_do_stop(void) > +static void memory_global_dirty_log_do_stop(int flags) > { > - global_dirty_log = false; > + if (flags & GLOBAL_DIRTY_MIGRATION) { > + global_dirty_log &= ~GLOBAL_DIRTY_MIGRATION; > + } > + > + if (flags & GLOBAL_DIRTY_DIRTY_RATE) { > + global_dirty_log &= ~GLOBAL_DIRTY_DIRTY_RATE; > + } Same here? Thanks, -- Peter Xu