On Thu, 6 Apr 2023 at 06:09, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Replace with an explicit barrier and a comment.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> util/qemu-coroutine.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index 849452369201..17a88f65053e 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -127,9 +127,13 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
> Coroutine *to = QSIMPLEQ_FIRST(&pending);
> CoroutineAction ret;
>
> - /* Cannot rely on the read barrier for to in aio_co_wake(), as there are
> - * callers outside of aio_co_wake() */
> - const char *scheduled = qatomic_mb_read(&to->scheduled);
> + /*
> + * Read to before to->scheduled; pairs with qatomic_cmpxchg in
> + * qemu_co_sleep(), aio_co_schedule() etc.
> + */
> + smp_read_barrier_depends();
I'm not a fan of nuanced memory ordering primitives. I don't
understand or remember all the primitives available in
docs/devel/atomics.rst and especially not how they interact with each
other.
Understood, that's why I want to remove qatomic_mb_read().
Does smp_read_barrier_depends() make sense for QEMU? Does QEMU support
Alpha host CPUs?
It makes sense in that it's cheaper than qatomic_load_acquire() or smp_rmb() on ARM and PPC (32-bit ARM is especially bad). Here I can use smp_rmb() if you prefer; I thought that the comment, explicitly referring to "to->scheduled" which depends on "to", would be enough.
I could also use QSIMPLEQ_FIRST_RCU(&pending) to hide the barrier, but it seems to be a bad idea because there's no RCU involvement here.
Paolo