[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/9] exec: protect mru_block with RCU
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH 5/9] exec: protect mru_block with RCU |
Date: |
Thu, 5 Feb 2015 14:23:01 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 <address@hidden>
> ---
> 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
>
>
[Qemu-devel] [PATCH 6/9] cosmetic changes preparing for the following patches, Paolo Bonzini, 2015/02/03
[Qemu-devel] [PATCH 8/9] exec: convert ram_list to QLIST, Paolo Bonzini, 2015/02/03