qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 1/2] main-loop: Pass AioContext into qemu_pol


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v2 1/2] main-loop: Pass AioContext into qemu_poll_ns
Date: Thu, 2 Oct 2014 16:26:18 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Tue, Sep 30, 2014 at 11:35:17AM +0800, Fam Zheng wrote:
> diff --git a/main-loop.c b/main-loop.c
> index d2e64f1..4641ef4 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -234,7 +234,8 @@ static int os_host_main_loop_wait(int64_t timeout)
>          spin_counter++;
>      }
>  
> -    ret = qemu_poll_ns((GPollFD *)gpollfds->data, gpollfds->len, timeout);
> +    ret = qemu_poll_ns(qemu_aio_context, (GPollFD *)gpollfds->data,
> +                       gpollfds->len, timeout);
>  
>      if (timeout) {
>          qemu_mutex_lock_iothread();
> @@ -439,7 +440,8 @@ static int os_host_main_loop_wait(int64_t timeout)
>      poll_timeout_ns = qemu_soonest_timeout(poll_timeout_ns, timeout);
>  
>      qemu_mutex_unlock_iothread();
> -    g_poll_ret = qemu_poll_ns(poll_fds, n_poll_fds + w->num, 
> poll_timeout_ns);
> +    g_poll_ret = qemu_poll_ns(qemu_aio_context,
> +                              poll_fds, n_poll_fds + w->num, 
> poll_timeout_ns);
>  
>      qemu_mutex_lock_iothread();
>      if (g_poll_ret > 0) {

This is a hack.

The immediate problem is that we're not holding the QEMU global mutex
but are accessing qemu_aio_context.  What if other threads touch
qemu_aio_context while they hold the QEMU global mutex?

You didn't indicate what thread-safety rules need to be obeyed so I
wonder whether you looked into this.

I'm also concerned that this is somewhat abusing AioContext.  You want
to stash fields in there but this poll call is about more than just
AioContext, it also includes non-AioContext file descriptors.  So it
seems like a kludge to put the fields into AioContext.

Can you put the state into a separate struct so it's easy to understand
its thread-safety characteristics?

Stefan

Attachment: pgpXYSdMa23HY.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]