From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60963) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOk4T-0003K9-6q for qemu-devel@nongnu.org; Thu, 28 Jan 2016 05:45:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOk4S-0002sZ-68 for qemu-devel@nongnu.org; Thu, 28 Jan 2016 05:45:01 -0500 References: <1453976119-24372-1-git-send-email-alex.bennee@linaro.org> <1453976119-24372-6-git-send-email-alex.bennee@linaro.org> From: Paolo Bonzini Message-ID: <56A9F120.4000501@redhat.com> Date: Thu, 28 Jan 2016 11:44:48 +0100 MIME-Version: 1.0 In-Reply-To: <1453976119-24372-6-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 5/5] thread-pool: atomic fixes from 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, qemu block , 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: > Mark changes in thread pool state as explicitly atomic. Also in the > test-thread-pool code make accesses to data.n explicitly atomic. >=20 > Signed-off-by: Alex Benn=C3=A9e > --- > tests/test-thread-pool.c | 8 ++++---- > thread-pool.c | 12 ++++++------ > 2 files changed, 10 insertions(+), 10 deletions(-) Acked-by: Paolo Bonzini > diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c > index ccdee39..f51e284 100644 > --- a/tests/test-thread-pool.c > +++ b/tests/test-thread-pool.c > @@ -46,10 +46,10 @@ static void test_submit(void) > { > WorkerTestData data =3D { .n =3D 0 }; > thread_pool_submit(pool, worker_cb, &data); > - while (data.n =3D=3D 0) { > + while (atomic_read(&data.n) =3D=3D 0) { > aio_poll(ctx, true); > } > - g_assert_cmpint(data.n, =3D=3D, 1); > + g_assert_cmpint(atomic_read(&data.n), =3D=3D, 1); > } > =20 > static void test_submit_aio(void) > @@ -128,7 +128,7 @@ static void test_submit_many(void) > aio_poll(ctx, true); > } > for (i =3D 0; i < 100; i++) { > - g_assert_cmpint(data[i].n, =3D=3D, 1); > + g_assert_cmpint(atomic_read(&data[i].n), =3D=3D, 1); > g_assert_cmpint(data[i].ret, =3D=3D, 0); > } > } > @@ -183,7 +183,7 @@ static void do_test_cancel(bool sync) > g_assert_cmpint(num_canceled, <, 100); > =20 > for (i =3D 0; i < 100; i++) { > - if (data[i].aiocb && data[i].n !=3D 3) { > + if (data[i].aiocb && (atomic_read(&data[i].n) !=3D 3)) { > if (sync) { > /* Canceling the others will be a blocking operation. = */ > bdrv_aio_cancel(data[i].aiocb); > diff --git a/thread-pool.c b/thread-pool.c > index 402c778..431a6fb 100644 > --- a/thread-pool.c > +++ b/thread-pool.c > @@ -99,15 +99,15 @@ static void *worker_thread(void *opaque) > =20 > req =3D QTAILQ_FIRST(&pool->request_list); > QTAILQ_REMOVE(&pool->request_list, req, reqs); > - req->state =3D THREAD_ACTIVE; > + atomic_set(&req->state, THREAD_ACTIVE); > qemu_mutex_unlock(&pool->lock); > =20 > ret =3D req->func(req->arg); > =20 > - req->ret =3D ret; > + atomic_set(&req->ret, ret); > /* Write ret before state. */ > smp_wmb(); > - req->state =3D THREAD_DONE; > + atomic_set(&req->state, THREAD_DONE); > =20 > qemu_mutex_lock(&pool->lock); > =20 > @@ -167,7 +167,7 @@ static void thread_pool_completion_bh(void *opaque) > =20 > restart: > QLIST_FOREACH_SAFE(elem, &pool->head, all, next) { > - if (elem->state !=3D THREAD_DONE) { > + if (atomic_read(&elem->state) !=3D THREAD_DONE) { > continue; > } > =20 > @@ -184,7 +184,7 @@ restart: > */ > qemu_bh_schedule(pool->completion_bh); > =20 > - elem->common.cb(elem->common.opaque, elem->ret); > + elem->common.cb(elem->common.opaque, atomic_read(&elem->re= t)); > qemu_aio_unref(elem); > goto restart; > } else { > @@ -201,7 +201,7 @@ static void thread_pool_cancel(BlockAIOCB *acb) > trace_thread_pool_cancel(elem, elem->common.opaque); > =20 > qemu_mutex_lock(&pool->lock); > - if (elem->state =3D=3D THREAD_QUEUED && > + if (atomic_read(&elem->state) =3D=3D THREAD_QUEUED && > /* No thread has yet started working on elem. we can try to "s= teal" > * the item from the worker if we can get a signal from the > * semaphore. Because this is non-blocking, we can do it with >=20