From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O9JCy-0000sV-Lc for qemu-devel@nongnu.org; Tue, 04 May 2010 10:34:48 -0400 Received: from [140.186.70.92] (port=50612 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O9JCx-0000rZ-17 for qemu-devel@nongnu.org; Tue, 04 May 2010 10:34:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O9JCr-0004Rq-GZ for qemu-devel@nongnu.org; Tue, 04 May 2010 10:34:46 -0400 Received: from mail-pw0-f45.google.com ([209.85.160.45]:50985) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O9JCr-0004RW-Bt for qemu-devel@nongnu.org; Tue, 04 May 2010 10:34:41 -0400 Received: by pwi6 with SMTP id 6so1771522pwi.4 for ; Tue, 04 May 2010 07:34:40 -0700 (PDT) Message-ID: <4BE0307C.1010305@codemonkey.ws> Date: Tue, 04 May 2010 09:34:36 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] [RESEND] Make char muxer more robust wrt small FIFOs References: <1271782566-16420-1-git-send-email-agraf@suse.de> <4BE024A9.2040904@codemonkey.ws> <16E7906F-F39B-4121-B540-E2F0E67D405E@suse.de> In-Reply-To: <16E7906F-F39B-4121-B540-E2F0E67D405E@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Bastian Blank , qemu-devel Developers , Aurelien Jarno On 05/04/2010 09:30 AM, Alexander Graf wrote: > > > Am 04.05.2010 um 15:44 schrieb Anthony Liguori : > >> On 04/20/2010 11:56 AM, Alexander Graf wrote: >>> Virtio-Console can only process one character at a time. Using it on >>> S390 >>> gave me strage "lags" where I got the character I pressed before when >>> pressing one. So I typed in "abc" and only received "a", then >>> pressed "d" >>> but the guest received "b" and so on. >>> >>> While the stdio driver calls a poll function that just processes on its >>> queue in case virtio-console can't take multiple characters at once, >>> the >>> muxer does not have such callbacks, so it can't empty its queue. >>> >>> To work around that limitation, I introduced a new timer that only gets >>> active when the guest can not receive any more characters. In that case >>> it polls again after a while to check if the guest is now receiving >>> input. >>> >>> This patch fixes input when using -nographic on s390 for me. >>> >> >> I think this is really a kvm issue. I assume it's because s390 idles >> in the kernel so you never drop to userspace to repoll the descriptor. > > There is no polling for the muxer. That's why it never knows when > virtio-console can receive again. Maybe I'm missing something simple, but it looks to me like the muxer is polling. mux_chr_can_read() is going to eventually poll the muxed devices to figure this out. If the root of the problem is that mux_chr_can_read() isn't being invoked for a prolonged period of time, the real issue is the problem I described. Regards, Anthony Liguori > This patch basically adfs timer based polling for that exact case. > > > Alex > >> >> A timer is a hacky solution. You really need to use an io thread to >> solve this and then you need to switch away from qemu_set_fd_handler2 >> to qemu_set_fd_handler() and make sure that the later breaks select >> whenever it's invoked. >> >> Regards, >> >> Anthony Liguori >> >>> --- >>> >>> Please consider for stable. >>> --- >>> qemu-char.c | 10 ++++++++++ >>> 1 files changed, 10 insertions(+), 0 deletions(-) >>> >>> diff --git a/qemu-char.c b/qemu-char.c >>> index 05df971..ce9df3a 100644 >>> --- a/qemu-char.c >>> +++ b/qemu-char.c >>> @@ -235,6 +235,7 @@ typedef struct { >>> IOEventHandler *chr_event[MAX_MUX]; >>> void *ext_opaque[MAX_MUX]; >>> CharDriverState *drv; >>> + QEMUTimer *accept_timer; >>> int focus; >>> int mux_cnt; >>> int term_got_escape; >>> @@ -396,6 +397,13 @@ static void >>> mux_chr_accept_input(CharDriverState *chr) >>> d->chr_read[m](d->ext_opaque[m], >>> &d->buffer[m][d->cons[m]++& MUX_BUFFER_MASK], 1); >>> } >>> + >>> + /* We're still not able to sync producer and consumer, so let's >>> wait a bit >>> + and try again by then. */ >>> + if (d->prod[m] != d->cons[m]) { >>> + qemu_mod_timer(d->accept_timer, qemu_get_clock(vm_clock) >>> + + (int64_t)100000); >>> + } >>> } >>> >>> static int mux_chr_can_read(void *opaque) >>> @@ -478,6 +486,8 @@ static CharDriverState >>> *qemu_chr_open_mux(CharDriverState *drv) >>> chr->opaque = d; >>> d->drv = drv; >>> d->focus = -1; >>> + d->accept_timer = qemu_new_timer(vm_clock, >>> + >>> (QEMUTimerCB*)mux_chr_accept_input, chr); >>> chr->chr_write = mux_chr_write; >>> chr->chr_update_read_handler = mux_chr_update_read_handler; >>> chr->chr_accept_input = mux_chr_accept_input; >>> >>