[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v8 1/3] qemu-coroutine-sleep: introduce qemu_co_
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v8 1/3] qemu-coroutine-sleep: introduce qemu_co_sleep_wake |
Date: |
Tue, 17 Sep 2019 17:27:56 +0200 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
Am 21.08.2019 um 18:52 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Introduce a function to gracefully wake a coroutine sleeping in
> qemu_co_sleep_ns().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Hm, this has become a bit more complex with the new sleep_state, but
it's probably unavoidable even we need to timer_del() from the callback.
> include/qemu/coroutine.h | 17 ++++++++++++--
> block/null.c | 2 +-
> block/sheepdog.c | 2 +-
> tests/test-bdrv-drain.c | 6 ++---
> tests/test-block-iothread.c | 2 +-
> util/qemu-coroutine-sleep.c | 47 +++++++++++++++++++++++++++----------
> 6 files changed, 55 insertions(+), 21 deletions(-)
>
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index 9801e7f5a4..96780a4902 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -274,9 +274,22 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
> void qemu_co_rwlock_unlock(CoRwlock *lock);
>
> /**
> - * Yield the coroutine for a given duration
> + * Yield the coroutine for a given duration. During this yield @sleep_state
> (if
> + * not NULL) is set to opaque pointer, which may be used for
> + * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when
> timer
> + * shoots. Don't save obtained value to other variables and don't call
> + * qemu_co_sleep_wake from another aio context.
> */
> -void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns);
> +void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns,
> + void **sleep_state);
Let's make the typedef for QemuCoSleepState public so that we don't have
to use a void pointer here.
I wonder if it would make sense to rename this function and keep
qemu_co_sleep_ns() as a wrapper (maybe even just macro) so that most
callers don't have to add a NULL.
> +/**
> + * Wake a coroutine if it is sleeping by qemu_co_sleep_ns. Timer will be
> + * deleted. @sleep_state must be the variable which address was given to
> + * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
> + * qemu_co_sleep_wake().
> + */
> +void qemu_co_sleep_wake(void *sleep_state);
>
> /**
> * Yield until a file descriptor becomes readable
The actual implementation looks right to me, so with a public
QemuCoSleepState and optionally the wrapper:
Reviewed-by: Kevin Wolf <address@hidden>
- Re: [Qemu-block] [PATCH v8 1/3] qemu-coroutine-sleep: introduce qemu_co_sleep_wake,
Kevin Wolf <=