diff mbox

memory: do not add a reference to the owner of aliased regions

Message ID 1438007475-32212-1-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini July 27, 2015, 2:31 p.m. UTC
Very often the owner of the aliased region is the same as the owner of the alias
region itself.  When this happens, the reference count can never go back to 0 and
the owner is leaked.  This is for example breaking hot-unplug of virtio-pci
devices (the device cannot be plugged back again with the same id).

Another common use for alias is to transform the system I/O address space
into an MMIO regions; in this case the aliased region never dies, so there
is no problem.  Otherwise the owner is always the same for aliasing
and aliased region.

I checked all calls to memory_region_init_alias introduced after commit
dfde4e6 (memory: add ref/unref calls, 2013-05-06) and they do not need the
reference in order to keep the owner of the aliased region alive.

Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 7 -------
 1 file changed, 7 deletions(-)

Comments

Michael S. Tsirkin July 27, 2015, 3:11 p.m. UTC | #1
On Mon, Jul 27, 2015 at 04:31:15PM +0200, Paolo Bonzini wrote:
> Very often the owner of the aliased region is the same as the owner of the alias
> region itself.  When this happens, the reference count can never go back to 0 and
> the owner is leaked.  This is for example breaking hot-unplug of virtio-pci
> devices (the device cannot be plugged back again with the same id).
> 
> Another common use for alias is to transform the system I/O address space
> into an MMIO regions; in this case the aliased region never dies, so there
> is no problem.  Otherwise the owner is always the same for aliasing
> and aliased region.
> 
> I checked all calls to memory_region_init_alias introduced after commit
> dfde4e6 (memory: add ref/unref calls, 2013-05-06) and they do not need the
> reference in order to keep the owner of the aliased region alive.
> 
> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


Tested-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  memory.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 5e5f325..4eb138a 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -859,11 +859,6 @@ static void memory_region_destructor_ram(MemoryRegion *mr)
>      qemu_ram_free(mr->ram_addr);
>  }
>  
> -static void memory_region_destructor_alias(MemoryRegion *mr)
> -{
> -    memory_region_unref(mr->alias);
> -}
> -
>  static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
>  {
>      qemu_ram_free_from_ptr(mr->ram_addr);
> @@ -1272,8 +1267,6 @@ void memory_region_init_alias(MemoryRegion *mr,
>                                uint64_t size)
>  {
>      memory_region_init(mr, owner, name, size);
> -    memory_region_ref(orig);
> -    mr->destructor = memory_region_destructor_alias;
>      mr->alias = orig;
>      mr->alias_offset = offset;
>  }
> -- 
> 2.4.3
diff mbox

Patch

diff --git a/memory.c b/memory.c
index 5e5f325..4eb138a 100644
--- a/memory.c
+++ b/memory.c
@@ -859,11 +859,6 @@  static void memory_region_destructor_ram(MemoryRegion *mr)
     qemu_ram_free(mr->ram_addr);
 }
 
-static void memory_region_destructor_alias(MemoryRegion *mr)
-{
-    memory_region_unref(mr->alias);
-}
-
 static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
 {
     qemu_ram_free_from_ptr(mr->ram_addr);
@@ -1272,8 +1267,6 @@  void memory_region_init_alias(MemoryRegion *mr,
                               uint64_t size)
 {
     memory_region_init(mr, owner, name, size);
-    memory_region_ref(orig);
-    mr->destructor = memory_region_destructor_alias;
     mr->alias = orig;
     mr->alias_offset = offset;
 }