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

From: Alex Bligh <alex@alex.org.uk>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Alex Bligh <alex@alex.org.uk>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	liu ping fan <qemulist@gmail.com>,
	qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Date: Thu, 01 Aug 2013 15:20:11 +0100	[thread overview]
Message-ID: <D188B444048DFE2521549B34@Ximines.local> (raw)
In-Reply-To: <20130801135111.GL5245@mail.corp.redhat.com>

Paolo,

--On 1 August 2013 15:51:11 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote:

>> > 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.
>>
>> Yes. I believe there will be a similar problem if a timer
>> created or deleted an AioContext (clearly less likely!)
>>
>> > 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.
>>
>> I'm not sure that's true. If two AioContexts run in different
>> threads, would their BH's and timers not also run in those two
>> different threads?
>
> Suppose a thread wants to do qemu_clock_enable(foo, false), and the
> code after qemu_clock_enable assumes that no timers are running.
> Then you should move that code to a bottom half in foo's AioContext.

foo is a QEMUClock here.

A QEMUClock may not have just one AioContext. It could have several
each operated by a different thread.

-- 
Alex Bligh