Re: [Qemu-devel] [PATCH v2] qemu-char: eliminate busy waiting on can_read returning zero

[Amit Shah <amit.shah@redhat.com>]

On (Fri) 05 Apr 2013 [17:59:33], Paolo Bonzini wrote:
> The character backend refactoring introduced an undesirable busy wait.
> The busy wait happens if can_read returns zero and there is data available
> on the character device's file descriptor.  Then, the I/O watch will
> fire continuously and, with TCG, the CPU thread will never run.
> 
>     1) Char backend asks front end if it can write
>     2) Front end says no
>     3) poll() finds the char backend's descriptor is available
>     4) Goto (1)
> 
> What we really want is this (note that step 3 avoids the busy wait):
> 
>     1) Char backend asks front end if it can write
>     2) Front end says no
>     3) poll() goes on without char backend's descriptor
>     4) Goto (1) until qemu_chr_accept_input() called
> 
>     5) Char backend asks front end if it can write
>     6) Front end says yes
>     7) poll() finds the char backend's descriptor is available
>     8) Backend handler called
> 
> After this patch, the IOWatchPoll source and the watch source are
> separated.  The IOWatchPoll is simply a hook that runs during the prepare
> phase on each main loop iteration.  The hook adds/removes the actual
> source depending on the return value from can_read.
> 
> A simple reproducer is
> 
>     qemu-system-i386 -serial mon:stdio
> 
> ... followed by banging on the terminal as much as you can. :)  Without
> this patch, emulation will hang.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: use g_source_get_context to find if the watch was active

>  static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_)
>  {
>      IOWatchPoll *iwp = io_watch_poll_from_source(source);
> -
> -    iwp->max_size = iwp->fd_can_read(iwp->opaque);
> -    if (iwp->max_size == 0) {
> +    bool now_active = iwp->fd_can_read(iwp->opaque) > 0;
> +    bool was_active = g_source_get_context(iwp->src) != NULL;

This gives me a bunch of

(process:30075): GLib-CRITICAL **: g_source_get_context: assertion `!SOURCE_DESTROYED (source)' failed

messages

		Amit