Re: [PATCH] aio_wait_kick: add missing memory barrier

[Paolo Bonzini]

On 6/4/22 14:51, Roman Kagan wrote:
> On Tue, May 24, 2022 at 01:30:54PM -0400, Emanuele Giuseppe Esposito wrote:
> > It seems that aio_wait_kick always required a memory barrier
> > or atomic operation in the caller, but nobody actually
> > took care of doing it.
> >
> > Let's put the barrier in the function instead, and pair it
> > with another one in AIO_WAIT_WHILE. Read aio_wait_kick()
> > comment for further explanation.
> >
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> > ---
> >   include/block/aio-wait.h |  2 ++
> >   util/aio-wait.c          | 16 +++++++++++++++-
> >   2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
> > index b39eefb38d..54840f8622 100644
> > --- a/include/block/aio-wait.h
> > +++ b/include/block/aio-wait.h
> > @@ -81,6 +81,8 @@ extern AioWait global_aio_wait;
> >       AioContext *ctx_ = (ctx);                                      \
> >       /* Increment wait_->num_waiters before evaluating cond. */     \
> >       qatomic_inc(&wait_->num_waiters);                              \
> > +    /* Paired with smp_mb in aio_wait_kick(). */                   \
> > +    smp_mb();                                                      \
>
> IIRC qatomic_inc() ensures sequential consistency, isn't it enough here?

Nope, it only ensures sequential consistency with other SEQ_CST 
operations, i.e. not with qatomic_read or qatomic_set. :(

The smp_mb() is needed on ARM, in particular.


Paolo