Re: [Qemu-devel] [RFC v1 1/3] bouce buffer has fine grain lock

[Paolo Bonzini <pbonzini@redhat.com>]

Il 09/11/2012 04:14, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> 
> Signed-off-by: Liu Ping Fan <pingfank@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.

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,

     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);


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();
>  }
>  
>