char: kick main loop after adding a watch

Commit Message

Richard W.M. Jones March 31, 2017, 5:31 p.m. UTC

On Fri, Mar 31, 2017 at 06:53:56PM +0200, Paolo Bonzini wrote:
> glib is expecting QEMU to use g_main_context_acquire around accesses to 
> GMainContext.  However QEMU is not doing that, instead it is taking its 
> own mutex.  So we should add g_main_context_acquire and
> g_main_context_release in the two implementations of 
> os_host_main_loop_wait; these should undo the effect of Frediano's 
> glib patch.

Based on this paragraph, I'm testing the attached patch, and it does
also appear to solve the hanging serial port problem.

Rich.

Comments

Paolo Bonzini March 31, 2017, 5:31 p.m. UTC | #1

On 31/03/2017 19:31, Richard W.M. Jones wrote:
> On Fri, Mar 31, 2017 at 06:53:56PM +0200, Paolo Bonzini wrote:
>> glib is expecting QEMU to use g_main_context_acquire around accesses to 
>> GMainContext.  However QEMU is not doing that, instead it is taking its 
>> own mutex.  So we should add g_main_context_acquire and
>> g_main_context_release in the two implementations of 
>> os_host_main_loop_wait; these should undo the effect of Frediano's 
>> glib patch.
> 
> Based on this paragraph, I'm testing the attached patch, and it does
> also appear to solve the hanging serial port problem.

Great, can you do more testing and/or send the patch formally as a
toplevel message?

Thanks,

Paolo

Richard W.M. Jones March 31, 2017, 5:40 p.m. UTC | #2

On Fri, Mar 31, 2017 at 07:31:57PM +0200, Paolo Bonzini wrote:
> 
> 
> On 31/03/2017 19:31, Richard W.M. Jones wrote:
> > On Fri, Mar 31, 2017 at 06:53:56PM +0200, Paolo Bonzini wrote:
> >> glib is expecting QEMU to use g_main_context_acquire around accesses to 
> >> GMainContext.  However QEMU is not doing that, instead it is taking its 
> >> own mutex.  So we should add g_main_context_acquire and
> >> g_main_context_release in the two implementations of 
> >> os_host_main_loop_wait; these should undo the effect of Frediano's 
> >> glib patch.
> > 
> > Based on this paragraph, I'm testing the attached patch, and it does
> > also appear to solve the hanging serial port problem.
> 
> Great, can you do more testing and/or send the patch formally as a
> toplevel message?

Sure, I'm going to test it a lot more overnight, and then
if it survives all that I'll post it properly.

Thanks,

Rich.

Paolo Bonzini April 1, 2017, 10:08 a.m. UTC | #3

On 31/03/2017 19:31, Richard W.M. Jones wrote:
> On Fri, Mar 31, 2017 at 06:53:56PM +0200, Paolo Bonzini wrote:
>> glib is expecting QEMU to use g_main_context_acquire around accesses to 
>> GMainContext.  However QEMU is not doing that, instead it is taking its 
>> own mutex.  So we should add g_main_context_acquire and
>> g_main_context_release in the two implementations of 
>> os_host_main_loop_wait; these should undo the effect of Frediano's 
>> glib patch.
> 
> Based on this paragraph, I'm testing the attached patch, and it does
> also appear to solve the hanging serial port problem.

Turn out that it may actually be a glib bug.  Your patch would enable
Frediano's optimizations so it's worth applying anyway.

https://bugzilla.gnome.org/show_bug.cgi?id=761102

Paolo

Patch

diff --git a/util/main-loop.c b/util/main-loop.c
index 4534c89..19cad6b 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -218,9 +218,12 @@  static void glib_pollfds_poll(void)
 
 static int os_host_main_loop_wait(int64_t timeout)
 {
+    GMainContext *context = g_main_context_default();
     int ret;
     static int spin_counter;
 
+    g_main_context_acquire(context);
+
     glib_pollfds_fill(&timeout);
 
     /* If the I/O thread is very busy or we are incorrectly busy waiting in
@@ -256,6 +259,9 @@  static int os_host_main_loop_wait(int64_t timeout)
     }
 
     glib_pollfds_poll();
+
+    g_main_context_release(context);
+
     return ret;
 }
 #else
@@ -412,12 +418,15 @@  static int os_host_main_loop_wait(int64_t timeout)
     fd_set rfds, wfds, xfds;
     int nfds;
 
+    g_main_context_acquire(context);
+
     /* XXX: need to suppress polling by better using win32 events */
     ret = 0;
     for (pe = first_polling_entry; pe != NULL; pe = pe->next) {
         ret |= pe->func(pe->opaque);
     }
     if (ret != 0) {
+        g_main_context_release(context);
         return ret;
     }
 
@@ -472,6 +481,8 @@  static int os_host_main_loop_wait(int64_t timeout)
         g_main_context_dispatch(context);
     }
 
+    g_main_context_release(context);
+
     return select_ret || g_poll_ret;
 }
 #endif