Re: [Qemu-devel] [RFC 07/38] seqlock: read sequence number atomically

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/09/2015 17:53, Alex Bennée wrote:
>> > With this change we make sure that the compiler will not
>> > optimise the read of the sequence number in any way.
> What was it doing? Using atomic_read to work around a compiler bug seems
> a bit heavy handed if true atomicity isn't needed.

This is really the equivalent of C11 atomic_relaxed, so it isn't heavy
handed.  We really should move towards using atomic_read/atomic_set
around smp_rmb/smp_wmb/smp_mb.

Paolo

>> >
>> > Signed-off-by: Emilio G. Cota <cota@braap.org>
>> > ---
>> >  include/qemu/seqlock.h | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h
>> > index f1256f5..70b01fd 100644
>> > --- a/include/qemu/seqlock.h
>> > +++ b/include/qemu/seqlock.h
>> > @@ -55,18 +55,18 @@ static inline void seqlock_write_unlock(QemuSeqLock *sl)
>> >  static inline unsigned seqlock_read_begin(QemuSeqLock *sl)
>> >  {
>> >      /* Always fail if a write is in progress.  */
>> > -    unsigned ret = sl->sequence & ~1;
>> > +    unsigned ret = atomic_read(&sl->sequence);
>> >  
>> >      /* Read sequence before reading other fields.  */
>> >      smp_rmb();
>> > -    return ret;
>> > +    return ret & ~1;
>> >  }
>> >  
>> >  static inline int seqlock_read_retry(const QemuSeqLock *sl, unsigned start)
>> >  {
>> >      /* Read other fields before reading final sequence.  */
>> >      smp_rmb();
>> > -    return unlikely(sl->sequence != start);
>> > +    return unlikely(atomic_read(&sl->sequence) != start);