Re: [Qemu-devel] [PATCH 1/2] memory: Replace has_coalesced_range with add/del flags

[Paolo Bonzini]

On 17/08/19 11:32, Peter Xu wrote:
> The previous has_coalesced_range counter has a problem in that it only
> works for additions of coalesced mmio ranges but not deletions.  The
> reason is that has_coalesced_range information can be lost when the
> FlatView updates the topology again when the updated region is not
> covering the coalesced regions. When that happens, due to
> flatrange_equal() is not checking against has_coalesced_range, the new
> FlatRange will be seen as the same one as the old and the new
> instance (whose has_coalesced_range will be zero) will replace the old
> instance (whose has_coalesced_range _could_ be non-zero).
> 
> To fix it, we don't cache has_coalesced_range at all in the FlatRange.
> Instead we introduce two flags to make sure the coalesced_io_{add|del}
> will only be called once for every FlatRange instance.  This will even
> work if another FlatRange replaces current one.

It's still a bit ugly that coalesced_mmio_add_done ends up not being set
on the new (but equal) FlatRange.

Would something like this work too?

diff --git a/memory.c b/memory.c
index edd0c13..fc91f06 100644
--- a/memory.c
+++ b/memory.c
@@ -939,6 +939,7 @@ static void address_space_update_topology_pass(AddressSpace 
*as,
             /* In both and unchanged (except logging may have changed) */
 
             if (adding) {
+                frnew->has_coalesced_range = frold->has_coalesced_range;
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
                 if (frnew->dirty_log_mask & ~frold->dirty_log_mask) {
                     MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, 
log_start,

Thanks,

Paolo

> Without this patch, MemoryListener.coalesced_io_del is hardly being
> called due to has_coalesced_range will be mostly zero in
> flat_range_coalesced_io_del() when topologies frequently change for
> the "memory" address space.