[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 4/4] timer: make qemu_clock_enable sync betwe
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH v4 4/4] timer: make qemu_clock_enable sync between disable and timer's cb |
Date: |
Mon, 23 Sep 2013 08:26:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2013-09-22 10:11, 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 <address@hidden>
> ---
> 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.
> */
> 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;
What about "timers_done_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 */
So the caller of qemu_clock_enable has to hold the BQL? Then please add
that to the function description above instead.
Jan
> + 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;
> }
>
>
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
- Re: [Qemu-devel] [PATCH v4 1/4] seqlock: introduce read-write seqlock, (continued)
[Qemu-devel] [PATCH v4 1/4] seqlock: introduce read-write seqlock, Liu Ping Fan, 2013/09/22
[Qemu-devel] [PATCH v4 2/4] timer: protect timers_state's clock with seqlock, Liu Ping Fan, 2013/09/22
[Qemu-devel] [PATCH v4 3/4] qemu-thread: add QemuEvent, Liu Ping Fan, 2013/09/22
[Qemu-devel] [PATCH v4 4/4] timer: make qemu_clock_enable sync between disable and timer's cb, Liu Ping Fan, 2013/09/22
- Re: [Qemu-devel] [PATCH v4 4/4] timer: make qemu_clock_enable sync between disable and timer's cb,
Jan Kiszka <=