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

[Fam Zheng <famz@redhat.com>]

On Mon, 03/16 08:34, Paolo Bonzini wrote:
> 
> 
> 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 <famz@redhat.com>
> > ---
> >  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)?

Exactly, will rename. Thanks.

Fam

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