From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KxPVB-00082d-4B for qemu-devel@nongnu.org; Tue, 04 Nov 2008 12:15:37 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KxPV8-00081f-Vi for qemu-devel@nongnu.org; Tue, 04 Nov 2008 12:15:36 -0500 Received: from [199.232.76.173] (port=44090 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KxPV8-00081b-QR for qemu-devel@nongnu.org; Tue, 04 Nov 2008 12:15:34 -0500 Received: from fmmailgate02.web.de ([217.72.192.227]:47924) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KxPV8-0001Sw-HY for qemu-devel@nongnu.org; Tue, 04 Nov 2008 12:15:35 -0500 Received: from smtp08.web.de (fmsmtp08.dlan.cinetic.de [172.20.5.216]) by fmmailgate02.web.de (Postfix) with ESMTP id 1A5ADF5FF59B for ; Tue, 4 Nov 2008 18:15:33 +0100 (CET) Message-ID: <49108333.6080807@web.de> Date: Tue, 04 Nov 2008 18:15:31 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <490FFF23.5020704@web.de> <49105704.9070708@codemonkey.ws> <49105BE7.6030505@codemonkey.ws> In-Reply-To: <49105BE7.6030505@codemonkey.ws> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig2AB4375478CE16D8046495E3" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH] Fix alarm_timer race with select Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Jan Kiszka This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig2AB4375478CE16D8046495E3 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Anthony Liguori wrote: > Anthony Liguori wrote: >> Jan Kiszka wrote: >> >> Perhaps we should move the alarm timer check rearming out of the main >> loop and into a qemu_set_fd_handler2() handler? >=20 > And now that I think about it, I see no reason why the timer expiration= > checks couldn't be moved to the same handler. I'd have to look more > closely at how that would interact with icount though. I cannot yet follow you here. But I my impression is that this will be an additional change that should come in a separate patch, no? Find the O_NONBLOCK remark addressed below. Jan ------------> Changing the default IO timeout to 5 s (#5578) made a race visible between the alarm_timer and select() in main_loop_wait(): If the timer fired before select was able to block, the full select() timeout could have been applied instead of returning immediately. Since #5578, this causes heavy problems to the Musicpal board emulation with stalls up to 5 s. The following patch introduces a pipe that is written to by host_alarm_handler and select()'ed in main_loop_wait(). This avoids prevents that select() blocks though a timer has fired and waits for processing. Signed-off-by: Jan Kiszka --- vl.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) Index: b/vl.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- a/vl.c +++ b/vl.c @@ -884,6 +884,7 @@ static void qemu_rearm_alarm_timer(struc #define MIN_TIMER_REARM_US 250 =20 static struct qemu_alarm_timer *alarm_timer; +static int alarm_timer_rfd, alarm_timer_wfd; =20 #ifdef _WIN32 =20 @@ -1303,12 +1304,15 @@ static void host_alarm_handler(int host_ qemu_get_clock(vm_clock))) || qemu_timer_expired(active_timers[QEMU_TIMER_REALTIME], qemu_get_clock(rt_clock))) { + CPUState *env =3D next_cpu; + char byte =3D 0; + #ifdef _WIN32 struct qemu_alarm_win32 *data =3D ((struct qemu_alarm_timer*)dwU= ser)->priv; SetEvent(data->host_alarm); #endif - CPUState *env =3D next_cpu; =20 + write(alarm_timer_wfd, &byte, sizeof(byte)); alarm_timer->flags |=3D ALARM_FLAG_EXPIRED; =20 if (env) { @@ -1673,6 +1677,15 @@ static void init_timer_alarm(void) { struct qemu_alarm_timer *t =3D NULL; int i, err =3D -1; + int fds[2]; + + if (pipe(fds) || fcntl(fds[0], F_SETFL, O_NONBLOCK) + || fcntl(fds[1], F_SETFL, O_NONBLOCK)) { + perror("creating timer pipe"); + exit(1); + } + alarm_timer_rfd =3D fds[0]; + alarm_timer_wfd =3D fds[1]; =20 for (i =3D 0; alarm_timers[i].name; i++) { t =3D &alarm_timers[i]; @@ -4427,6 +4440,7 @@ void main_loop_wait(int timeout) /* XXX: separate device handlers from system ones */ nfds =3D -1; FD_ZERO(&rfds); + FD_SET(alarm_timer_rfd, &rfds); FD_ZERO(&wfds); FD_ZERO(&xfds); for(ioh =3D first_io_handler; ioh !=3D NULL; ioh =3D ioh->next) { @@ -4500,6 +4514,11 @@ void main_loop_wait(int timeout) qemu_get_clock(rt_clock)); =20 if (alarm_timer->flags & ALARM_FLAG_EXPIRED) { + char byte; + do { + ret =3D read(alarm_timer_rfd, &byte, sizeof(byte)); + } while (ret !=3D -1 || errno !=3D EAGAIN); + alarm_timer->flags &=3D ~(ALARM_FLAG_EXPIRED); qemu_rearm_alarm_timer(alarm_timer); } --------------enig2AB4375478CE16D8046495E3 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 iEYEARECAAYFAkkQgzMACgkQniDOoMHTA+kftQCZAduAhMLGe0vg5yseGMy1uTEs 8r0AnA5xez9EmNBeRP1ZIZd5pPuYBbNT =ya0K -----END PGP SIGNATURE----- --------------enig2AB4375478CE16D8046495E3--