[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 7/8] util/qht: atomically set b->hashes
From: |
Emilio G. Cota |
Subject: |
Re: [Qemu-devel] [RFC 7/8] util/qht: atomically set b->hashes |
Date: |
Mon, 19 Sep 2016 15:06:04 -0400 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, Sep 19, 2016 at 20:37:06 +0200, Paolo Bonzini wrote:
> On 19/09/2016 20:06, Emilio G. Cota wrote:
> > On Mon, Sep 19, 2016 at 16:51:38 +0100, Alex Bennée wrote:
> >> > ThreadSanitizer detects a possible race between reading/writing the
> >> > hashes. As ordering semantics are already documented for qht we just
> >> > need to ensure a race can't tear the hash value so we can use the
> >> > relaxed atomic_set/read functions.
> > This was discussed here:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03658.html
> >
> > To reiterate: reading torn hash values is fine, since the retry will
> > happen regardless (and all pointers[] remain valid through the RCU
> > read-critical section).
>
> True, but C11 says data races are undefined, not merely unspecified.
> seqlock-protected data requires a relaxed read and write, because they
> are read concurrently in the read and write sides.
Ah I see.
Let me then just point out that this comes at a small perf loss.
Running 'taskset -c 0 tests/qht-bench -n 1 -d 10' (i.e. all lookups) 10 times,
we get:
before the patch:
$ ./mean.pl 34.04 34.24 34.38 34.25 34.18 34.51 34.46 34.44 34.29 34.08
34.287 +- 0.160072900059109
after:
$ ./mean.pl 33.94 34.00 33.52 33.46 33.55 33.71 34.27 34.06 34.28 34.58
33.937 +- 0.374731014640279
But hey we can live with that.
Cheers,
E.
- Re: [Qemu-devel] [RFC 1/8] ui/vnc-enc-tight: add abort() for unexpected default, (continued)
[Qemu-devel] [RFC 3/8] new: blacklist.tsan, Alex Bennée, 2016/09/19
[Qemu-devel] [RFC 4/8] seqlock: use atomic writes for the sequence, Alex Bennée, 2016/09/19
[Qemu-devel] [RFC 8/8] .travis.yml: add gcc sanitizer build, Alex Bennée, 2016/09/19
[Qemu-devel] [RFC 7/8] util/qht: atomically set b->hashes, Alex Bennée, 2016/09/19