On 02/10/19 11:23, Jan Glauber wrote: > I've looked into this on ThunderX2. The arm64 code generated for the > atomic_[add|sub] accesses of ctx->notify_me doesn't contain any > memory barriers. It is just plain ldaxr/stlxr. > > From my understanding this is not sufficient for SMP sync. > > If I read this comment correct: > > void aio_notify(AioContext *ctx) > { > /* Write e.g. bh->scheduled before reading ctx->notify_me. Pairs > * with atomic_or in aio_ctx_prepare or atomic_add in aio_poll. > */ > smp_mb(); > if (ctx->notify_me) { > > it points out that the smp_mb() should be paired. But as > I said the used atomics don't generate any barriers at all.
Based on the rest of the thread, this patch should also fix the bug: diff --git a/util/async.c b/util/async.c index 47dcbfa..721ea53 100644 --- a/util/async.c +++ b/util/async.c @@ -249,7 +249,7 @@ aio_ctx_check(GSource *source) aio_notify_accept(ctx); for (bh = ctx->first_bh; bh; bh = bh->next) { - if (bh->scheduled) { + if (atomic_mb_read(&bh->scheduled)) { return true; } } And also the memory barrier in aio_notify can actually be replaced with a SEQ_CST load: diff --git a/util/async.c b/util/async.c index 47dcbfa..721ea53 100644 --- a/util/async.c +++ b/util/async.c @@ -349,11 +349,11 @@ LinuxAioState *aio_get_linux_aio(AioContext *ctx) void aio_notify(AioContext *ctx) { - /* Write e.g. bh->scheduled before reading ctx->notify_me. Pairs - * with atomic_or in aio_ctx_prepare or atomic_add in aio_poll. + /* Using atomic_mb_read ensures that e.g. bh->scheduled is written before + * ctx->notify_me is read. Pairs with atomic_or in aio_ctx_prepare or + * atomic_add in aio_poll. */ - smp_mb(); - if (ctx->notify_me) { + if (atomic_mb_read(&ctx->notify_me)) { event_notifier_set(&ctx->notifier); atomic_mb_set(&ctx->notified, true); } Would you be able to test these (one by one possibly)? > I've tried to verify me theory with this patch and didn't run into the > issue for ~500 iterations (usually I would trigger the issue ~20 iterations). Sorry for asking the obvious---500 iterations of what? Paolo