Re: [Qemu-devel] [FreeBSD] Host build i386 failing to build aarch64 targets

[Sean Bruno <sbruno@freebsd.org>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512



On 03/21/16 08:56, Paolo Bonzini wrote:
> 
> 
> On 21/03/2016 16:36, Alex Bennée 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) == -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.
> 
> Yes, the comment says why this is safe.
> 
> The change from -1 to positive is here:
> 
> if (vm_clock_warp_start == -1 || vm_clock_warp_start > clock) { 
> vm_clock_warp_start = clock; } 
> seqlock_write_unlock(&timers_state.vm_clock_seqlock); 
> timer_mod_anticipate(icount_warp_timer, clock + deadline);
> 
> If we get a race we must be like this:
> 
> icount_warp_rt           qemu_start_warp_timer --------------
> --------------------- read -1 write to vm_clock_warp_start unlock 
> timer_mod_anticipate (*)
> 
> As soon as you reach (*) the timer is rescheduled and will read a
> value other than -1.
> 
>> It seems to me the code should probably just be:
>> 
>> seqlock_write_lock(&timers_state.vm_clock_seqlock); if
>> (vm_clock_warp_start !== -1 && runstate_is_running()) { .. do
>> stuff .. } vm_clock_warp_start = -1; 
>> seqlock_write_unlock(&timers_state.vm_clock_seqlock);
>> 
>> if (we_did_stuff && qemu_clock_expired(QEMU_CLOCK_VIRTUAL)) { 
>> qemu_clock_notify(QEMU_CLOCK_VIRTUAL); }
> 
> 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.
> 
> Paolo
> 
> 


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=
=p9Xu
-----END PGP SIGNATURE-----