On Tue, Aug 27, 2013 at 11:32 PM, Alex Bligh <a...@alex.org.uk> wrote: > > On 27 Aug 2013, at 04:21, Liu Ping Fan wrote: > >> After disabling the QemuClock, we should make sure that no QemuTimers >> are still in flight. To implement that with light overhead, we resort >> to QemuEvent. The caller of disabling will wait on QemuEvent of each >> timerlist. >> >> Note, qemu_clock_enable(foo,false) can _not_ be called from timer's cb. >> And 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> >> --- >> include/qemu/timer.h | 4 ++++ >> qemu-timer.c | 20 +++++++++++++++++++- >> 2 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/include/qemu/timer.h b/include/qemu/timer.h >> index e4934dd..b26909a 100644 >> --- a/include/qemu/timer.h >> +++ b/include/qemu/timer.h >> @@ -185,6 +185,10 @@ void qemu_clock_notify(QEMUClockType type); >> * @enabled: true to enable, false to disable >> * >> * Enable or disable a clock >> + * Disabling the clock will wait for related timerlists to stop >> + * executing qemu_run_timers. Thus, this functions should not >> + * be used from the callback of a timer that is based on @clock. >> + * Doing so would cause a deadlock. > > Presumably they should also be called with the BQL held? > Yes, will document this.
Thx, Pingfan >> */ >> void qemu_clock_enable(QEMUClockType type, bool enabled); >> >> diff --git a/qemu-timer.c b/qemu-timer.c >> index 95ff47f..c500a76 100644 >> --- a/qemu-timer.c >> +++ b/qemu-timer.c >> @@ -45,6 +45,7 @@ >> /* timers */ >> >> typedef struct QEMUClock { >> + /* We rely on BQL to protect the timerlists */ >> QLIST_HEAD(, QEMUTimerList) timerlists; >> >> NotifierList reset_notifiers; >> @@ -70,6 +71,8 @@ struct QEMUTimerList { >> QLIST_ENTRY(QEMUTimerList) list; >> QEMUTimerListNotifyCB *notify_cb; >> void *notify_opaque; >> + /* light weight method to mark the end of timerlist's running */ >> + QemuEvent ev; >> }; >> >> /** >> @@ -98,6 +101,7 @@ QEMUTimerList *timerlist_new(QEMUClockType type, >> QEMUClock *clock = qemu_clock_ptr(type); >> >> timer_list = g_malloc0(sizeof(QEMUTimerList)); >> + qemu_event_init(&timer_list->ev, false); >> timer_list->clock = clock; >> timer_list->notify_cb = cb; >> timer_list->notify_opaque = opaque; >> @@ -140,13 +144,24 @@ void qemu_clock_notify(QEMUClockType type) >> } >> } >> >> +/* Disabling the clock will wait for related timerlists to stop >> + * executing qemu_run_timers. Thus, this functions should not >> + * be used from the callback of a timer that is based on @clock. >> + * Doing so would cause a deadlock. >> + */ >> void qemu_clock_enable(QEMUClockType type, bool enabled) >> { >> QEMUClock *clock = qemu_clock_ptr(type); >> + QEMUTimerList *tl; >> bool old = clock->enabled; >> clock->enabled = enabled; >> if (enabled && !old) { >> qemu_clock_notify(type); >> + } else if (!enabled && old) { >> + /* We rely on BQL to protect the timerlists */ >> + QLIST_FOREACH(tl, &clock->timerlists, list) { >> + qemu_event_wait(&tl->ev); >> + } >> } >> } >> >> @@ -373,8 +388,10 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) >> QEMUTimer *ts; >> int64_t current_time; >> bool progress = false; >> - >> + >> + qemu_event_reset(&timer_list->ev); >> if (!timer_list->clock->enabled) { >> + qemu_event_set(&timer_list->ev); >> return progress; >> } >> >> @@ -392,6 +409,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) >> ts->cb(ts->opaque); >> progress = true; >> } >> + qemu_event_set(&timer_list->ev); >> return progress; >> } >> >> -- >> 1.8.1.4 >> >> >> > > -- > Alex Bligh > > > >