Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2985 bytes --]

On 4/11/19 12:27 PM, Vladimir Sementsov-Ogievskiy wrote:
> Introduce a function to gracefully wake-up a coroutine, sleeping in
> qemu_co_sleep_ns() sleep.

Maybe:

Introduce a function to gracefully short-circuit the remainder of the
delay for a coroutine sleeping in qemu_co_sleep_ns().

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/qemu/coroutine.h    |  6 ++++++
>  util/qemu-coroutine-sleep.c | 20 ++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index 9801e7f5a4..ec765c26f0 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -278,6 +278,12 @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
>   */
>  void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns);
>  
> +/*
> + * Wake a coroutine if it is sleeping by qemu_co_sleep_ns. Timer will be
> + * deleted.

Maybe:

Wake a coroutine if it is sleeping in qemu_co_sleep_ns, and delete the
timer.

> +++ b/util/qemu-coroutine-sleep.c
> @@ -17,13 +17,24 @@
>  #include "qemu/timer.h"
>  #include "block/aio.h"
>  
> +const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
> +
> +void qemu_co_sleep_wake(Coroutine *co)
> +{
> +    /* Write of schedule protected by barrier write in aio_co_schedule */
> +    const char *scheduled = atomic_cmpxchg(&co->scheduled,
> +                                           qemu_co_sleep_ns__scheduled, NULL);
> +
> +    if (scheduled == qemu_co_sleep_ns__scheduled) {
> +        aio_co_wake(co);
> +    }
> +}
> +
>  static void co_sleep_cb(void *opaque)
>  {
>      Coroutine *co = opaque;
>  
> -    /* Write of schedule protected by barrier write in aio_co_schedule */
> -    atomic_set(&co->scheduled, NULL);
> -    aio_co_wake(co);
> +    qemu_co_sleep_wake(co);
>  }
>  
>  void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
> @@ -32,7 +43,8 @@ void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
>      QEMUTimer *ts;
>      Coroutine *co = qemu_coroutine_self();
>  
> -    const char *scheduled = atomic_cmpxchg(&co->scheduled, NULL, __func__);
> +    const char *scheduled = atomic_cmpxchg(&co->scheduled, NULL,
> +                                           qemu_co_sleep_ns__scheduled);
>      if (scheduled) {
>          fprintf(stderr,
>                  "%s: Co-routine was already scheduled in '%s'\n",
> 

Here, I'd rather get an additional review from anyone more familiar with
coroutine sleeps.  I'm guessing that your intent is to request a maximum
timeout for a given operation to complete in, but to leave the sleep
loop early if the operation completes earlier.  I don't know if any
existing coroutine code can be used to express that same idea.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]