Re: [Qemu-devel] [PATCH 2/6] test-rcu-list: avoid torn accesses to n_reclaims and n_nodes_removed

[Paolo Bonzini]

On 04/09/2018 22:56, Murilo Opsfelder Araujo wrote:
>>> +static inline void count_add(Count *c, long long val)
>>> +{
>>> +#ifdef CONFIG_ATOMIC64
>>> +    atomic_set__nocheck(&c->val, c->val + val);
>>> +#else
>>> +    seqlock_write_begin(&c->sequence);
>>> +    c->val += val;
>>> +    seqlock_write_end(&c->sequence);
>>> +#endif
>>> +}
>>> +
>>> +static inline void count_inc(Count *c)
>>> +{
>>> +    count_add(c, 1);
>>> +}
>> Are these `#ifdef CONFIG_ATOMIC64` required?
>>
>> The bodies of
>>
>>   seqlock_read_begin()
>>   seqlock_read_retry()
>>   seqlock_write_begin()
>>   seqlock_write_end()
>>
>> in include/qemu/seqlock.h make me think that they already use atomic 
>> operations.
>> What am I missing?
> atomic_read/set(*foo), as defined in qemu/atomic.h, are as wide as
> foo. The sequence number in a seqlock is an "unsigned", so those
> atomics won't be larger than 32 bits.
> 
> The counts we're dealing with here are 64-bits, so with
> #ifdef CONFIG_ATOMIC64 we ensure that the host can actually
> perform those 64-bit atomic accesses.

Like for patch 1, I'm not sure I like introducing the data races...
While reads inside the seqlock do not need atomics, I think the write
should be atomic in both cases.

Paolo