| Message ID | 1376133687-32457-1-git-send-email-alex@alex.org.uk |
|---|---|
| State | New |
| Headers | show |
On Sat, Aug 10, 2013 at 12:21:27PM +0100, Alex Bligh wrote: > Currently we use a separate timer list group (main_loop_tlg) > for the main loop. This patch replaces this with a dummy AioContext > used just for timers in the main loop. Things get interesting when we make main loop qemu_set_fd_handler() and timers just use AioContext. Basically, we are distilling out the main-loop.c stuff which is a low-level glib-style event loop from the AioContext fd handlers, timers, and BHs. This is a good step in that direction. I'm in favor of this although it current really is still a bit of a hack. > @@ -486,6 +499,20 @@ void init_clocks(void) > qemu_clock_init(type); > } > > + /* Make a dummy context for timers in the main loop. > + * > + * This context should not call the AioContext's > + * supplied notifier function (which calls > + * aio_notify) as it needs to call qemu_notify() > + * instead, as there's nothing actually using the > + * AioContext. This is a bit of a hack. > + */ > + qemu_dummy_timer_ctx = aio_context_new(); > + for (type = 0; type < QEMU_CLOCK_MAX; type++) { > + qemu_dummy_timer_ctx->tlg[type]->notify_cb = NULL; > + qemu_dummy_timer_ctx->tlg[type]->notify_opaque = NULL; > + } > + IIRC your previous patch series has something like: if (timer_list->notify_cb == NULL) { qemu_notify_event(); } else { timer_list->notify_cb(timer_list->notify_opaque); } So this should work.
On 15 Aug 2013, at 13:16, Stefan Hajnoczi wrote: > Things get interesting when we make main loop qemu_set_fd_handler() and > timers just use AioContext. > > Basically, we are distilling out the main-loop.c stuff which is a > low-level glib-style event loop from the AioContext fd handlers, timers, > and BHs. This is a good step in that direction. > > I'm in favor of this although it current really is still a bit of a > hack. Whilst I am hoping aio-timers10 (or a near equivalent) is good to base lots of other things on, personally I'm not yet convinced where this one fits. If it fits, then fine, but that probably should be part of unifying the event loop rather than the timers series (personal opinion obviously).
Il 10/08/2013 13:21, Alex Bligh ha scritto: > Currently we use a separate timer list group (main_loop_tlg) > for the main loop. This patch replaces this with a dummy AioContext > used just for timers in the main loop. > > Signed-off-by: Alex Bligh <alex@alex.org.uk> I guess for now this complicates things more than it simplifies them. You were right. :) Paolo > --- > include/block/aio.h | 3 +++ > include/qemu/timer.h | 9 ++------- > main-loop.c | 2 +- > qemu-timer.c | 39 +++++++++++++++++++++++++++++++++------ > 4 files changed, 39 insertions(+), 14 deletions(-) > > diff --git a/include/block/aio.h b/include/block/aio.h > index b08de19..50a064d 100644 > --- a/include/block/aio.h > +++ b/include/block/aio.h > @@ -81,6 +81,9 @@ typedef struct AioContext { > /* Returns 1 if there are still outstanding AIO requests; 0 otherwise */ > typedef int (AioFlushEventNotifierHandler)(EventNotifier *e); > > +/* Dummy AioContext for main loop timers */ > +extern AioContext *qemu_dummy_timer_ctx; > + > /** > * aio_context_new: Allocate a new AioContext. > * > diff --git a/include/qemu/timer.h b/include/qemu/timer.h > index 716f50b..aac3141 100644 > --- a/include/qemu/timer.h > +++ b/include/qemu/timer.h > @@ -58,8 +58,6 @@ typedef struct QEMUTimer { > int scale; > } QEMUTimer; > > -extern QEMUTimerListGroup main_loop_tlg; > - > /* > * QEMUClockType > */ > @@ -437,11 +435,8 @@ static inline QEMUTimer *timer_new_tl(QEMUTimerList *timer_list, > * > * Returns: a pointer to the timer > */ > -static inline QEMUTimer *timer_new(QEMUClockType type, int scale, > - QEMUTimerCB *cb, void *opaque) > -{ > - return timer_new_tl(main_loop_tlg[type], scale, cb, opaque); > -} > +QEMUTimer *timer_new(QEMUClockType type, int scale, > + QEMUTimerCB *cb, void *opaque); > > /** > * timer_new_ns: > diff --git a/main-loop.c b/main-loop.c > index 1f24ac1..f1ee0e5 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -479,7 +479,7 @@ int main_loop_wait(int nonblocking) > > timeout_ns = qemu_soonest_timeout(timeout_ns, > timerlistgroup_deadline_ns( > - main_loop_tlg)); > + qemu_dummy_timer_ctx->tlg)); > > ret = os_host_main_loop_wait(timeout_ns); > qemu_iohandler_poll(gpollfds, ret); > diff --git a/qemu-timer.c b/qemu-timer.c > index 0d36a21..dd0b00a 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -54,7 +54,8 @@ typedef struct QEMUClock { > bool enabled; > } QEMUClock; > > -QEMUTimerListGroup main_loop_tlg; > +AioContext *qemu_dummy_timer_ctx; > + > QEMUClock qemu_clocks[QEMU_CLOCK_MAX]; > > /* A QEMUTimerList is a list of timers attached to a clock. More > @@ -123,7 +124,6 @@ static void qemu_clock_init(QEMUClockType type) > clock->last = INT64_MIN; > QLIST_INIT(&clock->timerlists); > notifier_list_init(&clock->reset_notifiers); > - main_loop_tlg[type] = timerlist_new(type, NULL, NULL); > } > > bool qemu_clock_use_for_deadline(QEMUClockType type) > @@ -158,7 +158,7 @@ bool timerlist_has_timers(QEMUTimerList *timer_list) > bool qemu_clock_has_timers(QEMUClockType type) > { > return timerlist_has_timers( > - main_loop_tlg[type]); > + qemu_dummy_timer_ctx->tlg[type]); > } > > bool timerlist_expired(QEMUTimerList *timer_list) > @@ -171,7 +171,7 @@ bool timerlist_expired(QEMUTimerList *timer_list) > bool qemu_clock_expired(QEMUClockType type) > { > return timerlist_expired( > - main_loop_tlg[type]); > + qemu_dummy_timer_ctx->tlg[type]); > } > > /* > @@ -221,7 +221,7 @@ QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list) > > QEMUTimerList *qemu_clock_get_main_loop_timerlist(QEMUClockType type) > { > - return main_loop_tlg[type]; > + return qemu_dummy_timer_ctx->tlg[type]; > } > > void timerlist_notify(QEMUTimerList *timer_list) > @@ -291,6 +291,19 @@ void timer_init(QEMUTimer *ts, > ts->scale = scale; > } > > +QEMUTimer *timer_new(QEMUClockType type, int scale, > + QEMUTimerCB *cb, void *opaque) > +{ > + /* If this assert triggers, this is likely to mean > + * that a timer is allocated prior to the dummy > + * timer context being set up > + */ > + assert(qemu_dummy_timer_ctx); > + > + return timer_new_tl(qemu_dummy_timer_ctx->tlg[type], > + scale, cb, opaque); > +} > + > void timer_free(QEMUTimer *ts) > { > g_free(ts); > @@ -397,7 +410,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) > > bool qemu_clock_run_timers(QEMUClockType type) > { > - return timerlist_run_timers(main_loop_tlg[type]); > + return timerlist_run_timers(qemu_dummy_timer_ctx->tlg[type]); > } > > void timerlistgroup_init(QEMUTimerListGroup tlg, > @@ -486,6 +499,20 @@ void init_clocks(void) > qemu_clock_init(type); > } > > + /* Make a dummy context for timers in the main loop. > + * > + * This context should not call the AioContext's > + * supplied notifier function (which calls > + * aio_notify) as it needs to call qemu_notify() > + * instead, as there's nothing actually using the > + * AioContext. This is a bit of a hack. > + */ > + qemu_dummy_timer_ctx = aio_context_new(); > + for (type = 0; type < QEMU_CLOCK_MAX; type++) { > + qemu_dummy_timer_ctx->tlg[type]->notify_cb = NULL; > + qemu_dummy_timer_ctx->tlg[type]->notify_opaque = NULL; > + } > + > #ifdef CONFIG_PRCTL_PR_SET_TIMERSLACK > prctl(PR_SET_TIMERSLACK, 1, 0, 0, 0); > #endif >
diff --git a/include/block/aio.h b/include/block/aio.h index b08de19..50a064d 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -81,6 +81,9 @@ typedef struct AioContext { /* Returns 1 if there are still outstanding AIO requests; 0 otherwise */ typedef int (AioFlushEventNotifierHandler)(EventNotifier *e); +/* Dummy AioContext for main loop timers */ +extern AioContext *qemu_dummy_timer_ctx; + /** * aio_context_new: Allocate a new AioContext. * diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 716f50b..aac3141 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -58,8 +58,6 @@ typedef struct QEMUTimer { int scale; } QEMUTimer; -extern QEMUTimerListGroup main_loop_tlg; - /* * QEMUClockType */ @@ -437,11 +435,8 @@ static inline QEMUTimer *timer_new_tl(QEMUTimerList *timer_list, * * Returns: a pointer to the timer */ -static inline QEMUTimer *timer_new(QEMUClockType type, int scale, - QEMUTimerCB *cb, void *opaque) -{ - return timer_new_tl(main_loop_tlg[type], scale, cb, opaque); -} +QEMUTimer *timer_new(QEMUClockType type, int scale, + QEMUTimerCB *cb, void *opaque); /** * timer_new_ns: diff --git a/main-loop.c b/main-loop.c index 1f24ac1..f1ee0e5 100644 --- a/main-loop.c +++ b/main-loop.c @@ -479,7 +479,7 @@ int main_loop_wait(int nonblocking) timeout_ns = qemu_soonest_timeout(timeout_ns, timerlistgroup_deadline_ns( - main_loop_tlg)); + qemu_dummy_timer_ctx->tlg)); ret = os_host_main_loop_wait(timeout_ns); qemu_iohandler_poll(gpollfds, ret); diff --git a/qemu-timer.c b/qemu-timer.c index 0d36a21..dd0b00a 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -54,7 +54,8 @@ typedef struct QEMUClock { bool enabled; } QEMUClock; -QEMUTimerListGroup main_loop_tlg; +AioContext *qemu_dummy_timer_ctx; + QEMUClock qemu_clocks[QEMU_CLOCK_MAX]; /* A QEMUTimerList is a list of timers attached to a clock. More @@ -123,7 +124,6 @@ static void qemu_clock_init(QEMUClockType type) clock->last = INT64_MIN; QLIST_INIT(&clock->timerlists); notifier_list_init(&clock->reset_notifiers); - main_loop_tlg[type] = timerlist_new(type, NULL, NULL); } bool qemu_clock_use_for_deadline(QEMUClockType type) @@ -158,7 +158,7 @@ bool timerlist_has_timers(QEMUTimerList *timer_list) bool qemu_clock_has_timers(QEMUClockType type) { return timerlist_has_timers( - main_loop_tlg[type]); + qemu_dummy_timer_ctx->tlg[type]); } bool timerlist_expired(QEMUTimerList *timer_list) @@ -171,7 +171,7 @@ bool timerlist_expired(QEMUTimerList *timer_list) bool qemu_clock_expired(QEMUClockType type) { return timerlist_expired( - main_loop_tlg[type]); + qemu_dummy_timer_ctx->tlg[type]); } /* @@ -221,7 +221,7 @@ QEMUClockType timerlist_get_clock(QEMUTimerList *timer_list) QEMUTimerList *qemu_clock_get_main_loop_timerlist(QEMUClockType type) { - return main_loop_tlg[type]; + return qemu_dummy_timer_ctx->tlg[type]; } void timerlist_notify(QEMUTimerList *timer_list) @@ -291,6 +291,19 @@ void timer_init(QEMUTimer *ts, ts->scale = scale; } +QEMUTimer *timer_new(QEMUClockType type, int scale, + QEMUTimerCB *cb, void *opaque) +{ + /* If this assert triggers, this is likely to mean + * that a timer is allocated prior to the dummy + * timer context being set up + */ + assert(qemu_dummy_timer_ctx); + + return timer_new_tl(qemu_dummy_timer_ctx->tlg[type], + scale, cb, opaque); +} + void timer_free(QEMUTimer *ts) { g_free(ts); @@ -397,7 +410,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list) bool qemu_clock_run_timers(QEMUClockType type) { - return timerlist_run_timers(main_loop_tlg[type]); + return timerlist_run_timers(qemu_dummy_timer_ctx->tlg[type]); } void timerlistgroup_init(QEMUTimerListGroup tlg, @@ -486,6 +499,20 @@ void init_clocks(void) qemu_clock_init(type); } + /* Make a dummy context for timers in the main loop. + * + * This context should not call the AioContext's + * supplied notifier function (which calls + * aio_notify) as it needs to call qemu_notify() + * instead, as there's nothing actually using the + * AioContext. This is a bit of a hack. + */ + qemu_dummy_timer_ctx = aio_context_new(); + for (type = 0; type < QEMU_CLOCK_MAX; type++) { + qemu_dummy_timer_ctx->tlg[type]->notify_cb = NULL; + qemu_dummy_timer_ctx->tlg[type]->notify_opaque = NULL; + } + #ifdef CONFIG_PRCTL_PR_SET_TIMERSLACK prctl(PR_SET_TIMERSLACK, 1, 0, 0, 0); #endif
Currently we use a separate timer list group (main_loop_tlg) for the main loop. This patch replaces this with a dummy AioContext used just for timers in the main loop. Signed-off-by: Alex Bligh <alex@alex.org.uk> --- include/block/aio.h | 3 +++ include/qemu/timer.h | 9 ++------- main-loop.c | 2 +- qemu-timer.c | 39 +++++++++++++++++++++++++++++++++------ 4 files changed, 39 insertions(+), 14 deletions(-)