On Sat, Nov 10, 2012 at 9:49 AM, Paolo Bonzini <pbonz...@redhat.com> wrote: > Il 09/11/2012 04:14, Liu Ping Fan ha scritto: >> From: Liu Ping Fan <pingf...@linux.vnet.ibm.com> >> >> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com> >> --- >> exec.c | 21 +++++++++++++-------- >> 1 files changed, 13 insertions(+), 8 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 73d5242..fe84718 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -3296,6 +3296,15 @@ void address_space_destroy_dispatch(AddressSpace *as) >> as->dispatch = NULL; >> } >> >> +typedef struct { >> + QemuMutex lock; >> + void *buffer; >> + target_phys_addr_t addr; >> + target_phys_addr_t len; >> +} BounceBuffer; >> + >> +static BounceBuffer bounce; >> + >> static void memory_map_init(void) >> { >> system_memory = g_malloc(sizeof(*system_memory)); >> @@ -3308,6 +3317,8 @@ static void memory_map_init(void) >> address_space_init(&address_space_io, system_io); >> address_space_io.name = "I/O"; >> >> + qemu_mutex_init(&bounce.lock); >> + >> memory_listener_register(&core_memory_listener, &address_space_memory); >> memory_listener_register(&io_memory_listener, &address_space_io); >> memory_listener_register(&tcg_memory_listener, &address_space_memory); >> @@ -3671,14 +3682,6 @@ void cpu_physical_memory_write_rom(target_phys_addr_t >> addr, >> } >> } >> >> -typedef struct { >> - void *buffer; >> - target_phys_addr_t addr; >> - target_phys_addr_t len; >> -} BounceBuffer; >> - >> -static BounceBuffer bounce; >> - >> typedef struct MapClient { >> void *opaque; >> void (*callback)(void *opaque); >> @@ -3747,6 +3750,7 @@ void *address_space_map(AddressSpace *as, >> section = &mr_obj; >> >> if (!(memory_region_is_ram(section->mr) && !section->readonly)) { >> + qemu_mutex_lock(&bounce.lock); >> if (todo || bounce.buffer) { >> break; >> } > > "todo" must be checked before the if. > Applied, thanks.
> Also, you do not need to keep the lock after address_space_map exits. > In fact, it can be released as soon as bounce.buffer is written to. > After that point, bounce will not be touched (the lock only serves to > serialize writes to bounce.buffer). That is, > But w/o the lock, the content in bounce.buffer for threadA, can be flushed with thread B. > if (todo) { > break; > } > qemu_mutex_lock(&bounce.lock); > if (bounce.buffer) { > qemu_mutex_unlock(&bounce.lock); > break; > } > bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE); > qemu_mutex_unlock(&bounce.lock); > As above. Regards, Pingfan > > Of course, this must be documented. > > Paolo > >> @@ -3807,6 +3811,7 @@ void address_space_unmap(AddressSpace *as, void >> *buffer, target_phys_addr_t len, >> } >> qemu_vfree(bounce.buffer); >> bounce.buffer = NULL; >> + qemu_mutex_unlock(&bounce.lock); >> cpu_notify_map_clients(); >> } >> >> >