@samitouri / QOSamiQemu / commits / e554413bd2

coroutine: fix lost wakeup in qemu_co_sleep_wake()

cache_clean_timer_del_and_wait() cancels the cache-cleaner coroutine by setting s->cache_clean_interval = 0 and calling qemu_co_sleep_wake() to cut short its qemu_co_sleep_ns_wakeable(). qemu_co_sleep_wake() is fire-and-forget: it reads w->to_wake and silently returns when it is NULL. A sleeper that is between two iterations -- has just released s->lock but has not yet set w->to_wake inside qemu_co_sleep() -- loses the wake: iothread0 timer coroutine main thread (qcow2 close) ------------------------- ------------------------- while-body (holding s->lock): read interval = 600 wait_ns = 600 * NS release s->lock take s->lock interval = 0 qemu_co_sleep_wake(w): w->to_wake == NULL -> skip return qemu_co_queue_wait(exit, s->lock): release s->lock yield qemu_co_sleep_ns_wakeable: aio_timer_init(+600 s) qemu_co_sleep: cas scheduled NULL -> "qsns" w->to_wake = co yield [sleeps 600 s] cache_clean_timer_del_and_wait() then blocks on cache_clean_timer_exit until the original 600 s expiry fires, and qcow2_close() holds BQL the whole time so the VM stalls behind it. block_copy_kick() has the same shape. Fix the primitive once instead of working around it in each caller. Use a tri-state for QemuCoSleep::to_wake: NULL - idle co - sleeper parked PENDING - wake delivered, no sleeper yet (sticky) qemu_co_sleep_wake() xchgs PENDING into to_wake: a real sleeper is woken, NULL/PENDING is left untouched so the wake stays sticky. qemu_co_sleep() cmpxchg-publishes itself as the sleeper; if a wake was delivered before it got there or races the publish, the cmpxchg observes PENDING and returns without yielding. On normal resume qemu_co_sleep() clears the PENDING the waker left behind so the next sleep starts clean. A double-fire (real wake plus timer callback) is harmless: the first xchg returns the coroutine and wakes it; the second returns PENDING and is a no-op. Cancellation latency through qemu_co_sleep_wake() is now bounded by aio_co_wake() rather than by the sleep duration. Fixes: f86dde9a15 ("qcow2: Fix cache_clean_timer") Signed-off-by: Denis V. Lunev <den@openvz.org> Cc: Hanna Czenczek <hreitz@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Message-ID: <20260610115850.2410566-2-den@openvz.org> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Denis V. Lunev committed Jun 10, 2026 at 13:58 UTC e554413bd2b4d6e1f31eebfb499bce5238b91646

3 files changed +104 -19

include/qemu/coroutine.h

+13 -4

@@ -260,10 +260,19 @@ int coroutine_fn qemu_co_timeout(CoroutineEntry *entry, void *opaque,

260 uint64_t timeout_ns, CleanupFunc clean);

261

262 /**

263 - * Wake a coroutine if it is sleeping in qemu_co_sleep_ns. The timer will be

264 - * deleted. @sleep_state must be the variable whose address was given to

265 - * qemu_co_sleep_ns() and should be checked to be non-NULL before calling

266 - * qemu_co_sleep_wake().

263 + * Wake a coroutine sleeping in qemu_co_sleep() or qemu_co_sleep_ns_wakeable().

264 + * The timer set up by the latter is deleted on wakeup.

265 + *

266 + * The wake is sticky: if no sleeper is parked on @w at the time of the call,

267 + * the wake is recorded on @w and consumed by the next qemu_co_sleep() on the

268 + * same @w, which then returns without yielding. This closes the lost-wakeup

269 + * window between two sleeps and is the documented behavior callers should

270 + * rely on -- e.g. a cancellation signal raised between iterations of a

271 + * sleep/work loop will shorten the next sleep instead of being dropped.

272 + *

273 + * The state persists until consumed: if no further qemu_co_sleep() is ever

274 + * called on @w, the pending wake is harmlessly discarded when @w goes away.

275 + * Multiple wakes coalesce -- the next sleep consumes at most one.

276 */

277 void qemu_co_sleep_wake(QemuCoSleep *w);

278

tests/unit/test-coroutine.c

+53

@@ -421,6 +421,57 @@ static void test_co_rwlock_downgrade(void)

421 g_assert(c1_done);

422 }

423

424 +/*

425 + * Check that a wake delivered before the sleeper parks is not lost.

426 + *

427 + * qemu_co_sleep_wake() is fire-and-forget: a caller cancelling a

428 + * sleep/work loop may call it in the window after the sleeper has

429 + * decided to sleep but before it has published itself inside

430 + * qemu_co_sleep(). The wake must be sticky and shorten the next sleep

431 + * rather than being dropped (which would block until the full sleep

432 + * duration expired).

433 + *

434 + * No threads, timers or AioContext are needed: coroutines are

435 + * cooperative, so ordering the wake before the sleep deterministically

436 + * reproduces the state the racing waker would otherwise produce.

437 + */

438 +

439 +typedef struct {

440 + QemuCoSleep w;

441 + bool completed;

442 +} CoSleepWakeData;

443 +

444 +static void coroutine_fn co_sleep_wake_entry(void *opaque)

445 +{

446 + CoSleepWakeData *d = opaque;

447 +

448 + /*

449 + * The wake was already delivered before we got here. qemu_co_sleep()

450 + * must consume it and return without yielding.

451 + */

452 + qemu_co_sleep(&d->w);

453 + d->completed = true;

454 +}

455 +

456 +static void test_co_sleep_wake_before_sleep(void)

457 +{

458 + CoSleepWakeData d = { .w = { 0 }, .completed = false };

459 + Coroutine *co = qemu_coroutine_create(co_sleep_wake_entry, &d);

460 +

461 + /* Waker runs first, while no sleeper is parked on w. */

462 + qemu_co_sleep_wake(&d.w);

463 +

464 + /*

465 + * Entering runs qemu_co_sleep(), which consumes the pending wake and

466 + * returns without yielding, so the coroutine runs straight to

467 + * completion in this single enter. With the pre-fix primitive the wake

468 + * is dropped, qemu_co_sleep() parks, and completed stays false.

469 + */

470 + qemu_coroutine_enter(co);

471 +

472 + g_assert(d.completed);

473 +}

474 +

475 /*

476 * Check that creation, enter, and return work

477 */

@@ -660,6 +711,8 @@ int main(int argc, char **argv)

711 g_test_add_func("/locking/co-mutex/lockable", test_co_mutex_lockable);

712 g_test_add_func("/locking/co-rwlock/upgrade", test_co_rwlock_upgrade);

713 g_test_add_func("/locking/co-rwlock/downgrade", test_co_rwlock_downgrade);

714 + g_test_add_func("/locking/co-sleep/wake-before-sleep",

715 + test_co_sleep_wake_before_sleep);

716 if (g_test_perf()) {

717 g_test_add_func("/perf/lifecycle", perf_lifecycle);

718 g_test_add_func("/perf/nesting", perf_nesting);

util/qemu-coroutine-sleep.c

+38 -15

@@ -18,20 +18,29 @@

19 static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";

21 +/*

22 + * Sentinel stored in QemuCoSleep::to_wake by qemu_co_sleep_wake() when no

23 + * sleeper has parked yet. The next qemu_co_sleep() consumes it and returns

24 + * without yielding, so a wake that races the arming of a sleep is never

25 + * lost.

26 + */

27 +#define QEMU_CO_SLEEP_PENDING ((Coroutine *)(uintptr_t)1)

28 +

29 void qemu_co_sleep_wake(QemuCoSleep *w)

30 {

31 Coroutine *co;

25 - co = w->to_wake;

26 - w->to_wake = NULL;

27 - if (co) {

28 - /* Write of schedule protected by barrier write in aio_co_schedule */

29 - const char *scheduled = qatomic_cmpxchg(&co->scheduled,

30 - qemu_co_sleep_ns__scheduled, NULL);

31 -

32 - assert(scheduled == qemu_co_sleep_ns__scheduled);

33 - aio_co_wake(co);

33 + co = qatomic_xchg(&w->to_wake, QEMU_CO_SLEEP_PENDING);

34 + if (co == NULL || co == QEMU_CO_SLEEP_PENDING) {

35 + /* No sleeper, or a wake is already pending. */

36 + return;

37 }

38 +

39 + /* Write of scheduled protected by barrier write in aio_co_schedule */

40 + const char *scheduled = qatomic_cmpxchg(&co->scheduled,

41 + qemu_co_sleep_ns__scheduled, NULL);

42 + assert(scheduled == qemu_co_sleep_ns__scheduled);

43 + aio_co_wake(co);

44 }

46 static void co_sleep_cb(void *opaque)

@@ -43,6 +52,7 @@ static void co_sleep_cb(void *opaque)

52 void coroutine_fn qemu_co_sleep(QemuCoSleep *w)

53 {

54 Coroutine *co = qemu_coroutine_self();

55 + Coroutine *prev;

57 const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,

58 qemu_co_sleep_ns__scheduled);

@@ -53,11 +63,23 @@ void coroutine_fn qemu_co_sleep(QemuCoSleep *w)

63 abort();

64 }

56 - w->to_wake = co;

66 + /*

67 + * Publish ourselves as the sleeper. A wake delivered before we got here,

68 + * or one racing this publish, leaves QEMU_CO_SLEEP_PENDING in to_wake;

69 + * the cmpxchg then fails and we consume the wake without yielding.

70 + */

71 + prev = qatomic_cmpxchg(&w->to_wake, NULL, co);

72 + if (prev == QEMU_CO_SLEEP_PENDING) {

73 + qatomic_set(&w->to_wake, NULL);

74 + qatomic_set(&co->scheduled, NULL);

75 + return;

76 + }

77 + assert(prev == NULL);

78 +

79 qemu_coroutine_yield();

59 - /* w->to_wake is cleared before resuming this coroutine. */

60 - assert(w->to_wake == NULL);

81 + /* The waker left QEMU_CO_SLEEP_PENDING; clear it for the next sleep. */

82 + qatomic_set(&w->to_wake, NULL);

83 }

85 void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

@@ -70,9 +92,10 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

92 timer_mod(&ts, qemu_clock_get_ns(type) + ns);

94 /*

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

74 - * must happen after qemu_co_sleep yields and there is no race

75 - * between timer_mod and qemu_co_sleep.

95 + * A wake racing with the arming of the sleep -- including the timer

96 + * we just armed firing in another AioContext before qemu_co_sleep()

97 + * publishes itself -- is captured by the sticky PENDING state in

98 + * qemu_co_sleep_wake() and consumed here without yielding.

99 */

100 qemu_co_sleep(w);

101 timer_del(&ts);