Re: [Qemu-devel] [PATCH] memory: Fix double unref of flatview

[Paolo Bonzini]

On 12/02/2015 17:21, Matthew Rosato wrote:
> Since 374f2981d1 "memory: protect current_map by RCU",
> address_space_update_topology unrefs the old_flatview twice,
> once by call_rcu and once by direct call.  This patch removes
> the direct call in favor of the call_rcu.  Fixes at least one
> assertion failure seen in s390, where a ref count for a memory
> region attempts to go negative during hot-unplug of guest memory.
> 
> Signed-off-by: Matthew Rosato <address@hidden>

The two unrefs are correct.

One is needed to balance address_space_get_flatview; the other is 
needed because as->current_map does not point to old_view anymore.

You can remove them with something like this (your patch plus one
extra hunk):

diff --git a/memory.c b/memory.c
index a844ced..5add529 100644
--- a/memory.c
+++ b/memory.c
@@ -747,7 +747,7 @@ static void address_space_update_topology_pass(AddressSpace 
*as,
 
 static void address_space_update_topology(AddressSpace *as)
 {
-    FlatView *old_view = address_space_get_flatview(as);
+    FlatView *old_view = as->current_map;
     FlatView *new_view = generate_memory_topology(as->root);
 
     address_space_update_topology_pass(as, old_view, new_view, false);
@@ -755,7 +755,6 @@ static void address_space_update_topology(AddressSpace *as)
 
     /* Writes are protected by the BQL.  */
     atomic_rcu_set(&as->current_map, new_view);
-    call_rcu(old_view, flatview_unref, rcu);
 
     /* Note that all the old MemoryRegions are still alive up to this
      * point.  This relieves most MemoryListeners from the need to
@@ -763,7 +762,7 @@ static void address_space_update_topology(AddressSpace *as)
      * outside the iothread mutex, in which case precise reference
      * counting is necessary.
      */
-    flatview_unref(old_view);
+    call_rcu(old_view, flatview_unref, rcu);
 
     address_space_update_ioeventfds(as);
 }

but it wouldn't affect your bug.

Paolo