| Message ID | 20200522160755.886-16-robert.foley@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Thread Sanitizer support to QEMU | expand |
On Fri, May 22, 2020 at 12:07:51 -0400, Robert Foley wrote: > For example: > WARNING: ThreadSanitizer: data race (pid=23406) > Atomic read of size 4 at 0x7b100003e3c8 by thread T7: > #0 __tsan_atomic32_load <null> (qemu-system-aarch64+0x39a36c) > #1 qht_do_lookup util/qht.c:495:17 (qemu-system-aarch64+0xd82f7a) > #2 qht_lookup_custom util/qht.c:539:11 (qemu-system-aarch64+0xd82f7a) > Previous write of size 8 at 0x7b100003e3c8 by thread T6 (mutexes: write M166769147697783108, write M995435858420506688): > #0 posix_memalign <null> (qemu-system-aarch64+0x350dd1) > #1 qemu_try_memalign util/oslib-posix.c:189:11 (qemu-system-aarch64+0xd59317) > #2 qemu_memalign util/oslib-posix.c:205:27 (qemu-system-aarch64+0xd5943e) > #3 qht_insert__locked util/qht.c:583:9 (qemu-system-aarch64+0xd837c5) > > Signed-off-by: Robert Foley <robert.foley@linaro.org> > --- > util/qht.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/util/qht.c b/util/qht.c > index 67e5d5b916..739a53ced0 100644 > --- a/util/qht.c > +++ b/util/qht.c > @@ -69,6 +69,7 @@ > #include "qemu/qht.h" > #include "qemu/atomic.h" > #include "qemu/rcu.h" > +#include "qemu/tsan.h" > > //#define QHT_DEBUG > > @@ -580,10 +581,12 @@ static void *qht_insert__locked(const struct qht *ht, struct qht_map *map, > b = b->next; > } while (b); > > + TSAN_ANNOTATE_IGNORE_WRITES_BEGIN(); > b = qemu_memalign(QHT_BUCKET_ALIGN, sizeof(*b)); > memset(b, 0, sizeof(*b)); > new = b; > i = 0; > + TSAN_ANNOTATE_IGNORE_WRITES_END(); I cannot reproduce this warning post-series with detect_deadlocks=0 but my hypothesis is that this is a side effect of tsan not understanding the seqlock: tsan sees that below we "publish" this piece of memory with an atomic write (in atomic_rcu_set), and does not see that with seqlock_write_begin we have a write memory barrier. I wonder if what we need instead is to annotate the seqlock functions, not the callers. Thanks, E.
diff --git a/util/qht.c b/util/qht.c index 67e5d5b916..739a53ced0 100644 --- a/util/qht.c +++ b/util/qht.c @@ -69,6 +69,7 @@ #include "qemu/qht.h" #include "qemu/atomic.h" #include "qemu/rcu.h" +#include "qemu/tsan.h" //#define QHT_DEBUG @@ -580,10 +581,12 @@ static void *qht_insert__locked(const struct qht *ht, struct qht_map *map, b = b->next; } while (b); + TSAN_ANNOTATE_IGNORE_WRITES_BEGIN(); b = qemu_memalign(QHT_BUCKET_ALIGN, sizeof(*b)); memset(b, 0, sizeof(*b)); new = b; i = 0; + TSAN_ANNOTATE_IGNORE_WRITES_END(); atomic_inc(&map->n_added_buckets); if (unlikely(qht_map_needs_resize(map)) && needs_resize) { *needs_resize = true;
For example: WARNING: ThreadSanitizer: data race (pid=23406) Atomic read of size 4 at 0x7b100003e3c8 by thread T7: #0 __tsan_atomic32_load <null> (qemu-system-aarch64+0x39a36c) #1 qht_do_lookup util/qht.c:495:17 (qemu-system-aarch64+0xd82f7a) #2 qht_lookup_custom util/qht.c:539:11 (qemu-system-aarch64+0xd82f7a) Previous write of size 8 at 0x7b100003e3c8 by thread T6 (mutexes: write M166769147697783108, write M995435858420506688): #0 posix_memalign <null> (qemu-system-aarch64+0x350dd1) #1 qemu_try_memalign util/oslib-posix.c:189:11 (qemu-system-aarch64+0xd59317) #2 qemu_memalign util/oslib-posix.c:205:27 (qemu-system-aarch64+0xd5943e) #3 qht_insert__locked util/qht.c:583:9 (qemu-system-aarch64+0xd837c5) Signed-off-by: Robert Foley <robert.foley@linaro.org> --- util/qht.c | 3 +++ 1 file changed, 3 insertions(+)