Re: [Qemu-devel] [sheepdog] [PATCH v4 06/10] coroutine: add co_aio_sleep_ns() to allow sleep in block drivers

[liu ping fan]

On Fri, Aug 2, 2013 at 2:20 PM, MORITA Kazutaka
<address@hidden> wrote:
> At Tue, 30 Jul 2013 15:58:58 +0200,
> Stefan Hajnoczi wrote:
>>
>> On Fri, Jul 26, 2013 at 03:10:48PM +0900, MORITA Kazutaka wrote:
>> > This helper function behaves similarly to co_sleep_ns(), but the
>> > sleeping coroutine will be resumed when using qemu_aio_wait().
>> >
>> > Signed-off-by: MORITA Kazutaka <address@hidden>
>> > ---
>> >  include/block/coroutine.h |  8 ++++++++
>> >  qemu-coroutine-sleep.c    | 47 
>> > +++++++++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 55 insertions(+)
>> >
>> > diff --git a/include/block/coroutine.h b/include/block/coroutine.h
>> > index 377805a..23ea6e9 100644
>> > --- a/include/block/coroutine.h
>> > +++ b/include/block/coroutine.h
>> > @@ -210,6 +210,14 @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
>> >  void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
>> >
>> >  /**
>> > + * Yield the coroutine for a given duration
>> > + *
>> > + * Behaves similarly to co_sleep_ns(), but the sleeping coroutine will be
>> > + * resumed when using qemu_aio_wait().
>> > + */
>> > +void coroutine_fn co_aio_sleep_ns(int64_t ns);
>> > +
>> > +/**
>> >   * Yield until a file descriptor becomes readable
>> >   *
>> >   * Note that this function clobbers the handlers for the file descriptor.
>> > diff --git a/qemu-coroutine-sleep.c b/qemu-coroutine-sleep.c
>> > index 169ce5c..3955347 100644
>> > --- a/qemu-coroutine-sleep.c
>> > +++ b/qemu-coroutine-sleep.c
>> > @@ -13,6 +13,7 @@
>> >
>> >  #include "block/coroutine.h"
>> >  #include "qemu/timer.h"
>> > +#include "qemu/thread.h"
>> >
>> >  typedef struct CoSleepCB {
>> >      QEMUTimer *ts;
>> > @@ -37,3 +38,49 @@ void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t 
>> > ns)
>> >      qemu_del_timer(sleep_cb.ts);
>> >      qemu_free_timer(sleep_cb.ts);
>> >  }
>> > +
>> > +typedef struct CoAioSleepCB {
>> > +    QEMUBH *bh;
>> > +    int64_t ns;
>> > +    Coroutine *co;
>> > +} CoAioSleepCB;
>> > +
>> > +static void co_aio_sleep_cb(void *opaque)
>> > +{
>> > +    CoAioSleepCB *aio_sleep_cb = opaque;
>> > +
>> > +    qemu_coroutine_enter(aio_sleep_cb->co, NULL);
>> > +}
>> > +
>> > +static void *sleep_thread(void *opaque)
>> > +{
>> > +    CoAioSleepCB *aio_sleep_cb = opaque;
>> > +    struct timespec req = {
>> > +        .tv_sec = aio_sleep_cb->ns / 1000000000,
>> > +        .tv_nsec = aio_sleep_cb->ns % 1000000000,
>> > +    };
>> > +    struct timespec rem;
>> > +
>> > +    while (nanosleep(&req, &rem) < 0 && errno == EINTR) {
>>
>> This breaks the Windows build and makes qemu_aio_wait() spin instead of
>> blocking (wastes CPU).
>>
>> I think Alex Bligh and Ping Fan's QEMUTimer in AioContext work is needed
>> here.  I have CCed them.  Their patches would allow you to use
>> co_sleep_ns() in qemu_aio_wait().
>
> Okay, I'll update this patch based on the AioContext timer.  I'm also
> happy to help Alex and Pingfan to finish the implementation.
>
Alex, do you make a further step than V4? If you have, could you share
it with us?So we can rebase our patches onto yours.

Thanks,
Pingfan