Re: [Qemu-devel] [RFC] [PATCHv8 09/30] aio / timers: Add QEMUTimerListGroup and helper functions

[liu ping fan]

[...]
>>  typedef struct QEMUClock QEMUClock;
>>  typedef struct QEMUTimerList QEMUTimerList;
>> +typedef QEMUTimerList *QEMUTimerListGroup[QEMU_CLOCK_MAX];
>
> Please wrap this in a struct for easier future extensibility.
>
> I'm not a big fan of the TimerListGroup name, but I cannot think of
> anything better...
>
> ... except if we get rid of TimerListGroup completely and put it in
> AioContext.  To do so, we can have _two_ AioContexts in main-loop.c.
> One is for the block layer, the other is only used for timers.  Later we

Why can the AioContexts of block layer NOT hold timers?  For the
reason mainloop's iohandler runs after it?  Or any reason raised by
blocklayer itself?

Thx
Pingfan
> could move bottom halves from the first to the second, too.
>
> Sorry for not thinking of this before.
>
> Paolo
>
>>  typedef void QEMUTimerCB(void *opaque);
>>
>> +extern QEMUTimerListGroup main_loop_tlg;
>>  extern QEMUClock *qemu_clocks[QEMU_CLOCK_MAX];
>>
>>  /**
>> @@ -211,6 +213,49 @@ QEMUClock *timerlist_get_clock(QEMUTimerList 
>> *timer_list);
>>  bool timerlist_run_timers(QEMUTimerList *timer_list);
>>
>>  /**
>> + * timerlistgroup_init:
>> + * @tlg: the timer list group
>> + *
>> + * Initialise a timer list group. This must already be
>> + * allocated in memory and zeroed.
>> + */
>> +void timerlistgroup_init(QEMUTimerListGroup tlg);
>> +
>> +/**
>> + * timerlistgroup_deinit:
>> + * @tlg: the timer list group
>> + *
>> + * Deinitialise a timer list group. This must already be
>> + * initialised. Note the memory is not freed.
>> + */
>> +void timerlistgroup_deinit(QEMUTimerListGroup tlg);
>> +
>> +/**
>> + * timerlistgroup_run_timers:
>> + * @tlg: the timer list group
>> + *
>> + * Run the timers associated with a timer list group.
>> + * This will run timers on multiple clocks.
>> + *
>> + * Returns: true if any timer callback ran
>> + */
>> +bool timerlistgroup_run_timers(QEMUTimerListGroup tlg);
>> +
>> +/**
>> + * timerlistgroup_deadline_ns
>> + * @tlg: the timer list group
>> + *
>> + * Determine the deadline of the soonest timer to
>> + * expire associated with any timer list linked to
>> + * the timer list group. Only clocks suitable for
>> + * deadline calculation are included.
>> + *
>> + * Returns: the deadline in nanoseconds or -1 if no
>> + * timers are to expire.
>> + */
>> +int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup tlg);
>> +
>> +/**
>>   * qemu_timeout_ns_to_ms:
>>   * @ns: nanosecond timeout value
>>   *
>> diff --git a/qemu-timer.c b/qemu-timer.c
>> index 1a0a4b1..e151d24 100644
>> --- a/qemu-timer.c
>> +++ b/qemu-timer.c
>> @@ -59,6 +59,7 @@ struct QEMUClock {
>>      bool enabled;
>>  };
>>
>> +QEMUTimerListGroup main_loop_tlg;
>>  QEMUClock *qemu_clocks[QEMU_CLOCK_MAX];
>>
>>  /* A QEMUTimerList is a list of timers attached to a clock. More
>> @@ -575,6 +576,45 @@ bool qemu_run_timers(QEMUClock *clock)
>>      return timerlist_run_timers(clock->main_loop_timerlist);
>>  }
>>
>> +void timerlistgroup_init(QEMUTimerListGroup tlg)
>> +{
>> +    QEMUClockType type;
>> +    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
>> +        tlg[type] = timerlist_new(type);
>> +    }
>> +}
>> +
>> +void timerlistgroup_deinit(QEMUTimerListGroup tlg)
>> +{
>> +    QEMUClockType type;
>> +    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
>> +        timerlist_free(tlg[type]);
>> +    }
>> +}
>> +
>> +bool timerlistgroup_run_timers(QEMUTimerListGroup tlg)
>> +{
>> +    QEMUClockType type;
>> +    bool progress = false;
>> +    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
>> +        progress |= timerlist_run_timers(tlg[type]);
>> +    }
>> +    return progress;
>> +}
>> +
>> +int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup tlg)
>> +{
>> +    int64_t deadline = -1;
>> +    QEMUClockType type;
>> +    for (type = 0; type < QEMU_CLOCK_MAX; type++) {
>> +        if (qemu_clock_use_for_deadline(tlg[type]->clock)) {
>> +            deadline = qemu_soonest_timeout(deadline,
>> +                                            
>> timerlist_deadline_ns(tlg[type]));
>> +        }
>> +    }
>> +    return deadline;
>> +}
>> +
>>  int64_t qemu_get_clock_ns(QEMUClock *clock)
>>  {
>>      int64_t now, last;
>> @@ -616,6 +656,7 @@ void init_clocks(void)
>>      for (type = 0; type < QEMU_CLOCK_MAX; type++) {
>>          if (!qemu_clocks[type]) {
>>              qemu_clocks[type] = qemu_clock_new(type);
>> +            main_loop_tlg[type] = qemu_clocks[type]->main_loop_timerlist;
>>          }
>>      }
>>
>>
>