Re: [RFC] memory.c: improve refcounting for RAM vs MMIO regions

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 23/7/25 14:19, Albert Esteve wrote:
> In the last version of the SHMEM MAP/UNMAP [1] there was a
> comment [2] from Stefan about the lifecycle of the memory
> regions.
> 
> After some discussion, David Hildenbrand proposed
> to detect RAM regions and handle refcounting differently
> to clear the initial concern. This RFC patch is
> meant for gathering feedback from others
> (i.e., Paolo Bonzini and Peter Xu).
> 
> [1] https://patchwork.ozlabs.org/project/qemu-devel/list/?series=460121
> [2] https://patchwork.ozlabs.org/comment/3528600/
> 
> ---
> 
> This patch enhances memory_region_ref() and memory_region_unref()
> to handle RAM and MMIO memory regions differently, improving
> reference counting semantics.
> 
> RAM regions now reference/unreference the memory region object
> itself, while MMIO continue to reference/unreference the owner
> device as before.
> 
> An additional qtest has been added to stress the memory
> lifecycle. All tests pass as these changes keep backward
> compatibility (prior behaviour is kept for MMIO regions).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com >
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>   system/memory.c            | 22 +++++++++++++----
>   tests/qtest/ivshmem-test.c | 50 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 67 insertions(+), 5 deletions(-)
> 
> diff --git a/system/memory.c b/system/memory.c
> index 5646547940..48ab6e5592 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -1826,6 +1826,14 @@ Object *memory_region_owner(MemoryRegion *mr)
>   
>   void memory_region_ref(MemoryRegion *mr)
>   {
> +    /* Regions without an owner are considered static. */
> +    if (!mr || !mr->owner) {
> +        return;
> +    }
> +    if (mr->ram) {
> +        object_ref(OBJECT(mr));
> +        return;
> +    }
>       /* MMIO callbacks most likely will access data that belongs
>        * to the owner, hence the need to ref/unref the owner whenever
>        * the memory region is in use.
> @@ -1836,16 +1844,20 @@ void memory_region_ref(MemoryRegion *mr)
>        * Memory regions without an owner are supposed to never go away;

What are the use cases for MRs without QOM owner?

>        * we do not ref/unref them because it slows down DMA sensibly.
>        */
> -    if (mr && mr->owner) {
> -        object_ref(mr->owner);
> -    }
> +    object_ref(mr->owner);
>   }
>   
>   void memory_region_unref(MemoryRegion *mr)
>   {
> -    if (mr && mr->owner) {
> -        object_unref(mr->owner);
> +    /* Regions without an owner are considered static. */
> +    if (!mr || !mr->owner) {
> +        return;
> +    }
> +    if (mr->ram) {
> +        object_unref(OBJECT(mr));
> +        return;
>       }
> +    object_unref(mr->owner);
>   }