Re: [Qemu-devel] [PATCH v3 3/4] exec: Notify cpu_register_map_client caller if the bounce buffer is available

[Paolo Bonzini]

On 16/03/2015 06:31, Fam Zheng wrote:
> The caller's workflow is like
> 
>     if (!address_space_map()) {
>         ...
>         cpu_register_map_client();
>     }
> 
> If bounce buffer became available after address_space_map() but before
> cpu_register_map_client(), the caller could miss it and has to wait for the
> next bounce buffer notify, which may never happen in the worse case.
> 
> Just notify the list in cpu_register_map_client().
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  exec.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 3e54580..20381a0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2489,6 +2489,17 @@ QemuMutex map_client_list_lock;
>  static QLIST_HEAD(map_client_list, MapClient) map_client_list
>      = QLIST_HEAD_INITIALIZER(map_client_list);
>  
> +static void cpu_notify_map_clients_unlocked(void)
> +{
> +    MapClient *client;
> +
> +    while (!QLIST_EMPTY(&map_client_list)) {
> +        client = QLIST_FIRST(&map_client_list);
> +        client->callback(client->opaque);
> +        cpu_unregister_map_client(client);
> +    }
> +}

Isn't the convention to call these functions "*_locked" (e.g.
timer_mod_ns_locked, monitor_flush_locked, cpu_get_clock_locked)?

Otherwise okay.

Paolo

> +
>  void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque))
>  {
>      MapClient *client = g_malloc(sizeof(*client));
> @@ -2497,6 +2508,9 @@ void *cpu_register_map_client(void *opaque, void 
> (*callback)(void *opaque))
>      client->opaque = opaque;
>      client->callback = callback;
>      QLIST_INSERT_HEAD(&map_client_list, client, link);
> +    if (!atomic_read(&bounce.in_use)) {
> +        cpu_notify_map_clients_unlocked();
> +    }
>      qemu_mutex_unlock(&map_client_list_lock);
>      return client;
>  }
> @@ -2521,14 +2535,8 @@ static void cpu_unregister_map_client(void *_client)
>  
>  static void cpu_notify_map_clients(void)
>  {
> -    MapClient *client;
> -
>      qemu_mutex_lock(&map_client_list_lock);
> -    while (!QLIST_EMPTY(&map_client_list)) {
> -        client = QLIST_FIRST(&map_client_list);
> -        client->callback(client->opaque);
> -        cpu_unregister_map_client(client);
> -    }
> +    cpu_notify_map_clients_unlocked();
>      qemu_mutex_unlock(&map_client_list_lock);
>  }
>  
>