Re: [PATCH v2 3/3] memory: Stop piggybacking on memory region owners

[Paolo Bonzini]

On 9/6/25 04:39, Akihiko Odaki wrote:
+static int collect_memory_region(Object *child, void *opaque)
+{
+    if (object_dynamic_cast(child, TYPE_MEMORY_REGION)) {
+        g_array_append_val(opaque, child);
+    }
+
+    return 0;
+}
+
  static void device_unparent(Object *obj)
  {
+    g_autoptr(GArray) array = g_array_new(FALSE, FALSE, sizeof(Object *));

We can use GPtrArray too, instead.

      DeviceState *dev = DEVICE(obj);
      BusState *bus;
  
@@ -735,6 +745,12 @@ static void device_unparent(Object *obj)
          object_unref(OBJECT(dev->parent_bus));
          dev->parent_bus = NULL;
      }
+
+    object_child_foreach(OBJECT(dev), collect_memory_region, array);
+
+    for (gsize i = 0; i < array->len; i++) {
+        object_unparent(g_array_index(array, Object *, i));
+    }
  }
  
  static char *

Very good idea!  The purpose of unparent is basically to break the 
circular references, and it's a bit weird to use object_child_foreach 
but it makes a lot of sense.

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index f5cba8a7fa66..7afcaa4bfe2e 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -203,6 +203,7 @@ virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
          /* memory region owns self res->mr object and frees it by itself */
          memory_region_set_enabled(mr, false);
          memory_region_del_subregion(&b->hostmem, mr);
+        object_unparent(OBJECT(mr));
          object_unparent(OBJECT(vmr));
      }g
  

Can you do this in vmr's unparent callback?  Even the 
memory_region_set_enabled and del_subregion calls could belong there.

+static void memory_region_unparent(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    object_ref(mr->owner);
+}
+

This probably should assert that OBJECT(mr)->free == memory_region_free. 
 This provides safety, and also clarifies where the matching unref is.

Paolo