Re: [Qemu-devel] [RFC] [PATCHv2] aio / timers pt2: Replace main_loop_tlg with qemu_dummy_timer_ctx

[Paolo Bonzini]

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 <address@hidden>

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
>