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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 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