From e79e0881cd61c0108b2994f1f952df5ea3aec81d Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 22 Feb 2018 13:43:01 -0500 Subject: [PATCH] memory: RCU ram_list.dirty_memory[] for safe RAM hotplug Although accesses to ram_list.dirty_memory[] use atomics so multiple threads can safely dirty the bitmap, the data structure is not fully thread-safe yet. This patch handles the RAM hotplug case where ram_list.dirty_memory[] is grown. ram_list.dirty_memory[] is change from a regular bitmap to an RCU array of pointers to fixed-size bitmap blocks. Threads can continue accessing bitmap blocks while the array is being extended. See the comments in the code for an in-depth explanation of struct DirtyMemoryBlocks. I have tested that live migration with virtio-blk dataplane works. Backports commit 5b82b703b69acc67b78b98a5efc897a3912719eb from qemu --- qemu/exec.c | 86 +++++++++++++++++++---- qemu/include/exec/ram_addr.h | 131 +++++++++++++++++++++++++++++++---- qemu/include/exec/ramlist.h | 36 +++++++++- qemu/include/qemu/bitmap.h | 15 ++++ uc.c | 20 +++++- 5 files changed, 258 insertions(+), 30 deletions(-) diff --git a/qemu/exec.c b/qemu/exec.c index 77ff43cc..edf8b2e9 100644 --- a/qemu/exec.c +++ b/qemu/exec.c @@ -772,8 +772,9 @@ bool cpu_physical_memory_test_and_clear_dirty(struct uc_struct *uc, ram_addr_t length, unsigned client) { + DirtyMemoryBlocks *blocks; unsigned long end, page; - bool dirty; + bool dirty = false; if (length == 0) { return false; @@ -781,8 +782,25 @@ bool cpu_physical_memory_test_and_clear_dirty(struct uc_struct *uc, end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; page = start >> TARGET_PAGE_BITS; - dirty = bitmap_test_and_clear_atomic(uc->ram_list.dirty_memory[client], - page, end - page); + + // Unicorn: commented out + //rcu_read_lock(); + + // Unicorn: atomic_read instead of atomic_rcu_read used + blocks = atomic_read(&uc->ram_list.dirty_memory[client]); + + while (page < end) { + unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; + unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; + unsigned long num = MIN(end - page, DIRTY_MEMORY_BLOCK_SIZE - offset); + + dirty |= bitmap_test_and_clear_atomic(blocks->blocks[idx], + offset, num); + page += num; + } + + // Unicorn: commented out + //rcu_read_unlock(); if (dirty && tcg_enabled(uc)) { tlb_reset_dirty_range_all(uc, start, length); @@ -1109,6 +1127,51 @@ int qemu_ram_resize(struct uc_struct *uc, ram_addr_t base, ram_addr_t newsize, E return 0; } +/* Called with ram_list.mutex held */ +static void dirty_memory_extend(struct uc_struct *uc, + ram_addr_t old_ram_size, + ram_addr_t new_ram_size) +{ + ram_addr_t old_num_blocks = DIV_ROUND_UP(old_ram_size, + DIRTY_MEMORY_BLOCK_SIZE); + ram_addr_t new_num_blocks = DIV_ROUND_UP(new_ram_size, + DIRTY_MEMORY_BLOCK_SIZE); + int i; + + /* Only need to extend if block count increased */ + if (new_num_blocks <= old_num_blocks) { + return; + } + + for (i = 0; i < DIRTY_MEMORY_NUM; i++) { + DirtyMemoryBlocks *old_blocks; + DirtyMemoryBlocks *new_blocks; + int j; + + // Unicorn: atomic_read used instead of atomic_rcu_read + old_blocks = atomic_read(&uc->ram_list.dirty_memory[i]); + new_blocks = g_malloc(sizeof(*new_blocks) + + sizeof(new_blocks->blocks[0]) * new_num_blocks); + // Unicorn: unicorn-specific variable to make memory handling less painful. + new_blocks->num_blocks = new_num_blocks; + + if (old_num_blocks) { + memcpy(new_blocks->blocks, old_blocks->blocks, + old_num_blocks * sizeof(old_blocks->blocks[0])); + } + + for (j = old_num_blocks; j < new_num_blocks; j++) { + new_blocks->blocks[j] = bitmap_new(DIRTY_MEMORY_BLOCK_SIZE); + } + + // Unicorn: atomic_set used instead of atomic_rcu_set + atomic_set(&uc->ram_list.dirty_memory[i], new_blocks); + + // Unicorn: g_free used instead of g_free_rcu + g_free(old_blocks); + } +} + static void ram_block_add(struct uc_struct *uc, RAMBlock *new_block, Error **errp) { RAMBlock *block; @@ -1131,6 +1194,13 @@ static void ram_block_add(struct uc_struct *uc, RAMBlock *new_block, Error **err memory_try_enable_merging(new_block->host, new_block->max_length); } + new_ram_size = MAX(old_ram_size, + (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS); + if (new_ram_size > old_ram_size) { + // Unicorn: commented out + //migration_bitmap_extend(old_ram_size, new_ram_size); + dirty_memory_extend(uc, old_ram_size, new_ram_size); + } /* Keep the list sorted from biggest to smallest block. Unlike QTAILQ, * QLIST (which has an RCU-friendly variant) does not have insertion at * tail, so save the last element in last_block. @@ -1154,16 +1224,6 @@ static void ram_block_add(struct uc_struct *uc, RAMBlock *new_block, Error **err smp_wmb(); uc->ram_list.version++; - new_ram_size = last_ram_offset(uc) >> TARGET_PAGE_BITS; - - if (new_ram_size > old_ram_size) { - int i; - for (i = 0; i < DIRTY_MEMORY_NUM; i++) { - uc->ram_list.dirty_memory[i] = - bitmap_zero_extend(uc->ram_list.dirty_memory[i], - old_ram_size, new_ram_size); - } - } cpu_physical_memory_set_dirty_range(uc, new_block->offset, new_block->used_length, DIRTY_CLIENTS_ALL); diff --git a/qemu/include/exec/ram_addr.h b/qemu/include/exec/ram_addr.h index c0dca1b5..96a01042 100644 --- a/qemu/include/exec/ram_addr.h +++ b/qemu/include/exec/ram_addr.h @@ -74,30 +74,77 @@ static inline bool cpu_physical_memory_get_dirty(struct uc_struct *uc, ram_addr_ ram_addr_t length, unsigned client) { - unsigned long end, page, next; + DirtyMemoryBlocks *blocks; + unsigned long end, page; + bool dirty = false; assert(client < DIRTY_MEMORY_NUM); end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; page = start >> TARGET_PAGE_BITS; - next = find_next_bit(uc->ram_list.dirty_memory[client], end, page); - return next < end; + // Unicorn: commented out + //rcu_read_lock(); + + // Unicorn: atomic_read used instead of atomic_rcu_read + blocks = atomic_read(&uc->ram_list.dirty_memory[client]); + + while (page < end) { + unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; + unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; + unsigned long num = MIN(end - page, DIRTY_MEMORY_BLOCK_SIZE - offset); + + if (find_next_bit(blocks->blocks[idx], offset, num) < num) { + dirty = true; + break; + } + + page += num; + } + + // Unicorn: commented out + //rcu_read_unlock(); + + return dirty; + } static inline bool cpu_physical_memory_all_dirty(struct uc_struct *uc, ram_addr_t start, ram_addr_t length, unsigned client) { - unsigned long end, page, next; + DirtyMemoryBlocks *blocks; + unsigned long end, page; + bool dirty = true; assert(client < DIRTY_MEMORY_NUM); end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; page = start >> TARGET_PAGE_BITS; - next = find_next_zero_bit(uc->ram_list.dirty_memory[client], end, page); - return next >= end; + // Unicorn: commented out + //rcu_read_lock(); + + // Unicorn: atomic_read used instead of atomic_rcu_read + blocks = atomic_read(&uc->ram_list.dirty_memory[client]); + + while (page < end) { + unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; + unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; + unsigned long num = MIN(end - page, DIRTY_MEMORY_BLOCK_SIZE - offset); + + if (find_next_zero_bit(blocks->blocks[idx], offset, num) < num) { + dirty = false; + break; + } + + page += num; + } + + // Unicorn: commented out + //rcu_read_unlock(); + + return dirty; } static inline bool cpu_physical_memory_get_dirty_flag(struct uc_struct *uc, ram_addr_t addr, @@ -126,16 +173,34 @@ static inline bool cpu_physical_memory_range_includes_clean(struct uc_struct *uc static inline void cpu_physical_memory_set_dirty_flag(struct uc_struct *uc, ram_addr_t addr, unsigned client) { + unsigned long page, idx, offset; + DirtyMemoryBlocks *blocks; + assert(client < DIRTY_MEMORY_NUM); - set_bit_atomic(addr >> TARGET_PAGE_BITS, uc->ram_list.dirty_memory[client]); + + page = addr >> TARGET_PAGE_BITS; + idx = page / DIRTY_MEMORY_BLOCK_SIZE; + offset = page % DIRTY_MEMORY_BLOCK_SIZE; + + // Unicorn: commented out + //rcu_read_lock(); + + // Unicorn: atomic_read used instead of atomic_rcu_read + blocks = atomic_read(&uc->ram_list.dirty_memory[client]); + + set_bit_atomic(offset, blocks->blocks[idx]); + + // Unicorn: commented out + //rcu_read_unlock(); } static inline void cpu_physical_memory_set_dirty_range(struct uc_struct *uc, ram_addr_t start, ram_addr_t length, uint8_t mask) { + DirtyMemoryBlocks *blocks[DIRTY_MEMORY_NUM]; unsigned long end, page; - unsigned long **d = uc->ram_list.dirty_memory; + int i; if (!mask && !xen_enabled()) { return; @@ -143,10 +208,30 @@ static inline void cpu_physical_memory_set_dirty_range(struct uc_struct *uc, ram end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; page = start >> TARGET_PAGE_BITS; - if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) { - bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page); + + // Unicorn: commented out + //rcu_read_lock(); + + for (i = 0; i < DIRTY_MEMORY_NUM; i++) { + // Unicorn: atomic_read used instead of atomic_rcu_read + blocks[i] = atomic_read(&uc->ram_list.dirty_memory[i]); } + while (page < end) { + unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE; + unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE; + unsigned long num = MIN(end - page, DIRTY_MEMORY_BLOCK_SIZE - offset); + + if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) { + bitmap_set_atomic(blocks[DIRTY_MEMORY_CODE]->blocks[idx], + offset, num); + } + + page += num; + } + + // Unicorn: commented out + //rcu_read_unlock(); } #if !defined(_WIN32) @@ -165,19 +250,41 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(struct uc_struct *uc, /* start address is aligned at the start of a word? */ if ((((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) && (hpratio == 1)) { + unsigned long **blocks[DIRTY_MEMORY_NUM]; + unsigned long idx; + unsigned long offset; long k; long nr = BITS_TO_LONGS(pages); + idx = (start >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE; + offset = BIT_WORD((start >> TARGET_PAGE_BITS) % + DIRTY_MEMORY_BLOCK_SIZE); + + // Unicorn: commented out + //rcu_read_lock(); + + for (i = 0; i < DIRTY_MEMORY_NUM; i++) { + // Unicorn: atomic_read used instead of atomic_rcu_read + blocks[i] = atomic_read(&uc->ram_list.dirty_memory[i])->blocks; + } + for (k = 0; k < nr; k++) { if (bitmap[k]) { unsigned long temp = leul_to_cpu(bitmap[k]); - unsigned long **d = uc->ram_list.dirty_memory; if (tcg_enabled(uc)) { - atomic_or(&d[DIRTY_MEMORY_CODE][page + k], temp); + atomic_or(&blocks[DIRTY_MEMORY_CODE][idx][offset], temp); } } + + if (++offset >= BITS_TO_LONGS(DIRTY_MEMORY_BLOCK_SIZE)) { + offset = 0; + idx++; + } } + + // Unicorn: commented out + //rcu_read_unlock(); } else { uint8_t clients = tcg_enabled(uc) ? DIRTY_CLIENTS_ALL : DIRTY_CLIENTS_NOCODE; /* diff --git a/qemu/include/exec/ramlist.h b/qemu/include/exec/ramlist.h index c3d7f860..3e04a26a 100644 --- a/qemu/include/exec/ramlist.h +++ b/qemu/include/exec/ramlist.h @@ -7,11 +7,43 @@ #define DIRTY_MEMORY_CODE 0 #define DIRTY_MEMORY_NUM 1 /* num of dirty bits */ +/* The dirty memory bitmap is split into fixed-size blocks to allow growth + * under RCU. The bitmap for a block can be accessed as follows: + * + * rcu_read_lock(); + * + * DirtyMemoryBlocks *blocks = + * atomic_rcu_read(&ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]); + * + * ram_addr_t idx = (addr >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE; + * unsigned long *block = blocks.blocks[idx]; + * ...access block bitmap... + * + * rcu_read_unlock(); + * + * Remember to check for the end of the block when accessing a range of + * addresses. Move on to the next block if you reach the end. + * + * Organization into blocks allows dirty memory to grow (but not shrink) under + * RCU. When adding new RAMBlocks requires the dirty memory to grow, a new + * DirtyMemoryBlocks array is allocated with pointers to existing blocks kept + * the same. Other threads can safely access existing blocks while dirty + * memory is being grown. When no threads are using the old DirtyMemoryBlocks + * anymore it is freed by RCU (but the underlying blocks stay because they are + * pointed to from the new DirtyMemoryBlocks). + */ +#define DIRTY_MEMORY_BLOCK_SIZE ((ram_addr_t)256 * 1024 * 8) +typedef struct { + //struct rcu_head rcu; + // Unicorn: unicorn-specific variable to make memory handling less painful + size_t num_blocks; + unsigned long *blocks[]; +} DirtyMemoryBlocks; + typedef struct RAMList { - /* Protected by the iothread lock. */ - unsigned long *dirty_memory[DIRTY_MEMORY_NUM]; RAMBlock *mru_block; QLIST_HEAD(, RAMBlock) blocks; + DirtyMemoryBlocks *dirty_memory[DIRTY_MEMORY_NUM]; uint32_t version; } RAMList; diff --git a/qemu/include/qemu/bitmap.h b/qemu/include/qemu/bitmap.h index 9488f44a..e39566d6 100644 --- a/qemu/include/qemu/bitmap.h +++ b/qemu/include/qemu/bitmap.h @@ -48,6 +48,21 @@ #define DECLARE_BITMAP(name,bits) \ unsigned long name[BITS_TO_LONGS(bits)] +static inline unsigned long *bitmap_try_new(long nbits) +{ + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); + return g_try_malloc0(len); +} + +static inline unsigned long *bitmap_new(long nbits) +{ + unsigned long *ptr = bitmap_try_new(nbits); + if (ptr == NULL) { + abort(); + } + return ptr; +} + void bitmap_set(unsigned long *map, long i, long len); void bitmap_set_atomic(unsigned long *map, long i, long len); void bitmap_clear(unsigned long *map, long start, long nr); diff --git a/uc.c b/uc.c index 6d6f4c3e..d9adb19b 100644 --- a/uc.c +++ b/uc.c @@ -279,6 +279,22 @@ uc_err uc_open(uc_arch arch, uc_mode mode, uc_engine **result) } } +static void ramlist_free_dirty_memory(struct uc_struct *uc) +{ + int i; + DirtyMemoryBlocks** blocks = uc->ram_list.dirty_memory; + + for (i = 0; i < DIRTY_MEMORY_NUM; i++) { + DirtyMemoryBlocks* block = uc->ram_list.dirty_memory[i]; + int j; + + for (j = 0; j < block->num_blocks; j++) { + free(block->blocks[j]); + } + + free(blocks[i]); + } +} UNICORN_EXPORT uc_err uc_close(uc_engine *uc) @@ -327,9 +343,7 @@ uc_err uc_close(uc_engine *uc) g_hash_table_foreach(uc->type_table, free_table, uc); g_hash_table_destroy(uc->type_table); - for (i = 0; i < DIRTY_MEMORY_NUM; i++) { - free(uc->ram_list.dirty_memory[i]); - } + ramlist_free_dirty_memory(uc); // free hooks and hook lists for (i = 0; i < UC_HOOK_MAX; i++) {