Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb

[Paolo Bonzini <pbonzini@redhat.com>]

> > True, qemu_event basically works only when a single thread resets it.  But
> > there is no race condition here because qemu_run_timers cannot be executed
> > concurrently by multiple threads (like aio_poll in your bottom half
> > patches).
> 
> ... or, if rebasing on top of my patches, qemu_run_timers *can* be
> executed concurrently by mulitple threads, but in respect of any given
> QEMUTimerList, it will only be executed by one thread.

... so the event would be placed in QEMUTimerList, not QEMUClock.

qemu_clock_enable then will have to visit all timer lists.  That's
not hard to do, but as locks proliferate we need to have a clear
policy (e.g. BQL -> clock -> timerlist).

So actually there is another problem with this patch (both the
condvar and the event approach are equally buggy).  If a timer
on clock X disables clock X, qemu_clock_enable will deadlock.

I suppose it's easier to ask each user of qemu_clock_enable to
provide its own synchronization, and drop this patch.  The simplest
kind of synchronization is to delay some work to a bottom half in
the clock's AioContext.  If you do that, you know that the timers
are not running.

Ping Fan, this teaches once more the same lesson: let's not invent
complex concurrency mechanisms unless really necessary.  So far
they almost never survived (there are some exceptions, but we have
always taken them from somewhere else: QemuEvent is an abstraction
of liburcu code to make it portable, RCU and seqlock are from Linux,
and I cannot think of anything else).

Paolo