[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/4] exec: Notify cpu_register_map_client cal
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/4] exec: Notify cpu_register_map_client caller if the bounce buffer is available |
Date: |
Mon, 16 Mar 2015 15:44:47 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 <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)?
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);
> > }
> >
> >