On Thu, Aug 9, 2012 at 7:28 AM, liu ping fan <qemulist@xxxxxxxxx> wrote: > On Thu, Aug 9, 2012 at 3:17 AM, Blue Swirl <blauwirbel@xxxxxxxxx> wrote: >> On Wed, Aug 8, 2012 at 6:25 AM, Liu Ping Fan <qemulist@xxxxxxxxx> wrote: >>> From: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx> >>> >>> Using mem_map_lock to protect among updaters. So we can get the intact >>> snapshot of mem topology -- FlatView & radix-tree. >>> >>> Signed-off-by: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx> >>> --- >>> exec.c | 3 +++ >>> memory.c | 22 ++++++++++++++++++++++ >>> memory.h | 2 ++ >>> 3 files changed, 27 insertions(+), 0 deletions(-) >>> >>> diff --git a/exec.c b/exec.c >>> index 8244d54..0e29ef9 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -210,6 +210,8 @@ static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc; >>> The bottom level has pointers to MemoryRegionSections. */ >>> static PhysPageEntry phys_map = { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0 }; >>> >>> +QemuMutex mem_map_lock; >>> + >>> static void io_mem_init(void); >>> static void memory_map_init(void); >>> >>> @@ -637,6 +639,7 @@ void cpu_exec_init_all(void) >>> #if !defined(CONFIG_USER_ONLY) >>> memory_map_init(); >>> io_mem_init(); >>> + qemu_mutex_init(&mem_map_lock); >> >> I'd move this and the mutex to memory.c since there are no other uses. >> The mutex could be static then. >> > But the init entry is in exec.c, not memory.c. Memory subsystem does not have an init function of its own, this can be the start of it. > > Regards, > pingfan > >>> #endif >>> } >>> >>> diff --git a/memory.c b/memory.c >>> index aab4a31..5986532 100644 >>> --- a/memory.c >>> +++ b/memory.c >>> @@ -761,7 +761,9 @@ void memory_region_transaction_commit(void) >>> assert(memory_region_transaction_depth); >>> --memory_region_transaction_depth; >>> if (!memory_region_transaction_depth && memory_region_update_pending) { >>> + qemu_mutex_lock(&mem_map_lock); >>> memory_region_update_topology(NULL); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> } >>> >>> @@ -1069,8 +1071,10 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) >>> { >>> uint8_t mask = 1 << client; >>> >>> + qemu_mutex_lock(&mem_map_lock); >>> mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask); >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> bool memory_region_get_dirty(MemoryRegion *mr, target_phys_addr_t addr, >>> @@ -1103,8 +1107,10 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr) >>> void memory_region_set_readonly(MemoryRegion *mr, bool readonly) >>> { >>> if (mr->readonly != readonly) { >>> + qemu_mutex_lock(&mem_map_lock); >>> mr->readonly = readonly; >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> } >>> >>> @@ -1112,7 +1118,9 @@ void memory_region_rom_device_set_readable(MemoryRegion *mr, bool readable) >>> { >>> if (mr->readable != readable) { >>> mr->readable = readable; >>> + qemu_mutex_lock(&mem_map_lock); >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> } >>> >>> @@ -1206,6 +1214,7 @@ void memory_region_add_eventfd(MemoryRegion *mr, >>> }; >>> unsigned i; >>> >>> + qemu_mutex_lock(&mem_map_lock); >>> for (i = 0; i < mr->ioeventfd_nb; ++i) { >>> if (memory_region_ioeventfd_before(mrfd, mr->ioeventfds[i])) { >>> break; >>> @@ -1218,6 +1227,7 @@ void memory_region_add_eventfd(MemoryRegion *mr, >>> sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i)); >>> mr->ioeventfds[i] = mrfd; >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> void memory_region_del_eventfd(MemoryRegion *mr, >>> @@ -1236,6 +1246,7 @@ void memory_region_del_eventfd(MemoryRegion *mr, >>> }; >>> unsigned i; >>> >>> + qemu_mutex_lock(&mem_map_lock); >>> for (i = 0; i < mr->ioeventfd_nb; ++i) { >>> if (memory_region_ioeventfd_equal(mrfd, mr->ioeventfds[i])) { >>> break; >>> @@ -1248,6 +1259,7 @@ void memory_region_del_eventfd(MemoryRegion *mr, >>> mr->ioeventfds = g_realloc(mr->ioeventfds, >>> sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1); >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> static void memory_region_add_subregion_common(MemoryRegion *mr, >>> @@ -1259,6 +1271,8 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, >>> assert(!subregion->parent); >>> subregion->parent = mr; >>> subregion->addr = offset; >>> + >>> + qemu_mutex_lock(&mem_map_lock); >>> QTAILQ_FOREACH(other, &mr->subregions, subregions_link) { >>> if (subregion->may_overlap || other->may_overlap) { >>> continue; >>> @@ -1289,6 +1303,7 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, >>> QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link); >>> done: >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> >>> @@ -1316,8 +1331,11 @@ void memory_region_del_subregion(MemoryRegion *mr, >>> { >>> assert(subregion->parent == mr); >>> subregion->parent = NULL; >>> + >>> + qemu_mutex_lock(&mem_map_lock); >>> QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link); >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> void memory_region_set_enabled(MemoryRegion *mr, bool enabled) >>> @@ -1325,8 +1343,10 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled) >>> if (enabled == mr->enabled) { >>> return; >>> } >>> + qemu_mutex_lock(&mem_map_lock); >>> mr->enabled = enabled; >>> memory_region_update_topology(NULL); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> void memory_region_set_address(MemoryRegion *mr, target_phys_addr_t addr) >>> @@ -1361,7 +1381,9 @@ void memory_region_set_alias_offset(MemoryRegion *mr, target_phys_addr_t offset) >>> return; >>> } >>> >>> + qemu_mutex_lock(&mem_map_lock); >>> memory_region_update_topology(mr); >>> + qemu_mutex_unlock(&mem_map_lock); >>> } >>> >>> ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr) >>> diff --git a/memory.h b/memory.h >>> index 740c48e..fe6aefa 100644 >>> --- a/memory.h >>> +++ b/memory.h >>> @@ -25,6 +25,7 @@ >>> #include "iorange.h" >>> #include "ioport.h" >>> #include "int128.h" >>> +#include "qemu-thread.h" >>> >>> typedef struct MemoryRegionOps MemoryRegionOps; >>> typedef struct MemoryRegion MemoryRegion; >>> @@ -207,6 +208,7 @@ struct MemoryListener { >>> QTAILQ_ENTRY(MemoryListener) link; >>> }; >>> >>> +extern QemuMutex mem_map_lock; >>> /** >>> * memory_region_init: Initialize a memory region >>> * >>> -- >>> 1.7.4.4 >>> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html