Message ID | 1438007475-32212-1-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
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 --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; }
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(-)