From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LywCt-00044t-V9 for qemu-devel@nongnu.org; Tue, 28 Apr 2009 18:55:20 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LywCo-0003vz-DQ for qemu-devel@nongnu.org; Tue, 28 Apr 2009 18:55:18 -0400 Received: from [199.232.76.173] (port=60917 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LywCo-0003vU-69 for qemu-devel@nongnu.org; Tue, 28 Apr 2009 18:55:14 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:51406) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LywCn-0005AI-Dn for qemu-devel@nongnu.org; Tue, 28 Apr 2009 18:55:13 -0400 Message-ID: <49F7894B.7080103@web.de> Date: Wed, 29 Apr 2009 00:55:07 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <49F7848F.7020708@web.de> <20090428224149.GA15512@amt.cnet> In-Reply-To: <20090428224149.GA15512@amt.cnet> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig30DD54BD290E2E681DBCF2BB" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [7241] qemu: refactor main_loop (Marcelo Tosatti) List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcelo Tosatti Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig30DD54BD290E2E681DBCF2BB Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Marcelo Tosatti wrote: > On Wed, Apr 29, 2009 at 12:34:55AM +0200, Jan Kiszka wrote: >> Anthony Liguori wrote: >>> Revision: 7241 >>> http://svn.sv.gnu.org/viewvc/?view=3Drev&root=3Dqemu&revisi= on=3D7241 >>> Author: aliguori >>> Date: 2009-04-24 18:03:33 +0000 (Fri, 24 Apr 2009) >>> Log Message: >>> ----------- >>> qemu: refactor main_loop (Marcelo Tosatti) >>> >>> Break main loop into 3 main functions. >> I suspect this patch comes with a race between SIGALRM and >> qemu_calculate_timeout() so that I see occasional freezes of exactly 5= >> seconds if the IO thread is disabled. >=20 > host_alarm_handler writes to the notification fd (via > qemu_event_increment), which should cause the select() to exit > immediately, even if a pending timer was not taken into account by > qemu_calculate_timeout(). Yeah, now I remember. And I always wondered why my strace logs reported that writing to that file descriptor failed. I should have looked closer into this immediately... (patch will follow 8) ) >=20 > But 5 seconds is a good clue :) >=20 >> I do not yet understand what >> happens precisely or if this patch only widens an already existing rac= e >> window in the old code, but I'm on it. >> >> Besides that... >> >>> Signed-off-by: Marcelo Tosatti >>> Signed-off-by: Anthony Liguori >>> >>> Modified Paths: >>> -------------- >>> trunk/vl.c >>> >>> Modified: trunk/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 >>> --- trunk/vl.c 2009-04-24 18:03:29 UTC (rev 7240) >>> +++ trunk/vl.c 2009-04-24 18:03:33 UTC (rev 7241) >>> @@ -273,7 +273,7 @@ >>> =20 >>> static CPUState *cur_cpu; >>> static CPUState *next_cpu; >>> -static int event_pending =3D 1; >>> +static int timer_alarm_pending =3D 1; >>> /* Conversion factor from emulated instructions to virtual clock tic= ks. */ >>> static int icount_time_shift; >>> /* Arbitrarily pick 1MIPS as the minimum allowable speed. */ >>> @@ -1360,7 +1360,7 @@ >>> } >>> #endif >>> } >>> - event_pending =3D 1; >>> + timer_alarm_pending =3D 1; >>> qemu_notify_event(); >>> } >>> } >>> @@ -3879,153 +3879,175 @@ >>> =20 >>> } >>> =20 >>> -static int main_loop(void) >>> +static int qemu_cpu_exec(CPUState *env) >>> { >>> - int ret, timeout; >>> + int ret; >>> #ifdef CONFIG_PROFILER >>> int64_t ti; >>> #endif >>> + >>> +#ifdef CONFIG_PROFILER >>> + ti =3D profile_getclock(); >>> +#endif >>> + if (use_icount) { >>> + int64_t count; >>> + int decr; >>> + qemu_icount -=3D (env->icount_decr.u16.low + env->icount_ext= ra); >>> + env->icount_decr.u16.low =3D 0; >>> + env->icount_extra =3D 0; >>> + count =3D qemu_next_deadline(); >>> + count =3D (count + (1 << icount_time_shift) - 1) >>> + >> icount_time_shift; >>> + qemu_icount +=3D count; >>> + decr =3D (count > 0xffff) ? 0xffff : count; >>> + count -=3D decr; >>> + env->icount_decr.u16.low =3D decr; >>> + env->icount_extra =3D count; >>> + } >>> + ret =3D cpu_exec(env); >>> +#ifdef CONFIG_PROFILER >>> + qemu_time +=3D profile_getclock() - ti; >>> +#endif >>> + if (use_icount) { >>> + /* Fold pending instructions back into the >>> + instruction counter, and clear the interrupt flag. */ >>> + qemu_icount -=3D (env->icount_decr.u16.low >>> + + env->icount_extra); >>> + env->icount_decr.u32 =3D 0; >>> + env->icount_extra =3D 0; >>> + } >>> + return ret; >>> +} >>> + >>> +static int cpu_has_work(CPUState *env) >> ...this naming is suboptimal. There is already cpu_has_work() in >> target-*/exec.h which is at least confusing. Please rename. >=20 > Well its static. What name do you prefer (can't find a better name > really). do_cpu_exec?=20 >=20 Yes, it works, but it doesn't help reading the code. Also: tcg_has_work - isn't kvm also running through this? Jan --------------enig30DD54BD290E2E681DBCF2BB 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 iEYEARECAAYFAkn3iU8ACgkQniDOoMHTA+lVtwCfSaZwJCoQkntgadnCFPXxeuXR bEkAn2z5Pkf2/ngbQyvzOIYTUDJSHLS+ =t2/F -----END PGP SIGNATURE----- --------------enig30DD54BD290E2E681DBCF2BB--