From owner-svn-src-projects@FreeBSD.ORG Tue Sep 18 10:29:34 2012 Return-Path: Delivered-To: svn-src-projects@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 788E7106566B; Tue, 18 Sep 2012 10:29:34 +0000 (UTC) (envelope-from theraven@FreeBSD.org) Received: from theravensnest.org (theraven.freebsd.your.org [216.14.102.27]) by mx1.freebsd.org (Postfix) with ESMTP id 127668FC08; Tue, 18 Sep 2012 10:29:33 +0000 (UTC) Received: from c120.sec.cl.cam.ac.uk (c120.sec.cl.cam.ac.uk [128.232.18.120]) (authenticated bits=0) by theravensnest.org (8.14.5/8.14.5) with ESMTP id q8IATUa5077578 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES128-SHA bits=128 verify=NO); Tue, 18 Sep 2012 10:29:30 GMT (envelope-from theraven@FreeBSD.org) Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=us-ascii From: David Chisnall In-Reply-To: <505849DB.3090704@FreeBSD.org> Date: Tue, 18 Sep 2012 11:29:31 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <201207301350.q6UDobCI099069@svn.freebsd.org> <201207301732.33474.jhb@freebsd.org> <505849DB.3090704@FreeBSD.org> To: Dimitry Andric X-Mailer: Apple Mail (2.1278) Cc: Davide Italiano , src-committers@FreeBSD.org, John Baldwin , Jeff Roberson , attilio@FreeBSD.org, svn-src-projects@FreeBSD.org, Konstantin Belousov Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Sep 2012 10:29:34 -0000 On 18 Sep 2012, at 11:15, Dimitry Andric wrote: > 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(). =46rom 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. 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. =20 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... The atomic operations are memory barriers themselves, although our = versions are often much stronger barriers than are required. I would = 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=