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

2013-08-04 Thread liu ping fan
On Fri, Aug 2, 2013 at 10:43 PM, Stefan Hajnoczi wrote: > On Fri, Aug 02, 2013 at 11:33:40AM +0800, liu ping fan wrote: >> On Thu, Aug 1, 2013 at 9:28 PM, Alex Bligh wrote: >> > Paolo, >> > >> > >> > --On 1 August 2013 08:19:34 -0400 Paolo Bonzini >> > wrote: >> > >> >>> > True, qemu_event basi

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

2013-08-02 Thread Stefan Hajnoczi
On Fri, Aug 02, 2013 at 11:33:40AM +0800, liu ping fan wrote: > On Thu, Aug 1, 2013 at 9:28 PM, Alex Bligh wrote: > > Paolo, > > > > > > --On 1 August 2013 08:19:34 -0400 Paolo Bonzini wrote: > > > >>> > True, qemu_event basically works only when a single thread resets it. > >>> > But there is no

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

2013-08-02 Thread Paolo Bonzini
On Aug 02 2013, liu ping fan wrote: > On Thu, Aug 1, 2013 at 10:28 PM, Paolo Bonzini 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_ena

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

2013-08-01 Thread liu ping fan
On Thu, Aug 1, 2013 at 9:28 PM, Alex Bligh wrote: > Paolo, > > > --On 1 August 2013 08:19:34 -0400 Paolo Bonzini wrote: > >>> > 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 concurre

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

2013-08-01 Thread liu ping fan
On Thu, Aug 1, 2013 at 10:28 PM, Paolo Bonzini wrote: > On Aug 01 2013, Alex Bligh wrote: >> Paolo, >> >> --On 1 August 2013 15:51:11 +0200 Paolo Bonzini wrote: >> >> >>> So actually there is another problem with this patch (both the >> >>> condvar and the event approach are equally buggy). If

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

2013-08-01 Thread Paolo Bonzini
On Aug 01 2013, Alex Bligh wrote: > Paolo, > > --On 1 August 2013 15:51:11 +0200 Paolo Bonzini 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

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

2013-08-01 Thread Alex Bligh
Paolo, --On 1 August 2013 15:51:11 +0200 Paolo Bonzini 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 simila

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

2013-08-01 Thread Paolo Bonzini
On Aug 01 2013, Alex Bligh 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 > cre

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

2013-08-01 Thread Alex Bligh
Paolo, --On 1 August 2013 08:19:34 -0400 Paolo Bonzini wrote: > 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

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

2013-08-01 Thread Paolo Bonzini
> > 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

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

2013-08-01 Thread Alex Bligh
--On 1 August 2013 04:57:52 -0400 Paolo Bonzini wrote: 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,

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

2013-08-01 Thread Paolo Bonzini
> > Hmm, do we even need clock->using at this point? For example: > > > >qemu_clock_enable() > >{ > >clock->enabled = enabled; > >... > >if (!enabled) { > >/* If another thread is within qemu_run_timers, > > * wait for it to finish. > >

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

2013-07-31 Thread liu ping fan
On Tue, Jul 30, 2013 at 5:17 PM, Paolo Bonzini wrote: > Il 30/07/2013 04:42, liu ping fan ha scritto: >> On Mon, Jul 29, 2013 at 7:21 PM, Paolo Bonzini wrote: >>> Il 29/07/2013 10:10, liu ping fan ha scritto: On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini wrote: > Il 29/07/2013 05:16, L

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

2013-07-30 Thread Paolo Bonzini
Il 30/07/2013 11:51, Alex Bligh ha scritto: > As far as walking the QEMUTimerList itself is concerned, this is > something which is 99.999% done by the thread owning the AioContext. > qemu_clock_enable should not even be walking this list. So I don't > see why the protection here is needed. The pr

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

2013-07-30 Thread Alex Bligh
Paolo, --On 30 July 2013 11:17:05 +0200 Paolo Bonzini wrote: Hmm, do we even need clock->using at this point? For example: qemu_clock_enable() { clock->enabled = enabled; ... if (!enabled) { /* If another thread is within qemu_run_timers, * w

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

2013-07-30 Thread Paolo Bonzini
Il 30/07/2013 04:42, liu ping fan ha scritto: > On Mon, Jul 29, 2013 at 7:21 PM, Paolo Bonzini wrote: >> Il 29/07/2013 10:10, liu ping fan ha scritto: >>> On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini wrote: Il 29/07/2013 05:16, Liu Ping Fan ha scritto: > After disabling the QemuClock,

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

2013-07-29 Thread liu ping fan
On Mon, Jul 29, 2013 at 7:21 PM, Paolo Bonzini wrote: > Il 29/07/2013 10:10, liu ping fan ha scritto: >> On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini wrote: >>> Il 29/07/2013 05:16, Liu Ping Fan ha scritto: After disabling the QemuClock, we should make sure that no QemuTimers are stil

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

2013-07-29 Thread Paolo Bonzini
Il 29/07/2013 10:10, liu ping fan ha scritto: > On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini 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

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

2013-07-29 Thread liu ping fan
On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini 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

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

2013-07-28 Thread Paolo Bonzini
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,

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

2013-07-28 Thread Liu Ping Fan
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 F