[v2,0/5] iothread: create gcontext unconditionally

Message

Peter Xu March 6, 2019, 11:55 a.m. UTC

v2:
- add comment in patch 4
- add another patch to comment why we need explicit aio_poll() in
  iothread_run loop

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 (5):
  iothread: replace init_done_cond with a semaphore
  iothread: create the gcontext unconditionally
  iothread: create main loop unconditionally
  iothread: push gcontext earlier in the thread_fn
  iothread: document about why we need explicit aio_poll()

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

Comments

Stefan Hajnoczi March 8, 2019, 10:21 a.m. UTC | #1

On Wed, Mar 06, 2019 at 07:55:27PM +0800, Peter Xu wrote:
> v2:
> - add comment in patch 4
> - add another patch to comment why we need explicit aio_poll() in
>   iothread_run loop
> 
> 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 (5):
>   iothread: replace init_done_cond with a semaphore
>   iothread: create the gcontext unconditionally
>   iothread: create main loop unconditionally
>   iothread: push gcontext earlier in the thread_fn
>   iothread: document about why we need explicit aio_poll()
> 
>  include/sysemu/iothread.h |  5 +--
>  iothread.c                | 91 +++++++++++++++++++--------------------
>  2 files changed, 46 insertions(+), 50 deletions(-)
> 
> -- 
> 2.17.1
> 

I made the comment tweak discussed in this thread.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan