When I first read the iothread code, the gcontext confused me for
quite a while.  Meanwhile, I've been tackling with some races due to
this complexity as well.  How much we'll pay for creating the gcontext
unconditionally?  Do we really need this flexibitily (or is it really
a flexibility after all)?  I don't see much gain of existing code, but
I might be wrong.  Anyway, I wrote this patchset to see how the list
would think about it.

This series directly originates from previous discussion with
Marc-Andre where there's a slightly hacky way to try to acquire the
gcontext:

https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05460.html

Now with this series logically above patch is not needed any more.
Please read patch 4 for more information.

And if this patchset can survive... how about running gcontext
directly in iothread_run()?  I believe there could be a bit more
things to clean but I'll see.

Make check passes for me.

Comments welcomed.  Thanks,

Peter Xu (4):
  iothread: replace init_done_cond with a semaphore
  iothread: create the gcontext onconditionally
  iothread: create main loop unconditionally
  iothread: push gcontext earlier in the thread_fn

 include/sysemu/iothread.h |  5 +--
 iothread.c                | 77 +++++++++++++++------------------------
 2 files changed, 32 insertions(+), 50 deletions(-)

-- 
2.17.1

On Fri, Feb 22, 2019 at 11:14:09AM +0800, Peter Xu wrote:
> Comments welcomed.  Thanks,
> 
> Peter Xu (4):
>   iothread: replace init_done_cond with a semaphore
>   iothread: create the gcontext onconditionally
>   iothread: create main loop unconditionally
>   iothread: push gcontext earlier in the thread_fn

What is the status of this series?  Will you send another revision?

Stefan

On Wed, Mar 06, 2019 at 10:19:59AM +0000, Stefan Hajnoczi wrote:
> On Fri, Feb 22, 2019 at 11:14:09AM +0800, Peter Xu wrote:
> > Comments welcomed.  Thanks,
> > 
> > Peter Xu (4):
> >   iothread: replace init_done_cond with a semaphore
> >   iothread: create the gcontext onconditionally
> >   iothread: create main loop unconditionally
> >   iothread: push gcontext earlier in the thread_fn
> 
> What is the status of this series?  Will you send another revision?

I'll add some comment to patch 4 and repost, probably with another
patch to document why we can't drop aio_poll in iothread_run.

Regards,

-- 
Peter Xu

On 22/02/19 04:14, Peter Xu wrote:
> And if this patchset can survive... how about running gcontext
> directly in iothread_run()?  I believe there could be a bit more
> things to clean but I'll see.

Do you mean instead of aio_poll?  The problem is that GMainContext is
quite a bit slower than aio_poll.

Frediano and I tried to bring some of the optimizations of aio_poll to
GMainContext
(https://github.com/GNOME/glib/commit/e91c11841808ccca408da96136f433a82b2e2145),
but they broke webkit. :(

Paolo

On Fri, Feb 22, 2019 at 10:28:57AM +0100, Paolo Bonzini wrote:
> On 22/02/19 04:14, Peter Xu wrote:
> > And if this patchset can survive... how about running gcontext
> > directly in iothread_run()?  I believe there could be a bit more
> > things to clean but I'll see.
> 
> Do you mean instead of aio_poll?

Yes.

> The problem is that GMainContext is
> quite a bit slower than aio_poll.

That's really what I wanted to know; so it's about performance.

We should mention it somewhere in iothread_run.  I can do that after I
know how this series will go.

> 
> Frediano and I tried to bring some of the optimizations of aio_poll to
> GMainContext
> (https://github.com/GNOME/glib/commit/e91c11841808ccca408da96136f433a82b2e2145),
> but they broke webkit. :(

What a pity!

-- 
Peter Xu