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

[Wenchao Xia]

于 2013-8-20 14:48, 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

  Thanks for the explanation. It seems *timeout is always set
to -1 before calling GSource's prepare(), so
"*timeout = qemu_soonest_timeout(*timeout, 10);"
will always get *timeout = 10, so this call can be saved.
--
Best Regards

Wenchao Xia