From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33001) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOk5R-0004FO-1q for qemu-devel@nongnu.org; Thu, 28 Jan 2016 05:46:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOk5Q-0003Np-3M for qemu-devel@nongnu.org; Thu, 28 Jan 2016 05:46:00 -0500 References: <1453976119-24372-1-git-send-email-alex.bennee@linaro.org> <1453976119-24372-5-git-send-email-alex.bennee@linaro.org> From: Paolo Bonzini Message-ID: <56A9F15D.5080604@redhat.com> Date: Thu, 28 Jan 2016 11:45:49 +0100 MIME-Version: 1.0 In-Reply-To: <1453976119-24372-5-git-send-email-alex.bennee@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 4/5] async.c: various atomic fixes for tsan List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= , qemu-devel@nongnu.org Cc: mttcg@greensocs.com, peter.maydell@linaro.org, "open list:Block I/O path" , mark.burton@greensocs.com, a.rigo@virtualopensystems.com, stefanha@redhat.com, fred.konrad@greensocs.com On 28/01/2016 11:15, Alex Benn=C3=A9e wrote: > Running "make check" under ThreadSanitizer pointed to a number of > potential races. >=20 > - use atomic_mb_read to check bh->schedule > - use atomic_set/read primitives to manipulate bh->idle >=20 > The bh->idle changes are mostly for the benefit of explicit marking for > tsan but the code in non-sanitised builds should be the same. These are harmless because of the aio_notify calls that happen after bh->scheduled is written, but they do make the code easier to understand. Acked-by: Paolo Bonzini > Signed-off-by: Alex Benn=C3=A9e > --- > async.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) >=20 > diff --git a/async.c b/async.c > index e106072..a9cb9c3 100644 > --- a/async.c > +++ b/async.c > @@ -85,10 +85,10 @@ int aio_bh_poll(AioContext *ctx) > */ > if (!bh->deleted && atomic_xchg(&bh->scheduled, 0)) { > /* Idle BHs and the notify BH don't count as progress */ > - if (!bh->idle && bh !=3D ctx->notify_dummy_bh) { > + if (!atomic_read(&bh->idle) && bh !=3D ctx->notify_dummy_b= h) { > ret =3D 1; > } > - bh->idle =3D 0; > + atomic_set(&bh->idle, 0); > aio_bh_call(bh); > } > } > @@ -128,7 +128,7 @@ void qemu_bh_schedule(QEMUBH *bh) > AioContext *ctx; > =20 > ctx =3D bh->ctx; > - bh->idle =3D 0; > + atomic_set(&bh->idle, 0); > /* The memory barrier implicit in atomic_xchg makes sure that: > * 1. idle & any writes needed by the callback are done before the > * locations are read in the aio_bh_poll. > @@ -165,8 +165,8 @@ aio_compute_timeout(AioContext *ctx) > QEMUBH *bh; > =20 > for (bh =3D ctx->first_bh; bh; bh =3D bh->next) { > - if (!bh->deleted && bh->scheduled) { > - if (bh->idle) { > + if (!bh->deleted && atomic_mb_read(&bh->scheduled)) { > + if (atomic_read(&bh->idle)) { > /* idle bottom halves will be polled at least > * every 10ms */ > timeout =3D 10000000; > @@ -286,7 +286,7 @@ void aio_notify(AioContext *ctx) > * with atomic_or in aio_ctx_prepare or atomic_add in aio_poll. > */ > smp_mb(); > - if (ctx->notify_me) { > + if (atomic_read(&ctx->notify_me)) { > event_notifier_set(&ctx->notifier); > atomic_mb_set(&ctx->notified, true); > } >=20