Re: [Qemu-devel] [PATCH] Memory: use memory address space for cpu-memory

[Paolo Bonzini]

On 17/05/2017 19:01, Xu, Anthony wrote:
>>> -        AddressSpace *as = address_space_init_shareable(cpu->memory,
>>> -                                                        "cpu-memory");
>>> +        AddressSpace *as;
>>> +        if (cpu->memory == address_space_memory.root) {
>>> +            address_space_memory.ref_count++;
>> probably this would cause reference leak when vcpu is destroyed
> I thought address_space_destroy is called when vcpu is unplugged,
> seems that's not the case, then ref_count++ is not needed.

It should be called.  Alternatively you could try adding a new function
to mark address_space_memory as a never-destroyed AddressSpace:

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 99e0f54d86..b27b288c8f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -290,6 +290,7 @@ struct AddressSpace {
     MemoryRegion *root;
     int ref_count;
     bool malloced;
+    bool shared;
 
     /* Accessed via RCU.  */
     struct FlatView *current_map;
diff --git a/memory.c b/memory.c
index b727f5ec0e..190cd3d5ce 100644
--- a/memory.c
+++ b/memory.c
@@ -2432,6 +2432,7 @@ void address_space_init(AddressSpace *as, MemoryRegion 
*root, const char *name)
     as->ref_count = 1;
     as->root = root;
     as->malloced = false;
+    as->shared = false;
     as->current_map = g_new(FlatView, 1);
     flatview_init(as->current_map);
     as->ioeventfd_nb = 0;
@@ -2460,12 +2461,18 @@ static void do_address_space_destroy(AddressSpace *as)
     }
 }
 
+void address_space_init_static(AddressSpace *as, MemoryRegion *root, const 
char *name)
+{
+    address_space_init(as, root, name);
+    as->shared = true;
+}
+
 AddressSpace *address_space_init_shareable(MemoryRegion *root, const char 
*name)
 {
     AddressSpace *as;
 
     QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-        if (root == as->root && as->malloced) {
+        if (root == as->root && as->shared) {
             as->ref_count++;
             return as;
         }
@@ -2474,6 +2481,7 @@ AddressSpace *address_space_init_shareable(MemoryRegion 
*root, const char *name)
     as = g_malloc0(sizeof *as);
     address_space_init(as, root, name);
     as->malloced = true;
+    as->shared = true;
     return as;
 }
 
@@ -2485,6 +2493,8 @@ void address_space_destroy(AddressSpace *as)
     if (as->ref_count) {
         return;
     }
+    assert(!as->shared || as->malloced);
+
     /* Flush out anything from MemoryListeners listening in on this */
     memory_region_transaction_begin();
     as->root = NULL;


then CPUs can keep using address_space_init_shareable.

Paolo