* [PATCH 1/6] coroutine-sleep: use a stack-allocated timer
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:00 ` Vladimir Sementsov-Ogievskiy
2021-05-03 11:25 ` [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
The lifetime of the timer is well-known (it cannot outlive
qemu_co_sleep_ns_wakeable, because it's deleted by the time the
coroutine resumes), so it is not necessary to place it on the heap.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-coroutine-sleep.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 8c4dac4fd7..eec6e81f3f 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -21,7 +21,7 @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
struct QemuCoSleepState {
Coroutine *co;
- QEMUTimer *ts;
+ QEMUTimer ts;
QemuCoSleepState **user_state_pointer;
};
@@ -35,7 +35,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
if (sleep_state->user_state_pointer) {
*sleep_state->user_state_pointer = NULL;
}
- timer_del(sleep_state->ts);
+ timer_del(&sleep_state->ts);
aio_co_wake(sleep_state->co);
}
@@ -50,7 +50,6 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
AioContext *ctx = qemu_get_current_aio_context();
QemuCoSleepState state = {
.co = qemu_coroutine_self(),
- .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
.user_state_pointer = sleep_state,
};
@@ -63,10 +62,11 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
abort();
}
+ aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
if (sleep_state) {
*sleep_state = &state;
}
- timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
+ timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
if (sleep_state) {
/*
@@ -75,5 +75,4 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
*/
assert(*sleep_state == NULL);
}
- timer_free(state.ts);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] coroutine-sleep: use a stack-allocated timer
2021-05-03 11:25 ` [PATCH 1/6] coroutine-sleep: use a stack-allocated timer Paolo Bonzini
@ 2021-05-10 10:00 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-10 10:00 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: eesposit, stefanha
03.05.2021 14:25, Paolo Bonzini wrote:
> The lifetime of the timer is well-known (it cannot outlive
> qemu_co_sleep_ns_wakeable, because it's deleted by the time the
> coroutine resumes), so it is not necessary to place it on the heap.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
2021-05-03 11:25 ` [PATCH 1/6] coroutine-sleep: use a stack-allocated timer Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:03 ` Vladimir Sementsov-Ogievskiy
2021-05-03 11:25 ` [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
Simplify the code by removing conditionals. qemu_co_sleep_ns
can simply use point the argument to an on-stack temporary.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/coroutine.h | 5 +++--
util/qemu-coroutine-sleep.c | 18 +++++-------------
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index ce5b9c6851..c5d7742989 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -295,7 +295,7 @@ typedef struct QemuCoSleepState QemuCoSleepState;
/**
* Yield the coroutine for a given duration. During this yield, @sleep_state
- * (if not NULL) is set to an opaque pointer, which may be used for
+ * is set to an opaque pointer, which may be used for
* qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
* timer fires. Don't save the obtained value to other variables and don't call
* qemu_co_sleep_wake from another aio context.
@@ -304,7 +304,8 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
QemuCoSleepState **sleep_state);
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
{
- qemu_co_sleep_ns_wakeable(type, ns, NULL);
+ QemuCoSleepState *unused = NULL;
+ qemu_co_sleep_ns_wakeable(type, ns, &unused);
}
/**
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index eec6e81f3f..3f6f637e81 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -32,9 +32,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
qemu_co_sleep_ns__scheduled, NULL);
assert(scheduled == qemu_co_sleep_ns__scheduled);
- if (sleep_state->user_state_pointer) {
- *sleep_state->user_state_pointer = NULL;
- }
+ *sleep_state->user_state_pointer = NULL;
timer_del(&sleep_state->ts);
aio_co_wake(sleep_state->co);
}
@@ -63,16 +61,10 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
}
aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
- if (sleep_state) {
- *sleep_state = &state;
- }
+ *sleep_state = &state;
timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
- if (sleep_state) {
- /*
- * Note that *sleep_state is cleared during qemu_co_sleep_wake
- * before resuming this coroutine.
- */
- assert(*sleep_state == NULL);
- }
+
+ /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
+ assert(*sleep_state == NULL);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument
2021-05-03 11:25 ` [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument Paolo Bonzini
@ 2021-05-10 10:03 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-10 10:03 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: eesposit, stefanha
03.05.2021 14:25, Paolo Bonzini wrote:
> Simplify the code by removing conditionals. qemu_co_sleep_ns
> can simply use point the argument to an on-stack temporary.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qemu/coroutine.h | 5 +++--
> util/qemu-coroutine-sleep.c | 18 +++++-------------
> 2 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index ce5b9c6851..c5d7742989 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -295,7 +295,7 @@ typedef struct QemuCoSleepState QemuCoSleepState;
>
> /**
> * Yield the coroutine for a given duration. During this yield, @sleep_state
> - * (if not NULL) is set to an opaque pointer, which may be used for
> + * is set to an opaque pointer, which may be used for
> * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
> * timer fires. Don't save the obtained value to other variables and don't call
> * qemu_co_sleep_wake from another aio context.
> @@ -304,7 +304,8 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> QemuCoSleepState **sleep_state);
> static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
> {
> - qemu_co_sleep_ns_wakeable(type, ns, NULL);
> + QemuCoSleepState *unused = NULL;
> + qemu_co_sleep_ns_wakeable(type, ns, &unused);
> }
>
> /**
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index eec6e81f3f..3f6f637e81 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -32,9 +32,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
> qemu_co_sleep_ns__scheduled, NULL);
>
> assert(scheduled == qemu_co_sleep_ns__scheduled);
> - if (sleep_state->user_state_pointer) {
> - *sleep_state->user_state_pointer = NULL;
> - }
> + *sleep_state->user_state_pointer = NULL;
> timer_del(&sleep_state->ts);
> aio_co_wake(sleep_state->co);
> }
> @@ -63,16 +61,10 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> }
>
> aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
> - if (sleep_state) {
> - *sleep_state = &state;
> - }
> + *sleep_state = &state;
> timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
> qemu_coroutine_yield();
> - if (sleep_state) {
> - /*
> - * Note that *sleep_state is cleared during qemu_co_sleep_wake
> - * before resuming this coroutine.
> - */
> - assert(*sleep_state == NULL);
> - }
> +
> + /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
over-80 line
> + assert(*sleep_state == NULL);
> }
>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
2021-05-03 11:25 ` [PATCH 1/6] coroutine-sleep: use a stack-allocated timer Paolo Bonzini
2021-05-03 11:25 ` [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:15 ` Vladimir Sementsov-Ogievskiy
2021-05-03 11:25 ` [PATCH 4/6] coroutine-sleep: move timer out of QemuCoSleepState Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
All callers of qemu_co_sleep_wake are checking whether they are passing
a NULL argument inside the pointer-to-pointer: do the check in
qemu_co_sleep_wake itself.
As a side effect, qemu_co_sleep_wake can be called more than once and it
will only wake the coroutine once; after the first time, the argument
will be set to NULL via *sleep_state->user_state_pointer.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-copy.c | 4 +---
block/nbd.c | 8 ++------
util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
3 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 9b4af00614..f896dc56f2 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -674,9 +674,7 @@ out:
void block_copy_kick(BlockCopyCallState *call_state)
{
- if (call_state->sleep_state) {
- qemu_co_sleep_wake(call_state->sleep_state);
- }
+ qemu_co_sleep_wake(call_state->sleep_state);
}
/*
diff --git a/block/nbd.c b/block/nbd.c
index 1d4668d42d..1c6315b168 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -289,9 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
s->drained = true;
- if (s->connection_co_sleep_ns_state) {
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
- }
+ qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
nbd_co_establish_connection_cancel(bs, false);
@@ -330,9 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
s->state = NBD_CLIENT_QUIT;
if (s->connection_co) {
- if (s->connection_co_sleep_ns_state) {
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
- }
+ qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
nbd_co_establish_connection_cancel(bs, true);
}
if (qemu_in_coroutine()) {
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 3f6f637e81..096eea3444 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -27,19 +27,22 @@ struct QemuCoSleepState {
void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
{
- /* Write of schedule protected by barrier write in aio_co_schedule */
- const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
- qemu_co_sleep_ns__scheduled, NULL);
+ if (sleep_state) {
+ /* Write of schedule protected by barrier write in aio_co_schedule */
+ const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+ qemu_co_sleep_ns__scheduled, NULL);
- assert(scheduled == qemu_co_sleep_ns__scheduled);
- *sleep_state->user_state_pointer = NULL;
- timer_del(&sleep_state->ts);
- aio_co_wake(sleep_state->co);
+ assert(scheduled == qemu_co_sleep_ns__scheduled);
+ *sleep_state->user_state_pointer = NULL;
+ timer_del(&sleep_state->ts);
+ aio_co_wake(sleep_state->co);
+ }
}
static void co_sleep_cb(void *opaque)
{
- qemu_co_sleep_wake(opaque);
+ QemuCoSleepState **sleep_state = opaque;
+ qemu_co_sleep_wake(*sleep_state);
}
void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
@@ -60,7 +63,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
abort();
}
- aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
+ aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
*sleep_state = &state;
timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing
2021-05-03 11:25 ` [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing Paolo Bonzini
@ 2021-05-10 10:15 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-10 10:15 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: eesposit, stefanha
03.05.2021 14:25, Paolo Bonzini wrote:
> All callers of qemu_co_sleep_wake are checking whether they are passing
> a NULL argument inside the pointer-to-pointer: do the check in
> qemu_co_sleep_wake itself.
>
> As a side effect, qemu_co_sleep_wake can be called more than once and it
> will only wake the coroutine once; after the first time, the argument
> will be set to NULL via *sleep_state->user_state_pointer.
>
commit message say nothing about change in co_sleep_cb: it now passes the user pointer too.
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/block-copy.c | 4 +---
> block/nbd.c | 8 ++------
> util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
> 3 files changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/block/block-copy.c b/block/block-copy.c
> index 9b4af00614..f896dc56f2 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -674,9 +674,7 @@ out:
>
> void block_copy_kick(BlockCopyCallState *call_state)
> {
> - if (call_state->sleep_state) {
> - qemu_co_sleep_wake(call_state->sleep_state);
> - }
> + qemu_co_sleep_wake(call_state->sleep_state);
> }
>
> /*
> diff --git a/block/nbd.c b/block/nbd.c
> index 1d4668d42d..1c6315b168 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -289,9 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
> BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
>
> s->drained = true;
> - if (s->connection_co_sleep_ns_state) {
> - qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> - }
> + qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
>
> nbd_co_establish_connection_cancel(bs, false);
>
> @@ -330,9 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
>
> s->state = NBD_CLIENT_QUIT;
> if (s->connection_co) {
> - if (s->connection_co_sleep_ns_state) {
> - qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> - }
> + qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> nbd_co_establish_connection_cancel(bs, true);
> }
> if (qemu_in_coroutine()) {
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index 3f6f637e81..096eea3444 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -27,19 +27,22 @@ struct QemuCoSleepState {
>
> void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
> {
> - /* Write of schedule protected by barrier write in aio_co_schedule */
> - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> - qemu_co_sleep_ns__scheduled, NULL);
> + if (sleep_state) {
> + /* Write of schedule protected by barrier write in aio_co_schedule */
> + const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> + qemu_co_sleep_ns__scheduled, NULL);
over-80 line (and indentation is broken)
>
> - assert(scheduled == qemu_co_sleep_ns__scheduled);
> - *sleep_state->user_state_pointer = NULL;
> - timer_del(&sleep_state->ts);
> - aio_co_wake(sleep_state->co);
> + assert(scheduled == qemu_co_sleep_ns__scheduled);
> + *sleep_state->user_state_pointer = NULL;
> + timer_del(&sleep_state->ts);
> + aio_co_wake(sleep_state->co);
> + }
> }
>
> static void co_sleep_cb(void *opaque)
> {
> - qemu_co_sleep_wake(opaque);
> + QemuCoSleepState **sleep_state = opaque;
> + qemu_co_sleep_wake(*sleep_state);
> }
>
> void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> @@ -60,7 +63,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> abort();
> }
>
> - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
> + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
> *sleep_state = &state;
> timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
> qemu_coroutine_yield();
>
Patch looks OK itself:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
(still, please improve commit message).
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] coroutine-sleep: move timer out of QemuCoSleepState
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
` (2 preceding siblings ...)
2021-05-03 11:25 ` [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:19 ` Vladimir Sementsov-Ogievskiy
2021-05-03 11:25 ` [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
This simplification is enabled by the previous patch. Now aio_co_wake
will only be called once, therefore we do not care about a spurious
firing of the timer after a qemu_co_sleep_wake.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-coroutine-sleep.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 096eea3444..68e9505b2e 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -21,7 +21,6 @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
struct QemuCoSleepState {
Coroutine *co;
- QEMUTimer ts;
QemuCoSleepState **user_state_pointer;
};
@@ -34,7 +33,6 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
assert(scheduled == qemu_co_sleep_ns__scheduled);
*sleep_state->user_state_pointer = NULL;
- timer_del(&sleep_state->ts);
aio_co_wake(sleep_state->co);
}
}
@@ -49,6 +47,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
QemuCoSleepState **sleep_state)
{
AioContext *ctx = qemu_get_current_aio_context();
+ QEMUTimer ts;
QemuCoSleepState state = {
.co = qemu_coroutine_self(),
.user_state_pointer = sleep_state,
@@ -63,10 +62,11 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
abort();
}
- aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
*sleep_state = &state;
- timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
+ timer_del(&ts);
/* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
assert(*sleep_state == NULL);
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
` (3 preceding siblings ...)
2021-05-03 11:25 ` [PATCH 4/6] coroutine-sleep: move timer out of QemuCoSleepState Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:38 ` Vladimir Sementsov-Ogievskiy
2021-05-03 11:25 ` [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep Paolo Bonzini
2021-05-11 14:07 ` [PATCH 0/6] coroutine: new sleep/wake API Stefan Hajnoczi
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
Right now, users of qemu_co_sleep_ns_wakeable are passing
a pointer to QemuCoSleepState by reference to the function, but
QemuCoSleepState really is just a Coroutine*. Making the
content of the struct public is just as efficient and lets us
skip the user_state_pointer indirection: the Coroutine* is
cleared directly, rather than the pointer to it.
Since the usage is changed, take the occasion to rename the
struct to QemuCoSleep.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-copy.c | 8 +++----
block/nbd.c | 10 ++++-----
include/qemu/coroutine.h | 22 +++++++++----------
util/qemu-coroutine-sleep.c | 43 ++++++++++++++++---------------------
4 files changed, 39 insertions(+), 44 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index f896dc56f2..c2e5090412 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -50,7 +50,7 @@ typedef struct BlockCopyCallState {
/* State */
int ret;
bool finished;
- QemuCoSleepState *sleep_state;
+ QemuCoSleep sleep;
bool cancelled;
/* OUT parameters */
@@ -625,8 +625,8 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
if (ns > 0) {
block_copy_task_end(task, -EAGAIN);
g_free(task);
- qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
- &call_state->sleep_state);
+ qemu_co_sleep_ns_wakeable(&call_state->sleep,
+ QEMU_CLOCK_REALTIME, ns);
continue;
}
}
@@ -674,7 +674,7 @@ out:
void block_copy_kick(BlockCopyCallState *call_state)
{
- qemu_co_sleep_wake(call_state->sleep_state);
+ qemu_co_sleep_wake(&call_state->sleep);
}
/*
diff --git a/block/nbd.c b/block/nbd.c
index 1c6315b168..616f9ae6c4 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -116,7 +116,7 @@ typedef struct BDRVNBDState {
CoQueue free_sema;
Coroutine *connection_co;
Coroutine *teardown_co;
- QemuCoSleepState *connection_co_sleep_ns_state;
+ QemuCoSleep reconnect_sleep;
bool drained;
bool wait_drained_end;
int in_flight;
@@ -289,7 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
s->drained = true;
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+ qemu_co_sleep_wake(&s->reconnect_sleep);
nbd_co_establish_connection_cancel(bs, false);
@@ -328,7 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
s->state = NBD_CLIENT_QUIT;
if (s->connection_co) {
- qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+ qemu_co_sleep_wake(&s->reconnect_sleep);
nbd_co_establish_connection_cancel(bs, true);
}
if (qemu_in_coroutine()) {
@@ -685,8 +685,8 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
}
bdrv_inc_in_flight(s->bs);
} else {
- qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
- &s->connection_co_sleep_ns_state);
+ qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
+ QEMU_CLOCK_REALTIME, timeout);
if (s->drained) {
continue;
}
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index c5d7742989..77cb8ce459 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -291,21 +291,21 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
*/
void qemu_co_rwlock_unlock(CoRwlock *lock);
-typedef struct QemuCoSleepState QemuCoSleepState;
+typedef struct QemuCoSleep {
+ Coroutine *to_wake;
+} QemuCoSleep;
/**
- * Yield the coroutine for a given duration. During this yield, @sleep_state
- * is set to an opaque pointer, which may be used for
- * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
- * timer fires. Don't save the obtained value to other variables and don't call
- * qemu_co_sleep_wake from another aio context.
+ * Yield the coroutine for a given duration. During this yield, @w
+ * can be used with qemu_co_sleep_wake() to terminate the sleep.
*/
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
- QemuCoSleepState **sleep_state);
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+ QEMUClockType type, int64_t ns);
+
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
{
- QemuCoSleepState *unused = NULL;
- qemu_co_sleep_ns_wakeable(type, ns, &unused);
+ QemuCoSleep w = { 0 };
+ qemu_co_sleep_ns_wakeable(&w, type, ns);
}
/**
@@ -314,7 +314,7 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
* qemu_co_sleep_ns() and should be checked to be non-NULL before calling
* qemu_co_sleep_wake().
*/
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
+void qemu_co_sleep_wake(QemuCoSleep *w);
/**
* Yield until a file descriptor becomes readable
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 68e9505b2e..89c3b758c5 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -19,42 +19,37 @@
static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
-struct QemuCoSleepState {
+void qemu_co_sleep_wake(QemuCoSleep *w)
+{
Coroutine *co;
- QemuCoSleepState **user_state_pointer;
-};
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-{
- if (sleep_state) {
+ co = w->to_wake;
+ w->to_wake = NULL;
+ if (co) {
/* Write of schedule protected by barrier write in aio_co_schedule */
- const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
- qemu_co_sleep_ns__scheduled, NULL);
+ const char *scheduled = qatomic_cmpxchg(&co->scheduled,
+ qemu_co_sleep_ns__scheduled, NULL);
assert(scheduled == qemu_co_sleep_ns__scheduled);
- *sleep_state->user_state_pointer = NULL;
- aio_co_wake(sleep_state->co);
+ aio_co_wake(co);
}
}
static void co_sleep_cb(void *opaque)
{
- QemuCoSleepState **sleep_state = opaque;
- qemu_co_sleep_wake(*sleep_state);
+ QemuCoSleep *w = opaque;
+ qemu_co_sleep_wake(w);
}
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
- QemuCoSleepState **sleep_state)
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+ QEMUClockType type, int64_t ns)
{
+ Coroutine *co = qemu_coroutine_self();
AioContext *ctx = qemu_get_current_aio_context();
QEMUTimer ts;
- QemuCoSleepState state = {
- .co = qemu_coroutine_self(),
- .user_state_pointer = sleep_state,
- };
- const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
- qemu_co_sleep_ns__scheduled);
+ const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
+ qemu_co_sleep_ns__scheduled);
if (scheduled) {
fprintf(stderr,
"%s: Co-routine was already scheduled in '%s'\n",
@@ -62,12 +57,12 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
abort();
}
- aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
- *sleep_state = &state;
+ w->to_wake = co;
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
timer_mod(&ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
timer_del(&ts);
- /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
- assert(*sleep_state == NULL);
+ /* w->to_wake is cleared before resuming this coroutine. */
+ assert(w->to_wake == NULL);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API
2021-05-03 11:25 ` [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API Paolo Bonzini
@ 2021-05-10 10:38 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-10 10:38 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: eesposit, stefanha
03.05.2021 14:25, Paolo Bonzini wrote:
> Right now, users of qemu_co_sleep_ns_wakeable are passing
> a pointer to QemuCoSleepState by reference to the function, but
> QemuCoSleepState really is just a Coroutine*. Making the
> content of the struct public is just as efficient and lets us
> skip the user_state_pointer indirection: the Coroutine* is
> cleared directly, rather than the pointer to it.
>
> Since the usage is changed, take the occasion to rename the
> struct to QemuCoSleep.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/block-copy.c | 8 +++----
> block/nbd.c | 10 ++++-----
> include/qemu/coroutine.h | 22 +++++++++----------
> util/qemu-coroutine-sleep.c | 43 ++++++++++++++++---------------------
> 4 files changed, 39 insertions(+), 44 deletions(-)
>
> diff --git a/block/block-copy.c b/block/block-copy.c
> index f896dc56f2..c2e5090412 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -50,7 +50,7 @@ typedef struct BlockCopyCallState {
> /* State */
> int ret;
> bool finished;
> - QemuCoSleepState *sleep_state;
> + QemuCoSleep sleep;
> bool cancelled;
>
> /* OUT parameters */
> @@ -625,8 +625,8 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
> if (ns > 0) {
> block_copy_task_end(task, -EAGAIN);
> g_free(task);
> - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
> - &call_state->sleep_state);
> + qemu_co_sleep_ns_wakeable(&call_state->sleep,
> + QEMU_CLOCK_REALTIME, ns);
> continue;
> }
> }
> @@ -674,7 +674,7 @@ out:
>
> void block_copy_kick(BlockCopyCallState *call_state)
> {
> - qemu_co_sleep_wake(call_state->sleep_state);
> + qemu_co_sleep_wake(&call_state->sleep);
> }
>
> /*
> diff --git a/block/nbd.c b/block/nbd.c
> index 1c6315b168..616f9ae6c4 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -116,7 +116,7 @@ typedef struct BDRVNBDState {
> CoQueue free_sema;
> Coroutine *connection_co;
> Coroutine *teardown_co;
> - QemuCoSleepState *connection_co_sleep_ns_state;
> + QemuCoSleep reconnect_sleep;
> bool drained;
> bool wait_drained_end;
> int in_flight;
> @@ -289,7 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
> BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
>
> s->drained = true;
> - qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> + qemu_co_sleep_wake(&s->reconnect_sleep);
>
> nbd_co_establish_connection_cancel(bs, false);
>
> @@ -328,7 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
>
> s->state = NBD_CLIENT_QUIT;
> if (s->connection_co) {
> - qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> + qemu_co_sleep_wake(&s->reconnect_sleep);
> nbd_co_establish_connection_cancel(bs, true);
> }
> if (qemu_in_coroutine()) {
> @@ -685,8 +685,8 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
> }
> bdrv_inc_in_flight(s->bs);
> } else {
> - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
> - &s->connection_co_sleep_ns_state);
> + qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
> + QEMU_CLOCK_REALTIME, timeout);
> if (s->drained) {
> continue;
> }
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index c5d7742989..77cb8ce459 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -291,21 +291,21 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
> */
> void qemu_co_rwlock_unlock(CoRwlock *lock);
>
> -typedef struct QemuCoSleepState QemuCoSleepState;
> +typedef struct QemuCoSleep {
> + Coroutine *to_wake;
> +} QemuCoSleep;
>
> /**
> - * Yield the coroutine for a given duration. During this yield, @sleep_state
> - * is set to an opaque pointer, which may be used for
> - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
> - * timer fires. Don't save the obtained value to other variables and don't call
> - * qemu_co_sleep_wake from another aio context.
> + * Yield the coroutine for a given duration. During this yield, @w
> + * can be used with qemu_co_sleep_wake() to terminate the sleep.
I'd add that function initializes @w.
> */
> -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> - QemuCoSleepState **sleep_state);
> +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
> + QEMUClockType type, int64_t ns);
> +
> static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
> {
> - QemuCoSleepState *unused = NULL;
> - qemu_co_sleep_ns_wakeable(type, ns, &unused);
> + QemuCoSleep w = { 0 };
> + qemu_co_sleep_ns_wakeable(&w, type, ns);
> }
>
> /**
> @@ -314,7 +314,7 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
> * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
> * qemu_co_sleep_wake().
> */
> -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
> +void qemu_co_sleep_wake(QemuCoSleep *w);
>
> /**
> * Yield until a file descriptor becomes readable
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index 68e9505b2e..89c3b758c5 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -19,42 +19,37 @@
>
> static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
>
> -struct QemuCoSleepState {
> +void qemu_co_sleep_wake(QemuCoSleep *w)
> +{
> Coroutine *co;
> - QemuCoSleepState **user_state_pointer;
> -};
>
> -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
> -{
> - if (sleep_state) {
> + co = w->to_wake;
> + w->to_wake = NULL;
Don't you like g_steal_pointer()?
> + if (co) {
> /* Write of schedule protected by barrier write in aio_co_schedule */
> - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> - qemu_co_sleep_ns__scheduled, NULL);
> + const char *scheduled = qatomic_cmpxchg(&co->scheduled,
> + qemu_co_sleep_ns__scheduled, NULL);
indentation fixed, but not over-80 line )
>
> assert(scheduled == qemu_co_sleep_ns__scheduled);
> - *sleep_state->user_state_pointer = NULL;
> - aio_co_wake(sleep_state->co);
> + aio_co_wake(co);
> }
> }
>
> static void co_sleep_cb(void *opaque)
> {
> - QemuCoSleepState **sleep_state = opaque;
> - qemu_co_sleep_wake(*sleep_state);
> + QemuCoSleep *w = opaque;
> + qemu_co_sleep_wake(w);
> }
>
> -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> - QemuCoSleepState **sleep_state)
> +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
> + QEMUClockType type, int64_t ns)
> {
> + Coroutine *co = qemu_coroutine_self();
> AioContext *ctx = qemu_get_current_aio_context();
> QEMUTimer ts;
> - QemuCoSleepState state = {
> - .co = qemu_coroutine_self(),
> - .user_state_pointer = sleep_state,
> - };
>
> - const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
> - qemu_co_sleep_ns__scheduled);
> + const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
> + qemu_co_sleep_ns__scheduled);
indentation fixed..
> if (scheduled) {
> fprintf(stderr,
> "%s: Co-routine was already scheduled in '%s'\n",
> @@ -62,12 +57,12 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> abort();
> }
>
> - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
> - *sleep_state = &state;
> + w->to_wake = co;
> + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
> timer_mod(&ts, qemu_clock_get_ns(type) + ns);
> qemu_coroutine_yield();
> timer_del(&ts);
>
> - /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */
> - assert(*sleep_state == NULL);
> + /* w->to_wake is cleared before resuming this coroutine. */
> + assert(w->to_wake == NULL);
> }
>
with indentations and over-80 line fixed in appropriate previous patch:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
` (4 preceding siblings ...)
2021-05-03 11:25 ` [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API Paolo Bonzini
@ 2021-05-03 11:25 ` Paolo Bonzini
2021-05-10 10:46 ` Vladimir Sementsov-Ogievskiy
2021-05-11 14:07 ` [PATCH 0/6] coroutine: new sleep/wake API Stefan Hajnoczi
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-05-03 11:25 UTC (permalink / raw)
To: qemu-devel; +Cc: eesposit, stefanha
Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
In the future we could introduce an API that can work even if the
sleep and wake happen from different threads. For now, initializing
w->to_wake after timer_mod is fine because the timer can only fire in
the same AioContext.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/coroutine.h | 5 +++++
util/qemu-coroutine-sleep.c | 20 +++++++++++++-------
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 77cb8ce459..b53e9632b9 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -302,6 +302,11 @@ typedef struct QemuCoSleep {
void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
QEMUClockType type, int64_t ns);
+/**
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
+ */
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
+
static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
{
QemuCoSleep w = { 0 };
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 89c3b758c5..df6edba2e4 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -41,12 +41,9 @@ static void co_sleep_cb(void *opaque)
qemu_co_sleep_wake(w);
}
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
- QEMUClockType type, int64_t ns)
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
{
Coroutine *co = qemu_coroutine_self();
- AioContext *ctx = qemu_get_current_aio_context();
- QEMUTimer ts;
const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
qemu_co_sleep_ns__scheduled);
@@ -58,11 +55,20 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
}
w->to_wake = co;
- aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
- timer_mod(&ts, qemu_clock_get_ns(type) + ns);
qemu_coroutine_yield();
- timer_del(&ts);
/* w->to_wake is cleared before resuming this coroutine. */
assert(w->to_wake == NULL);
}
+
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+ QEMUClockType type, int64_t ns)
+{
+ AioContext *ctx = qemu_get_current_aio_context();
+ QEMUTimer ts;
+
+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);
+ qemu_co_sleep(w);
+ timer_del(&ts);
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep
2021-05-03 11:25 ` [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep Paolo Bonzini
@ 2021-05-10 10:46 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-10 10:46 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: eesposit, stefanha
03.05.2021 14:25, Paolo Bonzini wrote:
> Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
> This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
>
> In the future we could introduce an API that can work even if the
> sleep and wake happen from different threads. For now, initializing
> w->to_wake after timer_mod is fine because the timer can only fire in
> the same AioContext.
worth a comment inside qemu_co_sleep_ns_wakeable() ?
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/6] coroutine: new sleep/wake API
2021-05-03 11:25 [PATCH 0/6] coroutine: new sleep/wake API Paolo Bonzini
` (5 preceding siblings ...)
2021-05-03 11:25 ` [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep Paolo Bonzini
@ 2021-05-11 14:07 ` Stefan Hajnoczi
6 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2021-05-11 14:07 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: eesposit, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
On Mon, May 03, 2021 at 01:25:44PM +0200, Paolo Bonzini wrote:
> This is a revamp of the qemu_co_sleep* API that makes it easier to
> extend the API: the state that is needed to wake up a coroutine is now
> part of the public API instead of hidden behind a pointer-to-pointer;
> the API is made more extensible by pushing the rest of QemuCoSleepState
> into local variables.
>
> In the future, this will be extended to introduce a prepare/sleep/cancel
> API similar to Linux's prepare_to_wait/schedule/finish_wait functions.
> For now, this is just a nice refactoring.
>
> Paolo Bonzini (6):
> coroutine-sleep: use a stack-allocated timer
> coroutine-sleep: disallow NULL QemuCoSleepState** argument
> coroutine-sleep: allow qemu_co_sleep_wake(NULL)
> coroutine-sleep: move timer out of QemuCoSleepState
> coroutine-sleep: replace QemuCoSleepState pointer with struct in the
> API
> coroutine-sleep: introduce qemu_co_sleep
>
> block/block-copy.c | 10 +++---
> block/nbd.c | 14 +++-----
> include/qemu/coroutine.h | 26 ++++++++------
> util/qemu-coroutine-sleep.c | 69 +++++++++++++++++--------------------
> 4 files changed, 57 insertions(+), 62 deletions(-)
>
> --
> 2.31.1
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread