[Top][All Lists]

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

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

From: Alex Bligh
Subject: Re: [Qemu-devel] [PATCH] [RFC] aio/async: Add timed bottom-halves
Date: Tue, 16 Jul 2013 17:14:36 +0100


3. aio_poll calls aio_bh_poll. If this returns true, this indicates
  at least one non-idle bh exists, which causes aio_poll not to

No, this indicates that at least one scheduled non-idle bh exist*ed*,
which causes aio_poll not to block (because some progress has been done).

Ah yes, in the sense the callback will have been called and it now
doesn't exist.

4. aio_poll then calls g_poll (POSIX) or WaitForMultipleObjects
  (Windows). However, the timeout is either 0 or infinite.
  Both functions take a milliseconds (yuck) timeout, but that
  is not used.

I agree with the yuck. :)  But Linux has the nanoseconds-resolution
ppoll, too.

Sadly I don't think we have a g_ppoll.

So, the first thing I don't understand is why aio_poll needs the
return value of aio_bh_poll at all. Firstly, after sampling it,
it then causes aio_dispatch, and that can presumably set its own
bottom half callbacks; if this happens 'int blocking' won't be
cleared, and it will still enter g_poll with an infinite timeout.
Secondly, there seems to be an entirely separate mechanism
(aio_notify) in any case. If a non-idle bh has been scheduled,
this will cause g_poll to exit immediately as a read will be
ready. I believe this is cleared by the bh being used.

Sorry - forgot to delete the questions from above when I moved
them down below. Yes that make sense.

The second thing I don't understand is why we aren't using
the timeout on g_poll / WaitForMultipleObjects.

Because so far it wasn't needed (insert rant about idle BHs being a
hack).  This is a good occasion to use it.


But I wouldn't introduce a
new one-off concept (almost as much of a hack as idle BHs), I would
rather reuse as much code as possible from QEMUTimer/QEMUClock.  I must
admit I don't have a clear idea of how the API would look like.

So the reason I was trying to avoid using QEMUTimer stuff was that
bh's get called from aio_poll and it was not evident that all timers
would be safe to call from aio_poll.

My original approach was simply to call qemu_run_all_timers from
aio_poll at the top. I now know I'd have to (slightly) modify
the poll routine too use a specific timeout if there are any timers
ready to run and the timeout would otherwise be infinity. However
Stefan H pointed out on IRC that we'd then have to audit every
timer usage to check it was safe to call from aio_poll, or have
a 'isSafeToCallFromAioPoll' type flag (yuck). Hence we thought the
timed bh would be less intrusive as actually what I and other block
driver folks might want is something more like a bh. Equally happy
to recode it as a QEMUTimer.

What do you think? In the end I thought the schedule_bh_at stuff
was simpler.

It would
seem to be reasonably easy to make aio_poll call aio_ctx_prepare
or something that does the same calculation. This would fix
idle bh's to be more reliable (we know it's safe to call them
within aio_poll anyway, it's just a question of whether
we turn an infinite wait into a 10ms wait).

Idle BHs could be changed to timers as well, and then they would

Yes I was in essence hoping an idle BH would be treated as an
epsilon millisecond timed bh, or (if above) an epsilon millisecond
timer. By that I mean poll() would be called at least once before
it ran, but with a 0 ms timeout.

Alex Bligh

reply via email to

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