* Peter Xu ([email protected]) wrote: > Some of the memory listener may want to do log synchronization without > being able to specify a range of memory to sync but always globally. > Such a memory listener should provide this new method instead of the > log_sync() method. > > Obviously we can also achieve similar thing when we put the global > sync logic into a log_sync() handler. However that's not efficient > enough because otherwise memory_global_dirty_log_sync() may do the > global sync N times, where N is the number of flat views. > > Make this new method be exclusive to log_sync(). > > Signed-off-by: Peter Xu <[email protected]>
OK, so I guess the idea here is that when you have a ring with dirty pages on it, you just need to clear all outstanding things on the ring whereever they came from. Reviewed-by: Dr. David Alan Gilbert <[email protected]> > --- > include/exec/memory.h | 12 ++++++++++++ > memory.c | 33 +++++++++++++++++++++++---------- > 2 files changed, 35 insertions(+), 10 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index e85b7de99a..c4427094bb 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -533,6 +533,18 @@ struct MemoryListener { > */ > void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section); > > + /** > + * @log_sync_global: > + * > + * This is the global version of @log_sync when the listener does > + * not have a way to synchronize the log with finer granularity. > + * When the listener registers with @log_sync_global defined, then > + * its @log_sync must be NULL. Vice versa. > + * > + * @listener: The #MemoryListener. > + */ > + void (*log_sync_global)(MemoryListener *listener); > + > /** > * @log_clear: > * > diff --git a/memory.c b/memory.c > index aeaa8dcc9e..53828ba00c 100644 > --- a/memory.c > +++ b/memory.c > @@ -2016,6 +2016,10 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr > addr, > > memory_region_get_dirty_log_mask(mr)); > } > > +/* > + * If memory region `mr' is NULL, do global sync. Otherwise, sync > + * dirty bitmap for the specified memory region. > + */ > static void memory_region_sync_dirty_bitmap(MemoryRegion *mr) > { > MemoryListener *listener; > @@ -2029,18 +2033,24 @@ static void > memory_region_sync_dirty_bitmap(MemoryRegion *mr) > * address space once. > */ > QTAILQ_FOREACH(listener, &memory_listeners, link) { > - if (!listener->log_sync) { > - continue; > - } > - as = listener->address_space; > - view = address_space_get_flatview(as); > - FOR_EACH_FLAT_RANGE(fr, view) { > - if (fr->dirty_log_mask && (!mr || fr->mr == mr)) { > - MemoryRegionSection mrs = section_from_flat_range(fr, view); > - listener->log_sync(listener, &mrs); > + if (listener->log_sync) { > + as = listener->address_space; > + view = address_space_get_flatview(as); > + FOR_EACH_FLAT_RANGE(fr, view) { > + if (fr->dirty_log_mask && (!mr || fr->mr == mr)) { > + MemoryRegionSection mrs = section_from_flat_range(fr, > view); > + listener->log_sync(listener, &mrs); > + } > } > + flatview_unref(view); > + } else if (listener->log_sync_global) { > + /* > + * No matter whether MR is specified, what we can do here > + * is to do a global sync, because we are not capable to > + * sync in a finer granularity. > + */ > + listener->log_sync_global(listener); > } > - flatview_unref(view); > } > } > > @@ -2727,6 +2737,9 @@ void memory_listener_register(MemoryListener *listener, > AddressSpace *as) > { > MemoryListener *other = NULL; > > + /* Only one of them can be defined for a listener */ > + assert(!(listener->log_sync && listener->log_sync_global)); > + > listener->address_space = as; > if (QTAILQ_EMPTY(&memory_listeners) > || listener->priority >= QTAILQ_LAST(&memory_listeners)->priority) { > -- > 2.24.1 > > -- Dr. David Alan Gilbert / [email protected] / Manchester, UK
