From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiOzs-0002eA-WF for qemu-devel@nongnu.org; Tue, 22 Mar 2016 12:17:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aiOzo-0008CF-0o for qemu-devel@nongnu.org; Tue, 22 Mar 2016 12:17:32 -0400 Received: from ignoranthack.me ([199.102.79.106]:58467 helo=mail.ignoranthack.me) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiOzn-00080j-SC for qemu-devel@nongnu.org; Tue, 22 Mar 2016 12:17:27 -0400 References: <56EEF805.8040008@freebsd.org> <87vb4grya5.fsf@linaro.org> <56F00ABC.9030700@freebsd.org> <87oaa7sv02.fsf@linaro.org> <56F019B9.4080803@redhat.com> From: Sean Bruno Message-ID: <56F16E8B.8030105@freebsd.org> Date: Tue, 22 Mar 2016 09:10:51 -0700 MIME-Version: 1.0 In-Reply-To: <56F019B9.4080803@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [FreeBSD] Host build i386 failing to build aarch64 targets List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: Peter Maydell , QEMU Developers -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 03/21/16 08:56, Paolo Bonzini wrote: >=20 >=20 > On 21/03/2016 16:36, Alex Benn=C3=A9e wrote: >>> 341 /* The icount_warp_timer is rescheduled soon after >>> vm_clock_warp_start 342 * changes from -1 to another >>> value, so the race here is okay. 343 */ 344 if >>> (atomic_read(&vm_clock_warp_start) =3D=3D -1) { 345 >>> return; 346 } 347 >> Odd, the comments say that vm_clock_warp start is protected by >> the seqlock, and in fact every other access to it is a plain >> access. >=20 > Yes, the comment says why this is safe. >=20 > The change from -1 to positive is here: >=20 > if (vm_clock_warp_start =3D=3D -1 || vm_clock_warp_start > clock) {=20 > vm_clock_warp_start =3D clock; }=20 > seqlock_write_unlock(&timers_state.vm_clock_seqlock);=20 > timer_mod_anticipate(icount_warp_timer, clock + deadline); >=20 > If we get a race we must be like this: >=20 > icount_warp_rt qemu_start_warp_timer -------------- > --------------------- read -1 write to vm_clock_warp_start unlock=20 > timer_mod_anticipate (*) >=20 > As soon as you reach (*) the timer is rescheduled and will read a > value other than -1. >=20 >> It seems to me the code should probably just be: >>=20 >> seqlock_write_lock(&timers_state.vm_clock_seqlock); if >> (vm_clock_warp_start !=3D=3D -1 && runstate_is_running()) { .. do >> stuff .. } vm_clock_warp_start =3D -1;=20 >> seqlock_write_unlock(&timers_state.vm_clock_seqlock); >>=20 >> if (we_did_stuff && qemu_clock_expired(QEMU_CLOCK_VIRTUAL)) {=20 >> qemu_clock_notify(QEMU_CLOCK_VIRTUAL); } >=20 > Yes, you can make it like that, or even better wrap the read with > a seqlock_read_begin/seqlock_read_retry loop. The condition will > often be false and it's pointless to lock/unlock the mutex for > that. >=20 > Paolo >=20 >=20 Alex: Do you want me to work up a patch for this or are you dealing with it? sean -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQF8BAEBCgBmBQJW8W6IXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kCPMH/3dbvHDfw5fxY8AX3mBoEsby 10+JEG8u2HojS37h8xx+gxV5ZI9xFUMVQ8niM5EdCrMUz1YeH3oyVYu6LBrlCm9T vlsG0huOXkNuVP+++nAVGr/bPm08IIUBYi1MNVSOZ02MxLgeWblsF4Z9slX5KrH2 FS49z7vDZYeJF2OWxBF/PHvFomNiqfOnsKelh8cWX6FDIubBSrz2dmHvKUu7Jumw EKeZlHP2G+QiuqdUl4t0zvrBYo15IGLUGXNZrp0zgEd2usieA80I1Vb3JQCfrFL1 GFZo7j86W1iXxGTwI//rM52bXAskk8HovtgtkwRbKG18SGpZJzhjqwQAFsDyuUU=3D =3Dp9Xu -----END PGP SIGNATURE-----