| Message ID | 1374819052-4292-7-git-send-email-morita.kazutaka@lab.ntt.co.jp |
|---|---|
| State | New |
| Headers | show |
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 <morita.kazutaka@lab.ntt.co.jp> > --- > 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(). > + req = rem; > + } > + > + qemu_bh_schedule(aio_sleep_cb->bh); > + > + return NULL; > +} > + > +void coroutine_fn co_aio_sleep_ns(int64_t ns) > +{ > + CoAioSleepCB aio_sleep_cb = { > + .ns = ns, > + .co = qemu_coroutine_self(), > + }; > + QemuThread thread; > + > + aio_sleep_cb.bh = qemu_bh_new(co_aio_sleep_cb, &aio_sleep_cb); > + qemu_thread_create(&thread, sleep_thread, &aio_sleep_cb, > + QEMU_THREAD_DETACHED); > + qemu_coroutine_yield(); > + qemu_bh_delete(aio_sleep_cb.bh); > +} > -- > 1.8.1.3.566.gaa39828 > >
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 <morita.kazutaka@lab.ntt.co.jp> > > --- > > 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. Thanks, Kazutaka
On Fri, Aug 2, 2013 at 2:20 PM, MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> 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 <morita.kazutaka@lab.ntt.co.jp> >> > --- >> > 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
Pingfan, --On 2 August 2013 16:19:31 +0800 liu ping fan <qemulist@gmail.com> wrote: >> 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. I've got no changes beyond v4 yet. The changes I am thinking of making are: 1. changing the AioContext * in QEMUTimerList to be a pointer to a callback instead. This is a tiny and self-contained change and may well be unnecessary. 2. Whatever comes out of Paolo's long email of yesterday, the contents of which I have not fully digested. However, I think what this boils down to is calling *_notify slightly less often. 3. Someone (Paolo/Kevin?) suggested renaming the *new* timer functions (i.e. the ones with timerlist in the name). That won't affect anything current. I suggest you go with v4 for the time being. If I end up doing a v5 and your code doesn't sit well on top of it, I can rebase to yours or something. I'd expect changes in v5 to be really small.
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) { + req = rem; + } + + qemu_bh_schedule(aio_sleep_cb->bh); + + return NULL; +} + +void coroutine_fn co_aio_sleep_ns(int64_t ns) +{ + CoAioSleepCB aio_sleep_cb = { + .ns = ns, + .co = qemu_coroutine_self(), + }; + QemuThread thread; + + aio_sleep_cb.bh = qemu_bh_new(co_aio_sleep_cb, &aio_sleep_cb); + qemu_thread_create(&thread, sleep_thread, &aio_sleep_cb, + QEMU_THREAD_DETACHED); + qemu_coroutine_yield(); + qemu_bh_delete(aio_sleep_cb.bh); +}
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 <morita.kazutaka@lab.ntt.co.jp> --- include/block/coroutine.h | 8 ++++++++ qemu-coroutine-sleep.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+)