On Wed, Jun 16, 2021 at 09:12:28AM +0800, huang...@chinatelecom.cn wrote: > diff --git a/include/exec/memory.h b/include/exec/memory.h > index b114f54..e31eef2 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -55,7 +55,11 @@ static inline void fuzz_dma_read_cb(size_t addr, > } > #endif > > -extern bool global_dirty_log; > +/* what is the purpose of current dirty log, migration or dirty rate ? */
Nitpick: I'll make it: /* Possible bits for global_dirty_log */ /* Dirty tracking enabled because migration is running */ #define GLOBAL_DIRTY_MIGRATION (1U << 0) /* Dirty tracking enabled because measuring dirty rate */ #define GLOBAL_DIRTY_DIRTY_RATE (1U << 1) > +#define GLOBAL_DIRTY_MIGRATION (1U << 0) > +#define GLOBAL_DIRTY_DIRTY_RATE (1U << 1) > + > +extern unsigned int global_dirty_log; > > typedef struct MemoryRegionOps MemoryRegionOps; > [...] > @@ -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; > +unsigned int global_dirty_log; I'm wondering whether it's a good chance to rename it to global_dirty_tracking, because "logging" has a hint on the method while it's not the only one now. > > static QTAILQ_HEAD(, MemoryListener) memory_listeners > = QTAILQ_HEAD_INITIALIZER(memory_listeners); > @@ -2659,14 +2659,19 @@ 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(unsigned int flags) > { > if (vmstate_change) { > qemu_del_vm_change_state_handler(vmstate_change); > vmstate_change = NULL; > } > > - global_dirty_log = true; > +#define GLOBAL_DIRTY_MASK (0x3) > + assert(flags && !(flags & (~GLOBAL_DIRTY_MASK))); > + assert(global_dirty_log ^ flags); Heh, this is probably my fault... I think what I wanted to suggest is actually: assert(!(global_dirty_log & flags)); Then for stop() below... > + global_dirty_log |= flags; > + > + trace_global_dirty_changed(global_dirty_log); > > MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward); > > @@ -2676,9 +2681,12 @@ 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(unsigned int flags) > { > - global_dirty_log = false; > + assert(flags && !(flags & (~GLOBAL_DIRTY_MASK))); ... it should probably be: assert((global_dirty_log & flags) == flags); Sorry about the confusion. -- Peter Xu