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.
This patch applies against Paolo Bonzini's rcu branch: https://github.com/bonzini/qemu/tree/rcu V2: - Addresses comments from Alex Bligh Signed-off-by: Mike Day <ncm...@ncultra.org> --- include/qemu/timer.h | 19 +++++++-------- qemu-timer.c | 69 ++++++++++++++++++++++++++-------------------------- 2 files changed, 44 insertions(+), 44 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..dad30a3 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; NotifierList reset_notifiers; int64_t last; - QEMUClockType type; bool enabled; @@ -75,6 +74,7 @@ struct QEMUTimerList { QLIST_ENTRY(QEMUTimerList) list; QEMUTimerListNotifyCB *notify_cb; void *notify_opaque; + QemuEvent timers_done_ev; }; /** @@ -87,6 +87,7 @@ struct QEMUTimerList { */ static inline QEMUClock *qemu_clock_ptr(QEMUClockType type) { + smp_rmb(); return &qemu_clocks[type]; } @@ -148,13 +149,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 +166,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 +178,17 @@ 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; } 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); + ret = (expire_time < qemu_clock_get_ns(timer_list->clock->type)); + rcu_read_unlock(); + return ret; } bool qemu_clock_expired(QEMUClockType type) @@ -220,16 +215,16 @@ 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; } 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); + rcu_read_unlock(); if (delta <= 0) { return 0; } @@ -332,11 +327,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 +346,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 +376,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; } @@ -416,16 +419,14 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time) void timer_mod_anticipate_ns(QEMUTimer *ts, int64_t expire_time) { QEMUTimerList *timer_list = ts->timer_list; - bool rearm; + bool rearm = false; qemu_mutex_lock(&timer_list->active_timers_lock); 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); @@ -461,12 +462,13 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) bool progress = false; QEMUTimerCB *cb; void *opaque; + bool enabled; - qemu_event_reset(&timer_list->timers_done_ev); - if (!timer_list->clock->enabled) { - goto out; + enabled = atomic_rcu_read(&timer_list->clock->enabled); + if (!enabled) { + return progress; } - + qemu_event_reset(&timer_list->timers_done_ev); current_time = qemu_clock_get_ns(timer_list->clock->type); for(;;) { qemu_mutex_lock(&timer_list->active_timers_lock); @@ -482,14 +484,13 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) ts->expire_time = -1; cb = ts->cb; opaque = ts->opaque; + rcu_read_lock(); qemu_mutex_unlock(&timer_list->active_timers_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.8.5.3
