Date: Tue, 29 Sep 2009 17:21:27 -0400 From: John Baldwin <jhb@freebsd.org> To: Marius =?iso-8859-1?q?N=FCnnerich?= <marius@nuenneri.ch> Cc: Attilio Rao <attilio@freebsd.org>, hackers@freebsd.org, Fabio Checconi <fabio@freebsd.org> Subject: Re: sx locks and memory barriers Message-ID: <200909291721.27755.jhb@freebsd.org> In-Reply-To: <b649e5e0909291326o6faa090epd5e1d32da1d73f80@mail.gmail.com> References: <20090924224935.GW473@gandalf.sssup.it> <3bbf2fe10909291215i2bdd73aj13c1ac433152cab4@mail.gmail.com> <b649e5e0909291326o6faa090epd5e1d32da1d73f80@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 29 September 2009 4:26:56 pm Marius N=FCnnerich wrote: > On Tue, Sep 29, 2009 at 21:15, Attilio Rao <attilio@freebsd.org> wrote: > > 2009/9/29 John Baldwin <jhb@freebsd.org>: > >> On Tuesday 29 September 2009 11:39:37 am Attilio Rao wrote: > >>> 2009/9/25 Fabio Checconi <fabio@freebsd.org>: > >>> > Hi all, > >>> > =A0looking at sys/sx.h I have some troubles understanding this comm= ent: > >>> > > >>> > =A0* A note about memory barriers. =A0Exclusive locks need to use t= he same > >>> > =A0* memory barriers as mutexes: _acq when acquiring an exclusive l= ock > >>> > =A0* and _rel when releasing an exclusive lock. =A0On the other sid= e, > >>> > =A0* shared lock needs to use an _acq barrier when acquiring the lo= ck > >>> > =A0* but, since they don't update any locked data, no memory barrie= r is > >>> > =A0* needed when releasing a shared lock. > >>> > > >>> > In particular, I'm not understanding what prevents the following se= quence > >>> > from happening: > >>> > > >>> > CPU A =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 CPU B > >>> > > >>> > sx_slock(&data->lock); > >>> > > >>> > sx_sunlock(&data->lock); > >>> > > >>> > /* reordered after the unlock > >>> > =A0 by the cpu */ > >>> > if (data->buffer) > >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0sx_xlock(&data->lock); > >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0free(data->buffer); > >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0data->buffer =3D NULL; > >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0sx_xunlock(&data->lock); > >>> > > >>> > =A0 =A0 =A0 =A0a =3D *data->buffer; > >>> > > >>> > IOW, even if readers do not modify the data protected by the lock, > >>> > without a release barrier a memory access may leak past the unlock = (as > >>> > the cpu won't notice any dependency between the unlock and the fetc= h, > >>> > feeling free to reorder them), thus potentially racing with an excl= usive > >>> > writer accessing the data. > >>> > > >>> > On architectures where atomic ops serialize memory accesses this wo= uld > >>> > never happen, otherwise the sequence above seems possible; am I mis= sing > >>> > something? > >>> > >>> I think your concerns are right, possibly we need this patch: > >>> http://www.freebsd.org/~attilio/sxrw_unlockb.diff > >> > >> Actually, since you are only worried about reads, I think this should = be > >> an "acq" barrier rather than a "rel". =A0In some cases "acq" is cheape= r, so we > >> should prefer the cheapest barrier that provides what we need. =A0You = would > >> still need to keep some language about the memory barriers since using= "acq" > >> for shared unlocking is different from exclusive unlocking. > > > > Actually, I don't think that an acq barrier ensures enough protection > > against the reordering of 'earlier' operation thus not fixing the > > architecture ordering problem reported by Fabio. Also, I don't think > > we just have to care about reads (or =A0I don't understand what you mean > > here). > > However, I'm not even sure that we have faster read barriers than the > > write one. As long as it should be true in theory I don't think that's > > what happen in practice. > > > >> The memory clobber is quite heavyweight. =A0It actually forces gcc to = forget any > >> cached memory items in registers and reload everything again. =A0What = I really > >> want is just a barrier to tell GCC to not reorder things. =A0If I read= a value > >> in the program before acquiring a lock it is in theory fine to keep th= at > >> cached across the barrier. =A0However, there isn't a way to do this so= rt of > >> thing with GCC currently. > > > > Yes, that's the only tool we have right now with GCC. I will try to > > look for another way, but it sounds difficult to discover. >=20 > Even if we would have a mechanism to tell GCC to not reorder the > instructions the CPU itself would still be free to reorder if there > are no barriers. Or am I missing something? No, the thing to do here for the second part is add "memory" clobbers to the existing atomic ops with barriers. It will still require barriers for them= to be enforced. =2D-=20 John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909291721.27755.jhb>