On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini <pbonz...@redhat.com> 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 <pingf...@linux.vnet.ibm.com> > > 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) >> >