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

[Alex Bligh <alex@alex.org.uk>]

Paolo,

--On 15 July 2013 16:25:01 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote:

Thanks for the review.

> Il 06/07/2013 18:24, Alex Bligh ha scritto:
>> Add timed bottom halves. A timed bottom half is a bottom half that
>> will not execute until a given time has passed (qemu_bh_schedule_at)
>> or a given interval has passed (qemu_bh_schedule_in).
>
> ... and may be delayed arbitrarily past that given interval if you are
> running in qemu-img or in other synchronous I/O APIs.

That's true. However, the problem with timers is worse, in that we
poll for timers even less frequently as far as I can tell.

> I'm especially
> worried that this will not have any effect if bdrv_aio_cancel is calling
> qemu_aio_wait.  bdrv_aio_cancel is presumably one place where you want
> timeout/reconnect functionality to trigger.

Well, I'm a newbie here, so may well be wrong but I thought qemu_aio_wait
/did/ call bottom halves (but didn't call QemuTimers). Provided time
does actually advance (which inspection suggests it does), then
these bh's should be called just like any other bh's. I may have missed
the point here entirely.

> I would really prefer to have a TimeEventNotifier or something like
> that, which is API-compatibile with EventNotifier (so you can use the
> regular aio-*.c APIs) but triggers when a given time has passed.
> Basically an "heavyweight" QEMUTimer; that would be a timerfd on Linux,
> and a queue timer on Windows.  No idea on other POSIX systems,
> unfortunately.

I was trying to use the bh API because that's what the existing block
drivers use, and what I really wanted was a bh that wouldn't execute
for a while. Do EventNotifiers run whilst AIO is polling for completion?

> Even better would be to remove the whole timer stuff (POSIX timers,
> setitimer, and the Win32 equivalents), and just make the timers use a
> shorter timeout for the main loop.  If you do this, I suspect adding
> timer support to AioContext would be much simpler.

In discussion with Stefan H on IRC, I originally suggested moving the
QemuTimer poll to the AIO loop (or adding another), which is a half
arsed way to do what you are suggesting. He suggested this would be
hairy because the existing users might not be safe to be called there.
This was an attempt at a minimal change to fix that use.

> BTW, note that qemu-nbd (and qemu-io too) does call timers.

I'd thought I tested qemu-io. qemu-convert definitely does not.

Alex

> Paolo
>
>> Any qemu
>> clock can be used, and times are specified in nanoseconds.
>>
>> Timed bottom halves can be used where timers cannot. For instance,
>> in block drivers where there is no mainloop that calls timers
>> (qemu-nbd, qemu-img), or where (per stefanha@redhat.com) the
>> aio code loops internally and thus timers never get called.
>>
>> Signed-off-by: Alex Bligh <alex@alex.org.uk>
>
>



-- 
Alex Bligh