On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini <[email protected]> wrote:
> Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
>> After disabling the QemuClock, we should make sure that no QemuTimers
>> are still in flight. To implement that, the caller of disabling will
>> wait until the last user's exit.
>>
>> Note, the callers of qemu_clock_enable() should be sync by themselves,
>> not protected by this patch.
>>
>> Signed-off-by: Liu Ping Fan <[email protected]>
>
> This is an interesting approach.
>
>> - if (!clock->enabled)
>> - return;
>> + atomic_inc(&clock->using);
>> + if (unlikely(!clock->enabled)) {
>> + goto exit;
>> + }
>
> This can return directly, it doesn't need to increment and decrement
> clock->using.
>
Here is a race condition like the following, so I swap the sequence
qemu_clock_enable(false) run timers
if(!clock->enabled) return;
------------>
clock->enabled=false
if(atomic_read(using)==0)
atomic_inc(using)
Regards,
Pingfan
> Paolo
>
>>
>> current_time = qemu_get_clock_ns(clock);
>> tlist = clock_to_timerlist(clock);
>> @@ -461,6 +482,15 @@ void qemu_run_timers(QEMUClock *clock)
>> /* run the callback (the timer list can be modified) */
>> ts->cb(ts->opaque);
>> }
>> +
>> +exit:
>> + qemu_mutex_lock(&clock->lock);
>> + if (atomic_fetch_dec(&clock->using) == 1) {
>> + if (unlikely(!clock->enabled)) {
>> + qemu_cond_signal(&clock->wait_using);
>> + }
>> + }
>> + qemu_mutex_unlock(&clock->lock);
>> }
>>
>> int64_t qemu_get_clock_ns(QEMUClock *clock)
>>
>
