Re: [Qemu-devel] [PATCH] [RFC] aio/async: Add timed bottom-halves

[Paolo Bonzini <pbonzini@redhat.com>]

Il 16/07/2013 01:04, Alex Bligh ha scritto:
> Paolo,
> 
> --On 15 July 2013 22:53:17 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> So far you are right.
>>
>> But this only happens if qemu_aio_wait() actually returns, so that on
>> the next call we poll for timers.  If QEMU is stuck in qemu_aio_wait()'s
>> infinite-timeout poll(), it will never advance and process the timed
>> bottom halves.
> 
> I may have misunderstood the code here. I thought what it was doing was
> setting the timeout to poll() to 0 if there was a bh queued, 10ms if
> there was a bh->idle timeout queued, or infinite otherwise.
> 
> What I changed it to do was set the timeout to:
> * 0 if there was an untimed bh ready (no change)
> * 10ms if there was an untimed idle bh ready (no change)
> * the number of ms to expiry if there is a timed bh ready (rounded up)
> * infinite otherwise (no change)
> * and if there is more the one, the minimum of those

You did.  But aio_wait() ignores the timeout.  It is only used by the
main loop.

> So the infinite timeout poll should not be entered if there is a timed
> bh there. This of course assumes a single thread as else there is a TOCTOU
> problem if a timed bh gets inserted between calculating the expiry time
> and the poll.
> 
>> This goes to the question of having aio_notify() or not.  If you have
>> it, you will immediately process timed BHs that are "born expired".  For
>> other bottom halves, there will be no difference if you add it or not.
> 
> You've lost me there. I'm taking it by 'born expired' you mean
> when poll() is entered, they're already expired. Timed BH's that are
> 'born expired' should (with the patch I sent) be treated exactly the
> same as untimed BH's, i.e. the timeout to poll should be being
> set to zero.

Yes, that's exactly what I meant.  Looks like you understood well! :)

Paolo