qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Date: Thu, 1 Aug 2013 08:19:34 -0400 (EDT)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]