From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYz3V-0003h2-Li for qemu-devel@nongnu.org; Mon, 07 Sep 2015 12:14:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYz3S-0003tV-BD for qemu-devel@nongnu.org; Mon, 07 Sep 2015 12:14:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48915) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYz3S-0003tK-6W for qemu-devel@nongnu.org; Mon, 07 Sep 2015 12:14:02 -0400 References: <1440375847-17603-1-git-send-email-cota@braap.org> <1440375847-17603-8-git-send-email-cota@braap.org> <87d1xu9qf0.fsf@linaro.org> From: Paolo Bonzini Message-ID: <55EDB7C4.2020901@redhat.com> Date: Mon, 7 Sep 2015 18:13:56 +0200 MIME-Version: 1.0 In-Reply-To: <87d1xu9qf0.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 07/38] seqlock: read sequence number atomically List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= , "Emilio G. Cota" Cc: mttcg@greensocs.com, mark.burton@greensocs.com, a.rigo@virtualopensystems.com, qemu-devel@nongnu.org, guillaume.delbergue@greensocs.com, Frederic Konrad On 07/09/2015 17:53, Alex Benn=C3=A9e 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 seem= s > 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 >> > --- >> > 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(QemuSeqL= ock *sl) >> > static inline unsigned seqlock_read_begin(QemuSeqLock *sl) >> > { >> > /* Always fail if a write is in progress. */ >> > - unsigned ret =3D sl->sequence & ~1; >> > + unsigned ret =3D atomic_read(&sl->sequence); >> > =20 >> > /* Read sequence before reading other fields. */ >> > smp_rmb(); >> > - return ret; >> > + return ret & ~1; >> > } >> > =20 >> > static inline int seqlock_read_retry(const QemuSeqLock *sl, unsigne= d start) >> > { >> > /* Read other fields before reading final sequence. */ >> > smp_rmb(); >> > - return unlikely(sl->sequence !=3D start); >> > + return unlikely(atomic_read(&sl->sequence) !=3D start);