Date: Tue, 18 Sep 2012 13:53:24 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: David Chisnall <theraven@freebsd.org> Cc: Davide Italiano <davide@freebsd.org>, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, Jeff Roberson <jeff@freebsd.org>, Dimitry Andric <dim@freebsd.org>, attilio@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern Message-ID: <20120918105323.GB37286@deviant.kiev.zoral.com.ua> In-Reply-To: <EAB6CB55-C1A4-4099-AD86-6FDE3593A867@FreeBSD.org> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <CAJ-FndBj8tpC_BJXs_RH8sG2TBG8yA=Lxu3-GTVT9Ap_zOCuVQ@mail.gmail.com> <CAJ-FndDnO7wjnWPV0tTu%2BUGHjsxa3YDarMxmyei3ZmjLAFvRkQ@mail.gmail.com> <201207301732.33474.jhb@freebsd.org> <CAJ-FndD5EO12xsWOAe6u0EvX00q33wxO4OivnGjzj0=T2Oe8uA@mail.gmail.com> <CAJ-FndCRg0UCThFkatp=tw7rUWWCvhsApLE=iztLpxpGBC1F9w@mail.gmail.com> <505849DB.3090704@FreeBSD.org> <EAB6CB55-C1A4-4099-AD86-6FDE3593A867@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--PsEZZJjWpOB92CxQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 18, 2012 at 11:29:31AM +0100, David Chisnall wrote: > On 18 Sep 2012, at 11:15, Dimitry Andric wrote: >=20 > > Please use gcc's __sync_synchronize() builtin[1] instead, which is > > specifically for this purpose. Clang also supports it. > >=20 > > The builtin will emit actual memory barrier instructions, if the target > > architecture supports it, otherwise it will emit the same asm statement > > you show above. See contrib/gcc/builtins.c, around line 5584, function > > expand_builtin_synchronize(). >=20 > >From Attilio's description of the problem in IRC, I believe that > atomic_signal_fence() is the correct thing to use here. He stated that > he cares about reordering of memory access with regard to the current > CPU, but not with regard to other CPUs / threads. He also said that he > only cares about the compiler performing the reordering, not about the > CPU, but I suspect that is incorrect as there are numerous subtle bugs > that can creep in on weakly-ordered architectures (e.g. Alpha, ARMv8) > if you only have a compiler barrier. > How can atomic_SIGNAL_fence be even remotely conceptually right for the kernel execution environment ? The fact that current implementation happens to offer similar guarantees is a coincidense. > That said, this is likely to be incorrect, because it's very unusual > for that to actually be the requirement, especially in multithreaded > code (where the atomic.h stuff is actually important). In most of the > cases where __compiler_membar() is being used, you actually want at > least a partial barrier. > > On 18 Sep 2012, at 11:22, Konstantin Belousov wrote: > > > We do not need CPU barriers there, which are already handled by the > > atomic asms. It is only to prevent compiler from exploiting the > > reorder. > > If the atomic asm does not state that it clobbers memory, then it is > a bug. If it does clobber memory, then following it with an empty asm > statement that also clobbers memory is redundant. Looking in atomic.h > on amd64, they all do already clobber memory... Agreed, but you did not looked at atomic.h. The memory clobbering is only performed for atomics which do not need asm. The load acquire and store release are implemented as the normal load and stores on x86, due to existing architecture guarantees. > > The atomic operations are memory barriers themselves, although our > versions are often much stronger barriers than are required. I would Exactly which atomic operations for x86 do you reference there ? Are you aware about e.g. AMD recommendations for the barrier/lock prefix use (look into atomic.h) ? > like to see us slowly deprecate atomic.h in favour of stdatomic.h, > which has three significant advantages: > > 1) It's part of the C standard 2) It provides a well-defined set of > barrier types for every operation 3) It's implemented with compiler > assistance (including GCC 4.2.1 support in our current version, > although that version always enforces sequentially consistent > ordering) and allows the compiler more freedom to perform safe > optimisations around it > > David --PsEZZJjWpOB92CxQ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAlBYUqMACgkQC3+MBN1Mb4jaIgCfRZeM2FFP2yjmOvqCndf2QZPi OmcAn0JXZeBN3AkuMRw2V2V/bVi1sL9G =w8rI -----END PGP SIGNATURE----- --PsEZZJjWpOB92CxQ--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120918105323.GB37286>