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

[Paolo Bonzini <pbonzini@redhat.com>]

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