Re: [Qemu-devel] [PATCH 4/4] iothread: push gcontext earlier in the thread_fn

[Marc-André Lureau]

Hi

On Fri, Feb 22, 2019 at 4:14 AM Peter Xu <address@hidden> wrote:
>
> We were pushing the context until right before running the gmainloop.
> Now since we have everything unconditionally, we can move this
> earlier.
>
> One benefit is that now it's done even before init_done_sem, so as
> long as the iothread user calls iothread_create() and completes, we
> know that the thread stack is ready.
>

This will change the default context in the iothread, for code running
there. This may not be a good idea. Until now, only sources dispatched
from iothread_get_g_main_context() would have default context
associated to it.

I don't know if the current behaviour is intentional, but it has some
logic. With this change, you may create hidden races, by changing the
default context of sources to the iothread.

> Signed-off-by: Peter Xu <address@hidden>
> ---
>  iothread.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/iothread.c b/iothread.c
> index 9abdbace66..7b7cba5d04 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -53,7 +53,7 @@ static void *iothread_run(void *opaque)
>      IOThread *iothread = opaque;
>
>      rcu_register_thread();
> -
> +    g_main_context_push_thread_default(iothread->worker_context);
>      my_iothread = iothread;
>      iothread->thread_id = qemu_get_thread_id();
>      qemu_sem_post(&iothread->init_done_sem);
> @@ -66,12 +66,11 @@ static void *iothread_run(void *opaque)
>           * changed in previous aio_poll()
>           */
>          if (iothread->running && atomic_read(&iothread->run_gcontext)) {
> -            g_main_context_push_thread_default(iothread->worker_context);
>              g_main_loop_run(iothread->main_loop);
> -            g_main_context_pop_thread_default(iothread->worker_context);
>          }
>      }
>
> +    g_main_context_pop_thread_default(iothread->worker_context);
>      rcu_unregister_thread();
>      return NULL;
>  }
> --
> 2.17.1
>