From 978f2205c791de0e02c8802a645bea657408abfd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 12 Sep 2013 11:02:19 +0200 Subject: qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe Introduce QEMUTimerList->active_timers_lock to protect the linked list of active timers. This allows qemu_timer_mod_ns() to be called from any thread. Note that vm_clock is not thread-safe and its use of qemu_clock_has_timers() works fine today but is also not thread-safe. The purpose of this patch is to eventually let device models set or cancel timers from a vcpu thread without holding the global mutex. Signed-off-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi --- include/qemu/timer.h | 17 ++++++++++ qemu-timer.c | 87 ++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 85 insertions(+), 19 deletions(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index e4934dd61b..b58903bef5 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -115,6 +115,10 @@ static inline int64_t qemu_clock_get_us(QEMUClockType type) * Determines whether a clock's default timer list * has timers attached * + * Note that this function should not be used when other threads also access + * the timer list. The return value may be outdated by the time it is acted + * upon. + * * Returns: true if the clock's default timer list * has timers attached */ @@ -271,6 +275,10 @@ void timerlist_free(QEMUTimerList *timer_list); * * Determine whether a timer list has active timers * + * Note that this function should not be used when other threads also access + * the timer list. The return value may be outdated by the time it is acted + * upon. + * * Returns: true if the timer list has timers. */ bool timerlist_has_timers(QEMUTimerList *timer_list); @@ -512,6 +520,9 @@ void timer_free(QEMUTimer *ts); * @ts: the timer * * Delete a timer from the active list. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_del(QEMUTimer *ts); @@ -521,6 +532,9 @@ void timer_del(QEMUTimer *ts); * @expire_time: the expiry time in nanoseconds * * Modify a timer to expire at @expire_time + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); @@ -531,6 +545,9 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); * * Modify a timer to expiry at @expire_time, taking into * account the scale associated with the timer. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_mod(QEMUTimer *ts, int64_t expire_timer); diff --git a/qemu-timer.c b/qemu-timer.c index ed3fcb2190..e5047479f2 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -66,6 +66,7 @@ QEMUClock qemu_clocks[QEMU_CLOCK_MAX]; struct QEMUTimerList { QEMUClock *clock; + QemuMutex active_timers_lock; QEMUTimer *active_timers; QLIST_ENTRY(QEMUTimerList) list; QEMUTimerListNotifyCB *notify_cb; @@ -101,6 +102,7 @@ QEMUTimerList *timerlist_new(QEMUClockType type, timer_list->clock = clock; timer_list->notify_cb = cb; timer_list->notify_opaque = opaque; + qemu_mutex_init(&timer_list->active_timers_lock); QLIST_INSERT_HEAD(&clock->timerlists, timer_list, list); return timer_list; } @@ -111,6 +113,7 @@ void timerlist_free(QEMUTimerList *timer_list) if (timer_list->clock) { QLIST_REMOVE(timer_list, list); } + qemu_mutex_destroy(&timer_list->active_timers_lock); g_free(timer_list); } @@ -163,9 +166,17 @@ bool qemu_clock_has_timers(QEMUClockType type) bool timerlist_expired(QEMUTimerList *timer_list) { - return (timer_list->active_timers && - timer_list->active_timers->expire_time < - qemu_clock_get_ns(timer_list->clock->type)); + int64_t expire_time; + + qemu_mutex_lock(&timer_list->active_timers_lock); + if (!timer_list->active_timers) { + qemu_mutex_unlock(&timer_list->active_timers_lock); + return false; + } + expire_time = timer_list->active_timers->expire_time; + qemu_mutex_unlock(&timer_list->active_timers_lock); + + return expire_time < qemu_clock_get_ns(timer_list->clock->type); } bool qemu_clock_expired(QEMUClockType type) @@ -182,13 +193,25 @@ bool qemu_clock_expired(QEMUClockType type) int64_t timerlist_deadline_ns(QEMUTimerList *timer_list) { int64_t delta; + int64_t expire_time; - if (!timer_list->clock->enabled || !timer_list->active_timers) { + if (!timer_list->clock->enabled) { return -1; } - delta = timer_list->active_timers->expire_time - - qemu_clock_get_ns(timer_list->clock->type); + /* The active timers list may be modified before the caller uses our return + * value but ->notify_cb() is called when the deadline changes. Therefore + * the caller should notice the change and there is no race condition. + */ + qemu_mutex_lock(&timer_list->active_timers_lock); + if (!timer_list->active_timers) { + qemu_mutex_unlock(&timer_list->active_timers_lock); + return -1; + } + expire_time = timer_list->active_timers->expire_time; + qemu_mutex_unlock(&timer_list->active_timers_lock); + + delta = expire_time - qemu_clock_get_ns(timer_list->clock->type); if (delta <= 0) { return 0; @@ -296,12 +319,11 @@ void timer_free(QEMUTimer *ts) g_free(ts); } -/* stop a timer, but do not dealloc it */ -void timer_del(QEMUTimer *ts) +static void timer_del_locked(QEMUTimerList *timer_list, QEMUTimer *ts) { QEMUTimer **pt, *t; - pt = &ts->timer_list->active_timers; + pt = &timer_list->active_timers; for(;;) { t = *pt; if (!t) @@ -314,16 +336,28 @@ void timer_del(QEMUTimer *ts) } } +/* stop a timer, but do not dealloc it */ +void timer_del(QEMUTimer *ts) +{ + QEMUTimerList *timer_list = ts->timer_list; + + qemu_mutex_lock(&timer_list->active_timers_lock); + timer_del_locked(timer_list, ts); + qemu_mutex_unlock(&timer_list->active_timers_lock); +} + /* modify the current timer so that it will be fired when current_time >= expire_time. The corresponding callback will be called. */ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) { + QEMUTimerList *timer_list = ts->timer_list; QEMUTimer **pt, *t; - timer_del(ts); + qemu_mutex_lock(&timer_list->active_timers_lock); + timer_del_locked(timer_list, ts); /* add the timer in the sorted list */ - pt = &ts->timer_list->active_timers; + pt = &timer_list->active_timers; for(;;) { t = *pt; if (!timer_expired_ns(t, expire_time)) { @@ -334,12 +368,13 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) ts->expire_time = expire_time; ts->next = *pt; *pt = ts; + qemu_mutex_unlock(&timer_list->active_timers_lock); /* Rearm if necessary */ - if (pt == &ts->timer_list->active_timers) { + if (pt == &timer_list->active_timers) { /* Interrupt execution to force deadline recalculation. */ - qemu_clock_warp(ts->timer_list->clock->type); - timerlist_notify(ts->timer_list); + qemu_clock_warp(timer_list->clock->type); + timerlist_notify(timer_list); } } @@ -350,13 +385,19 @@ void timer_mod(QEMUTimer *ts, int64_t expire_time) bool timer_pending(QEMUTimer *ts) { + QEMUTimerList *timer_list = ts->timer_list; QEMUTimer *t; - for (t = ts->timer_list->active_timers; t != NULL; t = t->next) { + bool found = false; + + qemu_mutex_lock(&timer_list->active_timers_lock); + for (t = timer_list->active_timers; t != NULL; t = t->next) { if (t == ts) { - return true; + found = true; + break; } } - return false; + qemu_mutex_unlock(&timer_list->active_timers_lock); + return found; } bool timer_expired(QEMUTimer *timer_head, int64_t current_time) @@ -369,23 +410,31 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) QEMUTimer *ts; int64_t current_time; bool progress = false; - + QEMUTimerCB *cb; + void *opaque; + if (!timer_list->clock->enabled) { return progress; } current_time = qemu_clock_get_ns(timer_list->clock->type); for(;;) { + qemu_mutex_lock(&timer_list->active_timers_lock); ts = timer_list->active_timers; if (!timer_expired_ns(ts, current_time)) { + qemu_mutex_unlock(&timer_list->active_timers_lock); break; } + /* remove timer from the list before calling the callback */ timer_list->active_timers = ts->next; ts->next = NULL; + cb = ts->cb; + opaque = ts->opaque; + qemu_mutex_unlock(&timer_list->active_timers_lock); /* run the callback (the timer list can be modified) */ - ts->cb(ts->opaque); + cb(opaque); progress = true; } return progress; -- cgit v1.2.1