-----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-----
