From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/7658 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Deduplicating atomics written in terms of CAS Date: Sun, 17 May 2015 12:22:21 -0400 Message-ID: <20150517162221.GM17573@brightrain.aerifal.cx> References: <20150517045536.GA25046@brightrain.aerifal.cx> <1431845344.11628.1.camel@inria.fr> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1431879758 3797 80.91.229.3 (17 May 2015 16:22:38 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 17 May 2015 16:22:38 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-7670-gllmg-musl=m.gmane.org@lists.openwall.com Sun May 17 18:22:37 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Yu1Km-0001WF-Vh for gllmg-musl@m.gmane.org; Sun, 17 May 2015 18:22:37 +0200 Original-Received: (qmail 29732 invoked by uid 550); 17 May 2015 16:22:35 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 29714 invoked from network); 17 May 2015 16:22:34 -0000 Content-Disposition: inline In-Reply-To: <1431845344.11628.1.camel@inria.fr> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:7658 Archived-At: On Sun, May 17, 2015 at 08:49:04AM +0200, Jens Gustedt wrote: > Hello, > > Am Sonntag, den 17.05.2015, 00:55 -0400 schrieb Rich Felker: > > Lots of archs define most or all of their atomics except a_cas in > > terms of a_cas. The attached atomic.h is a proposed replacement for > > arch-specific atomic.h that would live in src/internal. The arch > > atomic.h files would be replaced with atomic_arch.h, which could opt > > to define nothing but a_cas, or could define more primitives itself if > > it can do so more efficiently. > > I like the approach > > > The second attachment, atomic_generic.h, is an implementation of the > > atomics (and other non-atomic ops we've traditionally had in atomic.h) > > using GNU C builtins. This file can be used as-is for any new archs > > that satisfy the following conditions: > > > > - They're not supported by compilers too old to have the __sync_* > > builtins. > > > > - They don't need runtime switching/detection of atomic > > implementations. > > > > - GCC doesn't generate pathologically bad code for the builtins. > > shouldn't this file then define or macros such as a_swap, too ? Hm? I don't understand what you're asking. > On quick inspection I found issues with the two 64 bit functions: > > #ifndef a_and_64 > static inline void a_and_64(volatile uint64_t *p, uint64_t v) > { > union { uint64_t v; uint32_t r[2]; } u = { v }; > if (u.r[0]+1) a_and((int *)p, u.r[0]); > if (u.r[1]+1) a_and((int *)p+1, u.r[1]); > } > #endif > > #ifndef a_or_64 > static inline void a_or_64(volatile uint64_t *p, uint64_t v) > { > union { uint64_t v; uint32_t r[2]; } u = { v }; > if (u.r[0]) a_or((int *)p, u.r[0]); > if (u.r[1]) a_or((int *)p+1, u.r[1]); > } > #endif > > First I don't get it how we can expect these to be be atomic. It looks > to me that the two 32 bit words can be updated with quite a laps of > time between them if the thread is delayed. I didn't check this, do we > really need 64bit atomics? These are misnomers. They're only used/needed as atomic bit-set and bit-clear. It would be nice to eliminate them completely, but malloc is using them right now. It would be easy to put the above logic directly in malloc and have the bitmasks be kept as a union of uint64_t and int[], but that's mildly ugly too I think. > Then, the mix of uint32_t and int is unfortunate. This code is in > header files and thus visible to all compilation units, especially > user code that might use any optimization option that the compiler > offers. The cast to int* breaks aliasing rules, so compilers that are > used with aggressive optimization may produce wrong executables, in > pretending that *p didn't change. The cast itself doesn't break aliasing rules. Only accessing the memory as int does that. The intent was that a_or would only access the object via asm, so the C type rules would not apply -- that's how things originally worked when we only had i386 and x86_64. But now that a_or is a C wrapper for a_cas on many/most archs, we do have an aliasing problem, I think. That makes me more eagar to remove these. > I only recently learned that even cast to volatile doesn't help in > cases where the original object to which p points is not declared > volatile. The C standard states that only volatile *declared* objects > are subject to the rules of volatile. Accessing through a volatile > pointer doesn't help. I'm not so sure about that. See this question on SO, which has two conflicting and both reasonable-sounding answers: http://stackoverflow.com/questions/28654418/requirements-for-behavior-of-pointer-to-volatile-pointing-to-non-volatile-object In any case, all objects used with atomics in musl are declared volatile now, or that is the intent anyway. If I missed any please let me know. Rich