The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95:

Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100)

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21:

coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100)

Pull request

(Resent due to an email preparation mistake.)

Paolo Bonzini (6):

coroutine-sleep: use a stack-allocated timer

coroutine-sleep: disallow NULL QemuCoSleepState** argument

coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing

coroutine-sleep: move timer out of QemuCoSleepState

coroutine-sleep: replace QemuCoSleepState pointer with struct in the

API

coroutine-sleep: introduce qemu_co_sleep

Philippe Mathieu-Daudé (1):

bitops.h: Improve find_xxx_bit() documentation

Zenghui Yu (1):

multi-process: Initialize variables declared with g_auto*

include/qemu/bitops.h | 15 ++++++--

include/qemu/coroutine.h | 27 ++++++++-----

block/block-copy.c | 10 ++---

block/nbd.c | 14 +++----

hw/remote/memory.c | 5 +--

hw/remote/proxy.c | 3 +-

util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------

7 files changed, 79 insertions(+), 70 deletions(-)

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the code by removing conditionals. qemu_co_sleep_ns

can simply point the argument to an on-stack temporary.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-3-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@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

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ 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.

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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

From: Paolo Bonzini <pbonzini@redhat.com>

Right now, users of qemu_co_sleep_ns_wakeable are simply 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.

Since the usage is changed, take the occasion to rename the

struct to QemuCoSleep.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-6-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/coroutine.h | 23 +++++++++++----------

block/block-copy.c | 8 ++++----

block/nbd.c | 10 ++++-----

util/qemu-coroutine-sleep.c | 41 ++++++++++++++++---------------------

4 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ 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. Initializes @w so that,

+ * during this yield, it can be passed to 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);

}

/**

@@ -XXX,XX +XXX,XX @@ 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/block/block-copy.c b/block/block-copy.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-copy.c

+++ b/block/block-copy.c

@@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState {

/* State */

int ret;

bool finished;

- QemuCoSleepState *sleep_state;

+ QemuCoSleep sleep;

bool cancelled;

/* OUT parameters */

@@ -XXX,XX +XXX,XX @@ 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;

}

}

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/block/nbd.c

+++ b/block/nbd.c

@@ -XXX,XX +XXX,XX @@ 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;

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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()) {

@@ -XXX,XX +XXX,XX @@ 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/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@

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,

+ 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",

@@ -XXX,XX +XXX,XX @@ 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

From: Paolo Bonzini <pbonzini@redhat.com>

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>

Message-id: 20210517100548.28806-2-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@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

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";

struct QemuCoSleepState {

Coroutine *co;

- QEMUTimer *ts;

+ QEMUTimer ts;

QemuCoSleepState **user_state_pointer;

};

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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,

};

@@ -XXX,XX +XXX,XX @@ 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) {

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

*/

assert(*sleep_state == NULL);

}

- timer_free(state.ts);

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

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 will 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>

Message-id: 20210517100548.28806-7-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

include/qemu/coroutine.h | 5 +++++

util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------

2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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);

+

+ /*

+ * The timer will fire in the current AiOContext, so the callback

+ * must happen after qemu_co_sleep yields and there is no race

+ * between timer_mod and qemu_co_sleep.

+ */

+ qemu_co_sleep(w);

+ timer_del(&ts);

+}

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

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. However, this

would not be safe unless co_sleep_cb keeps using the QemuCoSleepState*

directly, so make it go through the pointer-to-pointer instead.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-4-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@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

--- a/block/block-copy.c

+++ b/block/block-copy.c

@@ -XXX,XX +XXX,XX @@ 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);

}