From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: mttcg@listserver.greensocs.com, peter.maydell@linaro.org,
"open list:Block I/O path" <qemu-block@nongnu.org>,
mark.burton@greensocs.com, a.rigo@virtualopensystems.com,
"Stefan Hajnoczi" <stefanha@redhat.com>,
pbonzini@redhat.com, "Alex Bennée" <alex.bennee@linaro.org>,
fred.konrad@greensocs.com
Subject: [Qemu-devel] [RFC PATCH 4/4] tsan: various fixes for make check
Date: Mon, 25 Jan 2016 16:49:18 +0000 [thread overview]
Message-ID: <1453740558-16303-5-git-send-email-alex.bennee@linaro.org> (raw)
In-Reply-To: <1453740558-16303-1-git-send-email-alex.bennee@linaro.org>
After building with the ThreadSanitizer I ran make check and started
going through the failures reported. Most are failures to use atomic
primitives to access variables previously atomically set. While this
likely will work on x86 it could cause problems on other architectures.
- async: use atomic reads for scheduled/notify_me
- thread-pool: use atomic_mb_read/set to for thread ->state
- test-thread-pool: use atomic read for data.n
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
async.c | 4 ++--
tests/test-thread-pool.c | 2 +-
thread-pool.c | 9 ++++-----
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/async.c b/async.c
index e106072..8d5f810 100644
--- a/async.c
+++ b/async.c
@@ -165,7 +165,7 @@ aio_compute_timeout(AioContext *ctx)
QEMUBH *bh;
for (bh = ctx->first_bh; bh; bh = bh->next) {
- if (!bh->deleted && bh->scheduled) {
+ if (!bh->deleted && atomic_read(&bh->scheduled)) {
if (bh->idle) {
/* idle bottom halves will be polled at least
* every 10ms */
@@ -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);
}
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index ccdee39..5694ad9 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -46,7 +46,7 @@ static void test_submit(void)
{
WorkerTestData data = { .n = 0 };
thread_pool_submit(pool, worker_cb, &data);
- while (data.n == 0) {
+ while (atomic_read(&data.n) == 0) {
aio_poll(ctx, true);
}
g_assert_cmpint(data.n, ==, 1);
diff --git a/thread-pool.c b/thread-pool.c
index 402c778..97b2c0c 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -99,15 +99,14 @@ static void *worker_thread(void *opaque)
req = QTAILQ_FIRST(&pool->request_list);
QTAILQ_REMOVE(&pool->request_list, req, reqs);
- req->state = THREAD_ACTIVE;
+ atomic_mb_set(&req->state, THREAD_ACTIVE);
qemu_mutex_unlock(&pool->lock);
ret = req->func(req->arg);
req->ret = ret;
/* Write ret before state. */
- smp_wmb();
- req->state = THREAD_DONE;
+ atomic_mb_set(&req->state, THREAD_DONE);
qemu_mutex_lock(&pool->lock);
@@ -167,7 +166,7 @@ static void thread_pool_completion_bh(void *opaque)
restart:
QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
- if (elem->state != THREAD_DONE) {
+ if (atomic_read(&elem->state) != THREAD_DONE) {
continue;
}
@@ -201,7 +200,7 @@ static void thread_pool_cancel(BlockAIOCB *acb)
trace_thread_pool_cancel(elem, elem->common.opaque);
qemu_mutex_lock(&pool->lock);
- if (elem->state == THREAD_QUEUED &&
+ if (atomic_mb_read(&elem->state) == THREAD_QUEUED &&
/* No thread has yet started working on elem. we can try to "steal"
* the item from the worker if we can get a signal from the
* semaphore. Because this is non-blocking, we can do it with
--
2.7.0
next prev parent reply other threads:[~2016-01-25 16:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 16:49 [Qemu-devel] [RFC PATCH 0/4] ThreadSanitizer support Alex Bennée
2016-01-25 16:49 ` [Qemu-devel] [RFC PATCH 1/4] configure: move EXTRA_CFLAGS append to the end Alex Bennée
2016-01-25 17:04 ` Peter Maydell
2016-01-25 17:37 ` Peter Maydell
2016-01-25 16:49 ` [Qemu-devel] [RFC PATCH 2/4] configure: introduce --extra-libs Alex Bennée
2016-01-25 17:08 ` Peter Maydell
2016-01-25 17:25 ` Alex Bennée
2016-01-25 17:36 ` Peter Maydell
2016-01-25 17:58 ` Paolo Bonzini
2016-01-25 18:15 ` Alex Bennée
2016-01-25 22:10 ` Paolo Bonzini
2016-01-26 8:43 ` Alex Bennée
2016-01-25 16:49 ` [Qemu-devel] [RFC PATCH 3/4] include/qemu/atomic.h: default to __atomic functions Alex Bennée
2016-01-25 18:04 ` Paolo Bonzini
2016-01-25 18:23 ` Alex Bennée
2016-01-25 22:02 ` Paolo Bonzini
2016-01-25 16:49 ` Alex Bennée [this message]
2016-01-25 18:09 ` [Qemu-devel] [RFC PATCH 4/4] tsan: various fixes for make check Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1453740558-16303-5-git-send-email-alex.bennee@linaro.org \
--to=alex.bennee@linaro.org \
--cc=a.rigo@virtualopensystems.com \
--cc=fred.konrad@greensocs.com \
--cc=mark.burton@greensocs.com \
--cc=mttcg@listserver.greensocs.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).