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
