[PATCH v9 2/2] memory: Do not create circular reference with subregion

[Akihiko Odaki]

memory_region_update_container_subregions() used to call
memory_region_ref(), which creates a reference to the owner of the
subregion, on behalf of the owner of the container. This results in a
circular reference if the subregion and container have the same owner.

memory_region_ref() creates a reference to the owner instead of the
memory region to match the lifetime of the owner and memory region. We
do not need such a hack if the subregion and container have the same
owner because the owner will be alive as long as the container is.
Therefore, create a reference to the subregion itself instead ot its
owner in such a case; the reference to the subregion is still necessary
to ensure that the subregion gets finalized after the container.

Signed-off-by: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
---
 system/memory.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 
56465479406f4a264bfe13e6a2bc7d9b6565410f..1f4111fc24f5a8dda9ee8c277f68f839f5277906
 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1848,6 +1848,31 @@ void memory_region_unref(MemoryRegion *mr)
     }
 }
 
+static void memory_region_ref_subregion(MemoryRegion *mr,
+                                        MemoryRegion *subregion)
+{
+    subregion->container = mr;
+
+    if (mr->owner == subregion->owner) {
+        object_ref(OBJECT(subregion));
+    } else {
+        memory_region_ref(subregion);
+    }
+}
+
+static void memory_region_unref_subregion(MemoryRegion *subregion)
+{
+    MemoryRegion *mr = subregion->container;
+
+    subregion->container = NULL;
+
+    if (mr->owner == subregion->owner) {
+        object_unref(OBJECT(subregion));
+     } else {
+        memory_region_unref(subregion);
+     }
+}
+
 uint64_t memory_region_size(MemoryRegion *mr)
 {
     if (int128_eq(mr->size, int128_2_64())) {
@@ -2618,14 +2643,15 @@ void memory_region_del_eventfd(MemoryRegion *mr,
     memory_region_transaction_commit();
 }
 
-static void memory_region_update_container_subregions(MemoryRegion *subregion)
+static void memory_region_update_container_subregions(MemoryRegion *mr,
+                                                      MemoryRegion *subregion)
 {
-    MemoryRegion *mr = subregion->container;
     MemoryRegion *other;
 
     memory_region_transaction_begin();
 
-    memory_region_ref(subregion);
+    memory_region_ref_subregion(mr, subregion);
+
     QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
         if (subregion->priority >= other->priority) {
             QTAILQ_INSERT_BEFORE(other, subregion, subregions_link);
@@ -2645,12 +2671,11 @@ static void 
memory_region_add_subregion_common(MemoryRegion *mr,
     MemoryRegion *alias;
 
     assert(!subregion->container);
-    subregion->container = mr;
     for (alias = subregion->alias; alias; alias = alias->alias) {
         alias->mapped_via_alias++;
     }
     subregion->addr = offset;
-    memory_region_update_container_subregions(subregion);
+    memory_region_update_container_subregions(mr, subregion);
 }
 
 void memory_region_add_subregion(MemoryRegion *mr,
@@ -2677,13 +2702,13 @@ void memory_region_del_subregion(MemoryRegion *mr,
 
     memory_region_transaction_begin();
     assert(subregion->container == mr);
-    subregion->container = NULL;
     for (alias = subregion->alias; alias; alias = alias->alias) {
         alias->mapped_via_alias--;
         assert(alias->mapped_via_alias >= 0);
     }
     QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
-    memory_region_unref(subregion);
+    memory_region_unref_subregion(subregion);
+
     memory_region_update_pending |= mr->enabled && subregion->enabled;
     memory_region_transaction_commit();
 }

-- 
2.51.0