qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext tim


From: Alex Bligh
Subject: Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll
Date: Tue, 17 Sep 2013 17:38:12 +0100

On 17 Sep 2013, at 17:19, Paolo Bonzini wrote:

> Il 17/09/2013 18:09, Jan Kiszka ha scritto:
>> On 2013-08-13 16:22, Stefan Hajnoczi wrote:
>>> On Tue, Aug 13, 2013 at 03:45:44PM +0200, Jan Kiszka wrote:
>>>> Yeah:
>>>> 
>>>> -    /* No AIO operations?  Get us out of here */
>>>> -    if (!busy) {
>>>> +    /* early return if we only have the aio_notify() fd */
>>>> +    if (ctx->pollfds->len == 1) {
>>>>         return progress;
>>>>     }
>>>> 
>>>> So this is even worse for my use case.
>>> 
>>> We can change the semantics of aio_poll() so long as we don't break
>>> existing callers and tests.  It would make sense to do that after
>>> merging the io_flush and AioContext timers series.
>> 
>> Need to pick up this topic again because above change is now mainline
>> and breaks aio_poll-based timer threads:
>> 
>> How can we make progress with overcoming that check, at least for the
>> timer thread use case? Additional argument "truly_block" for aio_poll?
> 
> I wonder if we still need that "if" at all.  Guys, do you remember what
> it is good for? O:-)

I can't remember whether the code above was in my patch or Stefan's
subsequent ones, but I had suggested
   if (blocking && (ctx->pollfds->len <= 1))

On reflection, I don't think the 'if' line should be there at all.

I think the argument was that it was meant to be for stupidity
protection, i.e. where someone calls with blocking set to 1 and no
fds, it would block indefinitely (if there were no timers)
as it would select with no timeout and no fds.

Personally I think if you call things this way you deserve all you
get. I'm not sure attempting to rescue the user by returning
immediately is a great plan. If there are truly no fds at all
(not even the notify fd) and an infinite timeout perhaps we should
set the timeout to a second and log an error?

I presume the reason it's breaking aio_poll based timer threads is
because there is only one fd (the aio_notify_fd), there are
no other fd's, but there is a timeout (from the timers)? If
that's true, I think we /shouldn't/ return. Equally if there
are no timers but something is genuinely attempting to wait
on an aio_notify, I don't think we should return.

-- 
Alex Bligh







reply via email to

[Prev in Thread] Current Thread [Next in Thread]