[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qemu-char: eliminate busy waiting on can_rea
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH v2] qemu-char: eliminate busy waiting on can_read returning zero |
Date: |
Sun, 7 Apr 2013 00:30:07 +0530 |
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 <address@hidden>
> ---
> 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