[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 05/15] seqlock: use atomic writes for the seq
From: |
Jonathan Neuschäfer |
Subject: |
Re: [Qemu-devel] [PATCH v3 05/15] seqlock: use atomic writes for the sequence |
Date: |
Sat, 1 Oct 2016 00:14:26 +0200 |
User-agent: |
NeoMutt/20160910 (1.7.0) |
On Fri, Sep 30, 2016 at 10:30:56PM +0100, Alex Bennée wrote:
> From: Paolo Bonzini <address@hidden>
>
> There is a data race if the sequence is written concurrently to the
> read. In C11 this has undefined behavior. Use atomic_set; the
> read side is already using atomic_read.
>
> Reported-by: Alex Bennée <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
> include/qemu/seqlock.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h
> index 2e2be4c..8dee11d 100644
> --- a/include/qemu/seqlock.h
> +++ b/include/qemu/seqlock.h
> @@ -31,7 +31,7 @@ static inline void seqlock_init(QemuSeqLock *sl)
> /* Lock out other writers and update the count. */
> static inline void seqlock_write_begin(QemuSeqLock *sl)
> {
> - ++sl->sequence;
> + atomic_set(&sl->sequence, sl->sequence + 1);
I'm not an atomics expert, but as I read the code, the load of
sl->sequence (on the right side) isn't atomic relative to the store
(atomic_set). Should this be atomic_inc(&sl->sequence) instead, or am I
missing something?
In particular, I'm worried about this situation:
thread 0 thread 1
seqlock_write_begin: seqlock_write_begin:
unsigned tmp = sl->sequence unsigned tmp = sl->sequence
tmp += 1 tmp += 1
atomic_set(&sl->sequence, tmp)
atomic_set(&sl->sequence, tmp)
... where sl->sequence will effectively only be incremented once (as far
as I can see).
Regards,
Jonathan Neuschäfer
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH v3 00/15] A number of fixes for ThreadSanitizer, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 01/15] atomic.h: fix __SANITIZE_THREAD__ build, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 03/15] exec-all.h: revert tb_page_addr_t to target_ulong, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 06/15] qom/object: update class cache atomically, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 02/15] atomic.h: comment on use of atomic_read/set, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 04/15] tcg/optimize: move default return out of if statement, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 10/15] linux-user/syscall: extend lock around cpu-list, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 05/15] seqlock: use atomic writes for the sequence, Alex Bennée, 2016/09/30
- Re: [Qemu-devel] [PATCH v3 05/15] seqlock: use atomic writes for the sequence,
Jonathan Neuschäfer <=
- [Qemu-devel] [PATCH v3 08/15] cpu: atomically modify cpu->exit_request, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 07/15] qom/cpu: atomically clear the tb_jmp_cache, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 11/15] qga/command: use QEMU atomic primitives, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 12/15] .travis.yml: add gcc sanitizer build, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 13/15] tcg: ensure cpu_tb_exec/tb_gen_code use atomic_read/write, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 15/15] translate-all: mark updates to PageDesc as atomic, Alex Bennée, 2016/09/30
- [Qemu-devel] [PATCH v3 09/15] util/qht: atomically set b->hashes, Alex Bennée, 2016/09/30