On Tue, 02/03 13:52, Paolo Bonzini wrote: > Hence, freeing a RAMBlock has to be switched to call_rcu. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > exec.c | 60 > +++++++++++++++++++++++++++++++++++--------------- > include/exec/cpu-all.h | 2 ++ > 2 files changed, 44 insertions(+), 18 deletions(-) > > diff --git a/exec.c b/exec.c > index a423def..05c5b44 100644 > --- a/exec.c > +++ b/exec.c > @@ -811,7 +811,7 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr) > RAMBlock *block; > > /* The list is protected by the iothread lock here. */ > - block = ram_list.mru_block; > + block = atomic_rcu_read(&ram_list.mru_block); > if (block && addr - block->offset < block->max_length) { > goto found; > } > @@ -825,6 +825,22 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr) > abort(); > > found: > + /* It is safe to write mru_block outside the iothread lock. This > + * is what happens: > + * > + * mru_block = xxx > + * rcu_read_unlock() > + * xxx removed from list > + * rcu_read_lock() > + * read mru_block > + * mru_block = NULL; > + * call_rcu(reclaim_ramblock, > xxx); > + * rcu_read_unlock() > + * > + * atomic_rcu_set is not needed here. The block was already published > + * when it was placed into the list. Here we're just making an extra > + * copy of the pointer. > + */ > ram_list.mru_block = block; > return block; > } > @@ -1381,14 +1397,16 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, > Error **errp) > QTAILQ_INSERT_TAIL(&ram_list.blocks, new_block, next); > } > ram_list.mru_block = NULL; > + atomic_rcu_set(&ram_list.version, ram_list.version + 1); > > - ram_list.version++;
Why is this not atomic_inc (or why is atomic_rcu_set necessary here)? Fam > qemu_mutex_unlock_ramlist(); > > new_ram_size = last_ram_offset() >> TARGET_PAGE_BITS; > > if (new_ram_size > old_ram_size) { > int i; > + > + /* ram_list.dirty_memory[] is protected by the iothread lock. */ > for (i = 0; i < DIRTY_MEMORY_NUM; i++) { > ram_list.dirty_memory[i] = > bitmap_zero_extend(ram_list.dirty_memory[i], > @@ -1525,14 +1543,32 @@ void qemu_ram_free_from_ptr(ram_addr_t addr) > if (addr == block->offset) { > QTAILQ_REMOVE(&ram_list.blocks, block, next); > ram_list.mru_block = NULL; > - ram_list.version++; > - g_free(block); > + atomic_rcu_set(&ram_list.version, ram_list.version + 1); > + call_rcu(block, (void (*)(struct RAMBlock *))g_free, rcu); > break; > } > } > qemu_mutex_unlock_ramlist(); > } > > +static void reclaim_ramblock(RAMBlock *block) > +{ > + if (block->flags & RAM_PREALLOC) { > + ; > + } else if (xen_enabled()) { > + xen_invalidate_map_cache_entry(block->host); > +#ifndef _WIN32 > + } else if (block->fd >= 0) { > + munmap(block->host, block->max_length); > + close(block->fd); > +#endif > + } else { > + qemu_anon_ram_free(block->host, block->max_length); > + } > + g_free(block); > +} > + > +/* Called with the iothread lock held */ > void qemu_ram_free(ram_addr_t addr) > { > RAMBlock *block; > @@ -1543,20 +1579,8 @@ void qemu_ram_free(ram_addr_t addr) > if (addr == block->offset) { > QTAILQ_REMOVE(&ram_list.blocks, block, next); > ram_list.mru_block = NULL; > - ram_list.version++; > - if (block->flags & RAM_PREALLOC) { > - ; > - } else if (xen_enabled()) { > - xen_invalidate_map_cache_entry(block->host); > -#ifndef _WIN32 > - } else if (block->fd >= 0) { > - munmap(block->host, block->max_length); > - close(block->fd); > -#endif > - } else { > - qemu_anon_ram_free(block->host, block->max_length); > - } > - g_free(block); > + atomic_rcu_set(&ram_list.version, ram_list.version + 1); > + call_rcu(block, reclaim_ramblock, rcu); > break; > } > } > diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h > index 2c48286..b8781d1 100644 > --- a/include/exec/cpu-all.h > +++ b/include/exec/cpu-all.h > @@ -24,6 +24,7 @@ > #include "exec/memory.h" > #include "qemu/thread.h" > #include "qom/cpu.h" > +#include "qemu/rcu.h" > > /* some important defines: > * > @@ -268,6 +269,7 @@ CPUArchState *cpu_copy(CPUArchState *env); > typedef struct RAMBlock RAMBlock; > > struct RAMBlock { > + struct rcu_head rcu; > struct MemoryRegion *mr; > uint8_t *host; > ram_addr_t offset; > -- > 1.8.3.1 > >