Message ID | 1433776757-61958-2-git-send-email-imammedo@redhat.com |
---|---|
State | New |
Headers | show |
On 08/06/2015 17:19, Igor Mammedov wrote: > - qemu_mutex_lock_ramlist(); > - QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > - if (addr == block->offset) { > - QLIST_REMOVE_RCU(block, next); > - ram_list.mru_block = NULL; > - /* Write list before version */ > - smp_wmb(); > - ram_list.version++; > - g_free_rcu(block, rcu); qemu_ram_free here does: call_rcu(block, reclaim_ramblock, rcu); which is different. Paolo > - break; > - } > - } > - qemu_mutex_unlock_ramlist();
On Mon, 08 Jun 2015 17:23:35 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 08/06/2015 17:19, Igor Mammedov wrote: > > - qemu_mutex_lock_ramlist(); > > - QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > > - if (addr == block->offset) { > > - QLIST_REMOVE_RCU(block, next); > > - ram_list.mru_block = NULL; > > - /* Write list before version */ > > - smp_wmb(); > > - ram_list.version++; > > - g_free_rcu(block, rcu); > > qemu_ram_free here does: > > call_rcu(block, reclaim_ramblock, rcu); > > which is different. qemu_ram_free() calls reclaim_ramblock() which does: if (!(block->flags & RAM_PREALLOC)) free_host_memory() g_free(block) while g_free_rcu(block, rcu) results -> g_free(block) and for memory_region_init_ram_ptr() we set RAM_PREALLOC so qemu_ram_free() degrades to g_free(block). > > Paolo > > > - break; > > - } > > - } > > - qemu_mutex_unlock_ramlist();
On 08/06/2015 18:08, Igor Mammedov wrote: > On Mon, 08 Jun 2015 17:23:35 +0200 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> >> >> On 08/06/2015 17:19, Igor Mammedov wrote: >>> - qemu_mutex_lock_ramlist(); >>> - QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { >>> - if (addr == block->offset) { >>> - QLIST_REMOVE_RCU(block, next); >>> - ram_list.mru_block = NULL; >>> - /* Write list before version */ >>> - smp_wmb(); >>> - ram_list.version++; >>> - g_free_rcu(block, rcu); >> >> qemu_ram_free here does: >> >> call_rcu(block, reclaim_ramblock, rcu); >> >> which is different. > > > qemu_ram_free() calls reclaim_ramblock() which does: > > if (!(block->flags & RAM_PREALLOC)) > free_host_memory() > > g_free(block) > > while > g_free_rcu(block, rcu) results -> g_free(block) > > and for memory_region_init_ram_ptr() we set RAM_PREALLOC > so qemu_ram_free() degrades to g_free(block). Please put this in the commit message. :) Paolo
On Mon, 08 Jun 2015 18:09:17 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 08/06/2015 18:08, Igor Mammedov wrote: > > On Mon, 08 Jun 2015 17:23:35 +0200 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > >> > >> > >> On 08/06/2015 17:19, Igor Mammedov wrote: > >>> - qemu_mutex_lock_ramlist(); > >>> - QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > >>> - if (addr == block->offset) { > >>> - QLIST_REMOVE_RCU(block, next); > >>> - ram_list.mru_block = NULL; > >>> - /* Write list before version */ > >>> - smp_wmb(); > >>> - ram_list.version++; > >>> - g_free_rcu(block, rcu); > >> > >> qemu_ram_free here does: > >> > >> call_rcu(block, reclaim_ramblock, rcu); > >> > >> which is different. > > > > > > qemu_ram_free() calls reclaim_ramblock() which does: > > > > if (!(block->flags & RAM_PREALLOC)) > > free_host_memory() > > > > g_free(block) > > > > while > > g_free_rcu(block, rcu) results -> g_free(block) > > > > and for memory_region_init_ram_ptr() we set RAM_PREALLOC > > so qemu_ram_free() degrades to g_free(block). > > Please put this in the commit message. :) ok, I'll post it as separate cleanup patch as it doesn't depend on the rest of series. > > Paolo
diff --git a/exec.c b/exec.c index e19ab22..d895a86 100644 --- a/exec.c +++ b/exec.c @@ -1555,25 +1555,6 @@ ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz, return qemu_ram_alloc_internal(size, maxsz, resized, NULL, true, mr, errp); } -void qemu_ram_free_from_ptr(ram_addr_t addr) -{ - RAMBlock *block; - - qemu_mutex_lock_ramlist(); - QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { - if (addr == block->offset) { - QLIST_REMOVE_RCU(block, next); - ram_list.mru_block = NULL; - /* Write list before version */ - smp_wmb(); - ram_list.version++; - g_free_rcu(block, rcu); - break; - } - } - qemu_mutex_unlock_ramlist(); -} - static void reclaim_ramblock(RAMBlock *block) { if (block->flags & RAM_PREALLOC) { diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index ff558a4..56d2c17 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -37,7 +37,6 @@ int qemu_get_ram_fd(ram_addr_t addr); void *qemu_get_ram_block_host_ptr(ram_addr_t addr); void *qemu_get_ram_ptr(ram_addr_t addr); void qemu_ram_free(ram_addr_t addr); -void qemu_ram_free_from_ptr(ram_addr_t addr); int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp); diff --git a/memory.c b/memory.c index 03c536b..929123f 100644 --- a/memory.c +++ b/memory.c @@ -869,11 +869,6 @@ 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); -} - static void memory_region_destructor_rom_device(MemoryRegion *mr) { qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK); @@ -1252,7 +1247,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, memory_region_init(mr, owner, name, size); mr->ram = true; mr->terminates = true; - mr->destructor = memory_region_destructor_ram_from_ptr; + mr->destructor = memory_region_destructor_ram; /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL. */ assert(ptr != NULL);
memory_region_destructor_ram_from_ptr() uses qemu_ram_free_from_ptr() to free RAMBlock without freeing host RAM but generic memory_region_destructor_ram() -> qemu_ram_free() does the same so reuse memory_region_destructor_ram() instead. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- exec.c | 19 ------------------- include/exec/ram_addr.h | 1 - memory.c | 7 +------ 3 files changed, 1 insertion(+), 26 deletions(-)