On Fri, Oct 11, 2019 at 06:05:25AM +0000, Jan Glauber wrote: > On Wed, Oct 09, 2019 at 11:15:04AM +0200, Paolo Bonzini wrote: > > On 09/10/19 10:02, Jan Glauber wrote: > > > > I'm still not sure what the actual issue is here, but could it be some bad > > > interaction between the notify_me and the list_lock? The are both 4 byte > > > and side-by-side: > > > > > > address notify_me: 0xaaaadb528aa0 sizeof notify_me: 4 > > > address list_lock: 0xaaaadb528aa4 sizeof list_lock: 4 > > > > > > AFAICS the generated code looks OK (all load/store exclusive done > > > with 32 bit size): > > > > > > e6c: 885ffc01 ldaxr w1, [x0] > > > e70: 11000821 add w1, w1, #0x2 > > > e74: 8802fc01 stlxr w2, w1, [x0] > > > > > > ...but if I bump notify_me size to uint64_t the issue goes away. > > > > Ouch. :) Is this with or without my patch(es)? > > > > Also, what if you just add a dummy uint32_t after notify_me? > > With the dummy the testcase also runs fine for 500 iterations. > > Dann, can you try if this works on the Hi1620 too?
On Hi1620, it hung on the first iteration. Here's the complete patch I'm running with: diff --git a/include/block/aio.h b/include/block/aio.h index 6b0d52f732..e6fd6f1a1a 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -82,7 +82,7 @@ struct AioContext { * Instead, the aio_poll calls include both the prepare and the * dispatch phase, hence a simple counter is enough for them. */ - uint32_t notify_me; + uint64_t notify_me; /* A lock to protect between QEMUBH and AioHandler adders and deleter, * and to ensure that no callbacks are removed while we're walking and diff --git a/util/async.c b/util/async.c index ca83e32c7f..024c4c567d 100644 --- a/util/async.c +++ b/util/async.c @@ -242,7 +242,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; } } @@ -342,12 +342,12 @@ 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) { - event_notifier_set(&ctx->notifier); + if (atomic_mb_read(&ctx->notify_me)) { + event_notifier_set(&ctx->notifier); atomic_mb_set(&ctx->notified, true); } }