Re: [Qemu-devel] [PATCHv11 13/31] aio / timers: aio_ctx_prepare sets timeout from AioContext timers

[Alex Bligh]

>> 
>>      for (bh = ctx->first_bh; bh; bh = bh->next) {
>>          if (!bh->deleted && bh->scheduled) {
>>              if (bh->idle) {
>>                  /* idle bottom halves will be polled at least
>>                   * every 10ms */
>> -                *timeout = 10;
>> +                *timeout = qemu_soonest_timeout(*timeout, 10);
>    glib will not set *timeout to a meaningful value before calling
> aio_ctx_prepare(), right? If so, "*timeout = 10" should be used.


I don't think that's correct. Each _prepare routine is called
and has the abilitiy to alter the timeout but need not
set it at all. Indeed it should not set it as there may already
be a shorter timeout there.

Here's the code before I touched it:

aio_ctx_prepare(GSource *source, gint    *timeout)
{
    AioContext *ctx = (AioContext *) source;
    QEMUBH *bh;

    for (bh = ctx->first_bh; bh; bh = bh->next) {
        if (!bh->deleted && bh->scheduled) {
            if (bh->idle) {
                /* idle bottom halves will be polled at least
                 * every 10ms */
                *timeout = 10;
            } else {
                /* non-idle bottom halves will be executed
                 * immediately */
                *timeout = 0;
                return true;
            }
        }
    }

    return false;
}

You'll note that what this does is:
a) if there are no bottom halves, leaves *timeout as is
b) if there is a non-idle bottom half, set timeout to 0
c) else set timeout to 10 if there are only idle bottom
   halves.

Arguably (c) is a bug if timeout was shorter than 10
on entry but the whole of idle bhs are a bit of a bodge.
This is fixed by my series.

If you are asking WHERE it gets set to -1, I don't claim
to be a glib expert but I believe it's the line
  gint source_timeout = -1
around line 2816 in glib/gmain.c

-- 
Alex Bligh