active_timers is a list of timer lists, associated with a Qemu Clock, that is read-mostly. This patch converts read accesses to the list to use RCU, which should hopefully mitigate most of the synchronization overhead. Write accesses are now via a mutex in the parent data structure. Functions that change the change the parent data structure are also protected by the mutex.
This patch applies against Paolo Bonzini's rcu branch: https://github.com/bonzini/qemu/tree/rcu V3: - Address comments from Alex Bligh and Paolo Bonzini - Move the mutex into the parent QemuClock structure Signed-off-by: Mike Day <ncm...@ncultra.org> --- include/qemu/timer.h | 19 +++++---- qemu-timer.c | 111 +++++++++++++++++++++++++++++---------------------- 2 files changed, 72 insertions(+), 58 deletions(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 5afcffc..f69ec49 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -5,8 +5,15 @@ #include "qemu-common.h" #include "qemu/notify.h" -/* timers */ +#ifdef __GNUC__ +#ifndef __ATOMIC_RELEASE +#define __ATOMIC_RELEASE +#endif +#endif +#include "qemu/atomic.h" +#include "qemu/rcu.h" +/* timers */ #define SCALE_MS 1000000 #define SCALE_US 1000 #define SCALE_NS 1 @@ -61,6 +68,7 @@ struct QEMUTimer { void *opaque; QEMUTimer *next; int scale; + struct rcu_head rcu; }; extern QEMUTimerListGroup main_loop_tlg; @@ -189,12 +197,6 @@ 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. - * - * Caller should hold BQL. */ void qemu_clock_enable(QEMUClockType type, bool enabled); @@ -543,7 +545,6 @@ void timer_del(QEMUTimer *ts); * freed while this function is running. */ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); - /** * timer_mod_anticipate_ns: * @ts: the timer @@ -685,9 +686,7 @@ static inline int64_t qemu_soonest_timeout(int64_t timeout1, int64_t timeout2) void init_clocks(void); int64_t cpu_get_ticks(void); -/* Caller must hold BQL */ void cpu_enable_ticks(void); -/* Caller must hold BQL */ void cpu_disable_ticks(void); static inline int64_t get_ticks_per_sec(void) diff --git a/qemu-timer.c b/qemu-timer.c index fb9e680..57a1545 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -29,6 +29,7 @@ #include "hw/hw.h" #include "qemu/timer.h" +#include "qemu/rcu_queue.h" #ifdef CONFIG_POSIX #include <pthread.h> #endif @@ -45,12 +46,10 @@ /* timers */ typedef struct QEMUClock { - /* We rely on BQL to protect the timerlists */ QLIST_HEAD(, QEMUTimerList) timerlists; - + QemuMutex timer_lock; NotifierList reset_notifiers; int64_t last; - QEMUClockType type; bool enabled; @@ -70,11 +69,11 @@ QEMUClock qemu_clocks[QEMU_CLOCK_MAX]; struct QEMUTimerList { QEMUClock *clock; - QemuMutex active_timers_lock; QEMUTimer *active_timers; QLIST_ENTRY(QEMUTimerList) list; QEMUTimerListNotifyCB *notify_cb; void *notify_opaque; + QemuEvent timers_done_ev; }; /** @@ -87,6 +86,7 @@ struct QEMUTimerList { */ static inline QEMUClock *qemu_clock_ptr(QEMUClockType type) { + smp_rmb(); return &qemu_clocks[type]; } @@ -107,7 +107,6 @@ 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; } @@ -118,7 +117,7 @@ void timerlist_free(QEMUTimerList *timer_list) if (timer_list->clock) { QLIST_REMOVE(timer_list, list); } - qemu_mutex_destroy(&timer_list->active_timers_lock); + qemu_event_destroy(&timer_list->timers_done_ev); g_free(timer_list); } @@ -129,6 +128,7 @@ static void qemu_clock_init(QEMUClockType type) clock->type = type; clock->enabled = true; clock->last = INT64_MIN; + qemu_mutex_init(&clock->timer_lock); QLIST_INIT(&clock->timerlists); notifier_list_init(&clock->reset_notifiers); main_loop_tlg.tl[type] = timerlist_new(type, NULL, NULL); @@ -148,13 +148,6 @@ 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. - * - * Caller should hold BQL. - */ void qemu_clock_enable(QEMUClockType type, bool enabled) { QEMUClock *clock = qemu_clock_ptr(type); @@ -172,7 +165,7 @@ void qemu_clock_enable(QEMUClockType type, bool enabled) bool timerlist_has_timers(QEMUTimerList *timer_list) { - return !!timer_list->active_timers; + return !!atomic_rcu_read(&timer_list->active_timers); } bool qemu_clock_has_timers(QEMUClockType type) @@ -184,16 +177,18 @@ bool qemu_clock_has_timers(QEMUClockType type) bool timerlist_expired(QEMUTimerList *timer_list) { int64_t expire_time; + bool ret; - qemu_mutex_lock(&timer_list->active_timers_lock); - if (!timer_list->active_timers) { - qemu_mutex_unlock(&timer_list->active_timers_lock); + rcu_read_lock(); + if (!atomic_rcu_read(&timer_list->active_timers)) { + rcu_read_unlock(); return false; } + int type = timer_list->clock->type; 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); + rcu_read_unlock(); + ret = (expire_time < qemu_clock_get_ns(type)); + return ret; } bool qemu_clock_expired(QEMUClockType type) @@ -220,16 +215,17 @@ int64_t timerlist_deadline_ns(QEMUTimerList *timer_list) * 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); + + rcu_read_lock(); + if (!atomic_rcu_read(&timer_list->active_timers)) { + rcu_read_unlock(); return -1; } + int type = timer_list->clock->type; 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); + delta = expire_time - qemu_clock_get_ns(type); + rcu_read_unlock(); if (delta <= 0) { return 0; } @@ -332,11 +328,18 @@ void timer_init(QEMUTimer *ts, ts->expire_time = -1; } -void timer_free(QEMUTimer *ts) +static void reclaim_timer(struct rcu_head *rcu) { + QEMUTimer *ts = container_of(rcu, QEMUTimer, rcu); g_free(ts); } +void timer_free(QEMUTimer *ts) +{ + call_rcu1(&ts->rcu, reclaim_timer); +} + + static void timer_del_locked(QEMUTimerList *timer_list, QEMUTimer *ts) { QEMUTimer **pt, *t; @@ -344,6 +347,8 @@ static void timer_del_locked(QEMUTimerList *timer_list, QEMUTimer *ts) ts->expire_time = -1; pt = &timer_list->active_timers; for(;;) { + /* caller's lock causes cache updates, so we don't need to use */ + /* atomic_rcu_read or atomic_rcu_set */ t = *pt; if (!t) break; @@ -372,7 +377,6 @@ static bool timer_mod_ns_locked(QEMUTimerList *timer_list, ts->expire_time = MAX(expire_time, 0); ts->next = *pt; *pt = ts; - return pt == &timer_list->active_timers; } @@ -386,24 +390,30 @@ static void timerlist_rearm(QEMUTimerList *timer_list) /* 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); + rcu_read_lock(); + QEMUTimerList *timer_list = ts->timer_list; + QemuMutex *lock = &timer_list->clock->timer_lock; + qemu_mutex_lock(lock); + rcu_read_unlock(); timer_del_locked(timer_list, ts); - qemu_mutex_unlock(&timer_list->active_timers_lock); + qemu_mutex_unlock(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; - bool rearm; - qemu_mutex_lock(&timer_list->active_timers_lock); + bool rearm; + rcu_read_lock(); + QEMUTimerList *timer_list = ts->timer_list; + QemuMutex *lock = &timer_list->clock->timer_lock; + qemu_mutex_lock(lock); + rcu_read_unlock(); timer_del_locked(timer_list, ts); rearm = timer_mod_ns_locked(timer_list, ts, expire_time); - qemu_mutex_unlock(&timer_list->active_timers_lock); + qemu_mutex_unlock(lock); if (rearm) { timerlist_rearm(timer_list); @@ -415,19 +425,19 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) The corresponding callback will be called. */ void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time) { + bool rearm = false; + rcu_read_lock(); QEMUTimerList *timer_list = ts->timer_list; - bool rearm; - - qemu_mutex_lock(&timer_list->active_timers_lock); + QemuMutex *lock = &timer_list->clock->timer_lock; + qemu_mutex_lock(lock); + rcu_read_unlock(); if (ts->expire_time == -1 || ts->expire_time > expire_time) { if (ts->expire_time != -1) { timer_del_locked(timer_list, ts); + rearm = timer_mod_ns_locked(timer_list, ts, expire_time); } - rearm = timer_mod_ns_locked(timer_list, ts, expire_time); - } else { - rearm = false; } - qemu_mutex_unlock(&timer_list->active_timers_lock); + qemu_mutex_unlock(lock); if (rearm) { timerlist_rearm(timer_list); @@ -462,17 +472,22 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) QEMUTimerCB *cb; void *opaque; + rcu_read_lock(); + QemuMutex *lock = &timer_list->clock->timer_lock; qemu_event_reset(&timer_list->timers_done_ev); + rcu_read_unlock(); if (!timer_list->clock->enabled) { goto out; } - + /* timer_list should also be protected here. */ + /* that is the subject of the next patch. */ + /* (which will also remove this comment). */ current_time = qemu_clock_get_ns(timer_list->clock->type); for(;;) { - qemu_mutex_lock(&timer_list->active_timers_lock); + qemu_mutex_lock(lock); ts = timer_list->active_timers; if (!timer_expired_ns(ts, current_time)) { - qemu_mutex_unlock(&timer_list->active_timers_lock); + qemu_mutex_unlock(lock); break; } @@ -482,13 +497,13 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) ts->expire_time = -1; cb = ts->cb; opaque = ts->opaque; - qemu_mutex_unlock(&timer_list->active_timers_lock); - + rcu_read_lock(); + qemu_mutex_unlock(lock); /* run the callback (the timer list can be modified) */ cb(opaque); + rcu_read_unlock(); progress = true; } - out: qemu_event_set(&timer_list->timers_done_ev); return progress; -- 1.9.0
