From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O9Z7F-00036M-AJ for qemu-devel@nongnu.org; Wed, 05 May 2010 03:33:57 -0400 Received: from [140.186.70.92] (port=57685 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O9Z7A-00031f-3y for qemu-devel@nongnu.org; Wed, 05 May 2010 03:33:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O9Z77-00037U-Md for qemu-devel@nongnu.org; Wed, 05 May 2010 03:33:51 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]:43716) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O9Z77-000372-3x for qemu-devel@nongnu.org; Wed, 05 May 2010 03:33:49 -0400 Message-ID: <4BE11F54.1030900@web.de> Date: Wed, 05 May 2010 09:33:40 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1271782566-16420-1-git-send-email-agraf@suse.de> <4BE024A9.2040904@codemonkey.ws> <16E7906F-F39B-4121-B540-E2F0E67D405E@suse.de> <4BE0307C.1010305@codemonkey.ws> <0D8C0E80-5325-4E50-A2EF-E73BB95AD8DA@suse.de> <4BE04A78.6070505@codemonkey.ws> <4BE05034.9060803@siemens.com> In-Reply-To: <4BE05034.9060803@siemens.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigF46A169A476F2EDA158B4A0D" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH] [RESEND] Make char muxer more robust wrt small FIFOs List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori , Alexander Graf Cc: Bastian Blank , qemu-devel Developers , Aurelien Jarno This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigF46A169A476F2EDA158B4A0D Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Jan Kiszka wrote: > Anthony Liguori wrote: >> On 05/04/2010 11:01 AM, Alexander Graf wrote: >>> Am 04.05.2010 um 16:34 schrieb Anthony Liguori : >>> >>>> 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= =20 >>>>>>> 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=20 >>>>>>> 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=20 >>>>>>> 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 onl= y=20 >>>>>>> gets >>>>>>> active when the guest can not receive any more characters. In tha= t=20 >>>>>>> case >>>>>>> it polls again after a while to check if the guest is now=20 >>>>>>> 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=20 >>>>>> idles in the kernel so you never drop to userspace to repoll the=20 >>>>>> descriptor. >>>>> There is no polling for the muxer. That's why it never knows when=20 >>>>> virtio-console can receive again. >>>> Maybe I'm missing something simple, but it looks to me like the muxe= r=20 >>>> is polling. mux_chr_can_read() is going to eventually poll the muxe= d=20 >>>> devices to figure this out. >>>> >>>> If the root of the problem is that mux_chr_can_read() isn't being=20 >>>> invoked for a prolonged period of time, the real issue is the proble= m=20 >>>> I described. >>> The problem is that the select list of fds includes the stdio fd, so = >>> that gets notified and is coupled with virtio-console, but there's=20 >>> nothing passing that on to mux and I don't think it'd be clever to=20 >>> expose internal data to the muxer to tell it about the backend fds. >> When stdio is readable, it should invoke qemu_chr_read() with the read= =20 >> data which in turn ought to invoke mux_chr_read(). >> >> I'm not sure I understand what signalling is missing. Jan, does the=20 >> problem Alex describes ring a bell? I seem to recall you saying that = >> mux was still fundamentally broken but ought to work most of the time.= =2E. >=20 > That problem was (and still is) that the muxer needs to accept > characters even if the active front-end device is not in order to filte= r > out control sequences. Once its queue is full, it will start dropping > those the active device would not if directly connected. Could only be > solved via some peek service on pending front-end data. >=20 > I think Alex' problem can be addressed by registering > qemu_set_fd_handler2(..., backend->read_poll, mux_chr_read, ...). That > means the backend has to tell us about its read poll handler (if any). Nonsense. In fact, the problem is the former issue: As the muxer reads the character the front-end is currently unable to receive, polling may stop until as the back-end has some further chars to deliver. But interestingly, the stdio back-end has a (single-byte) fifo as well. It just drives it a bit differently. Alex, does this help as well? diff --git a/qemu-char.c b/qemu-char.c index ac65a1c..2b115a4 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -404,6 +404,8 @@ static int mux_chr_can_read(void *opaque) MuxDriver *d =3D chr->opaque; int m =3D d->focus; =20 + mux_chr_accept_input(opaque); + if ((d->prod[m] - d->cons[m]) < MUX_BUFFER_SIZE) return 1; if (d->chr_can_read[m]) @@ -418,8 +420,6 @@ static void mux_chr_read(void *opaque, const uint8_t = *buf, int size) int m =3D d->focus; int i; =20 - mux_chr_accept_input (opaque); - for(i =3D 0; i < size; i++) if (mux_proc_byte(chr, d, buf[i])) { if (d->prod[m] =3D=3D d->cons[m] && I'm trying to reproduce in parallel. Jan --------------enigF46A169A476F2EDA158B4A0D Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkvhH1gACgkQitSsb3rl5xTUTACfYq3LA2gGIJic5YJzcOMF9UgR oZMAoJBkONp2/JRW7TYdBV45tfiY45LJ =9u+e -----END PGP SIGNATURE----- --------------enigF46A169A476F2EDA158B4A0D--