Re: [Qemu-devel] [PATCH] coroutine: Fix documentation of co_aio_sleep_ns()

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]

On Wed, Nov 08, 2017 at 09:57:47AM -0600, Eric Blake wrote:
> On 11/08/2017 09:47 AM, Paolo Bonzini wrote:
> > On 08/11/2017 16:42, Stefan Hajnoczi wrote:
> >>> In any case, co_aio_sleep_ns()
> >>> + * does not affect the #AioContext where the current coroutine is running,
> >>> + * as the coroutine will restart on the same #AioContext that it is
> >>> + * running on.
> >> I cannot parse the second sentence.  What does "affecting" an AioContext
> >> mean?  Does "where the current coroutine is running" simply mean "the
> >> caller"?
> >>
> >> What is it trying to say?  My guess is: the caller will be resumed in
> >> the current AioContext, not the timer's AioContext.
> > 
> > Yes, that is the intended meaning.  Perhaps just s/current//.
> 
> How about:
> 
> This function uses timers and hence needs to know the event loop
> (#AioContext) to place the timer on.  After the time elapses, the
> current coroutine will restart with the same #AioContext it is currently
> running in, even if that is different than the timer context passed to
> co_aio_sleep_ns().

These complicated semantics are a clue that the API should be
simplified.  QEMU has changed since this function was first introduced.
Now we can do the following:

  void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  {
      AioContext *ctx = qemu_get_current_aio_context();
      CoSleepCB sleep_cb = {
          .co = qemu_coroutine_self(),
      };
      sleep_cb.ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &sleep_cb);
      timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
      qemu_coroutine_yield();
      timer_del(sleep_cb.ts);
      timer_free(sleep_cb.ts);
  }

I don't see a reason for the caller to pass in an AioContext.

Any objections?  Will send a patch if this is okay.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]