Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764657AbXHQQvi (ORCPT ); Fri, 17 Aug 2007 12:51:38 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753807AbXHQQv0 (ORCPT ); Fri, 17 Aug 2007 12:51:26 -0400 Received: from smtp2.linux-foundation.org ([207.189.120.14]:40312 "EHLO smtp2.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753453AbXHQQvX (ORCPT ); Fri, 17 Aug 2007 12:51:23 -0400 Date: Fri, 17 Aug 2007 09:48:39 -0700 (PDT) From: Linus Torvalds To: Nick Piggin cc: Satyam Sharma , Herbert Xu , Paul Mackerras , Christoph Lameter , Chris Snook , Ilpo Jarvinen , "Paul E. McKenney" , Stefan Richter , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Netdev , Andrew Morton , ak@suse.de, heiko.carstens@de.ibm.com, David Miller , schwidefsky@de.ibm.com, wensong@linux-vs.org, horms@verge.net.au, wjiang@resilience.com, cfriesen@nortel.com, zlynx@acm.org, rpjday@mindspring.com, jesper.juhl@gmail.com, segher@kernel.crashing.org Subject: Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures In-Reply-To: <46C59717.4020108@cyberone.com.au> Message-ID: References: <18115.52863.638655.658466@cargo.ozlabs.ibm.com> <20070816053945.GB32442@gondor.apana.org.au> <18115.62741.807704.969977@cargo.ozlabs.ibm.com> <20070816070907.GA964@gondor.apana.org.au> <46C4ABA5.9010804@redhat.com> <18117.1287.779351.836552@cargo.ozlabs.ibm.com> <18117.6495.397597.582736@cargo.ozlabs.ibm.com> <20070817035342.GA14744@gondor.apana.org.au> <46C55E90.7010407@yahoo.com.au> <46C56ADF.8010501@cyberone.com.au> <46C59717.4020108@cyberone.com.au> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5087 Lines: 115 On Fri, 17 Aug 2007, Nick Piggin wrote: > > That's not obviously just taste to me. Not when the primitive has many > (perhaps, the majority) of uses that do not require said barriers. And > this is not solely about the code generation (which, as Paul says, is > relatively minor even on x86). I prefer people to think explicitly > about barriers in their lockless code. Indeed. I think the important issues are: - "volatile" itself is simply a badly/weakly defined issue. The semantics of it as far as the compiler is concerned are really not very good, and in practice tends to boil down to "I will generate so bad code that nobody can accuse me of optimizing anything away". - "volatile" - regardless of how well or badly defined it is - is purely a compiler thing. It has absolutely no meaning for the CPU itself, so it at no point implies any CPU barriers. As a result, even if the compiler generates crap code and doesn't re-order anything, there's nothing that says what the CPU will do. - in other words, the *only* possible meaning for "volatile" is a purely single-CPU meaning. And if you only have a single CPU involved in the process, the "volatile" is by definition pointless (because even without a volatile, the compiler is required to make the C code appear consistent as far as a single CPU is concerned). So, let's take the example *buggy* code where we use "volatile" to wait for other CPU's: atomic_set(&var, 0); while (!atomic_read(&var)) /* nothing */; which generates an endless loop if we don't have atomic_read() imply volatile. The point here is that it's buggy whether the volatile is there or not! Exactly because the user expects multi-processing behaviour, but "volatile" doesn't actually give any real guarantees about it. Another CPU may have done: external_ptr = kmalloc(..); /* Setup is now complete, inform the waiter */ atomic_inc(&var); but the fact is, since the other CPU isn't serialized in any way, the "while-loop" (even in the presense of "volatile") doesn't actually work right! Whatever the "atomic_read()" was waiting for may not have completed, because we have no barriers! So if "volatile" makes a difference, it is invariably a sign of a bug in serialization (the one exception is for IO - we use "volatile" to avoid having to use inline asm for IO on x86) - and for "random values" like jiffies). So the question should *not* be whether "volatile" actually fixes bugs. It *never* fixes a bug. But what it can do is to hide the obvious ones. In other words, adding a volaile in the above kind of situation of "atomic_read()" will certainly turn an obvious bug into something that works "practically all of the time). So anybody who argues for "volatile" fixing bugs is fundamentally incorrect. It does NO SUCH THING. By arguing that, such people only show that you have no idea what they are talking about. So the only reason to add back "volatile" to the atomic_read() sequence is not to fix bugs, but to _hide_ the bugs better. They're still there, they are just a lot harder to trigger, and tend to be a lot subtler. And hey, sometimes "hiding bugs well enough" is ok. In this case, I'd argue that we've successfully *not* had the volatile there for eight months on x86-64, and that should tell people something. (Does _removing_ the volatile fix bugs? No - callers still need to think about barriers etc, and lots of people don't. So I'm not claiming that removing volatile fixes any bugs either, but I *am* claiming that: - removing volatile makes some bugs easier to see (which is mostly a good thing: they were there before, anyway). - removing volatile generates better code (which is a good thing, even if it's just 0.1%) - removing volatile removes a huge mental *bug* that lots of people seem to have, as shown by this whole thread. Anybody who thinks that "volatile" actually fixes anything has a gaping hole in their head, and we should remove volatile just to make sure that nobody thinks that it means soemthign that it doesn't mean! In other words, this whole discussion has just convinced me that we should *not* add back "volatile" to "atomic_read()" - I was willing to do it for practical and "hide the bugs" reasons, but having seen people argue for it, thinking that it actually fixes something, I'm now convinced that the *last* thing we should do is to encourage that kind of superstitious thinking. "volatile" is like a black cat crossing the road. Sure, it affects *something* (at a minimum: before, the black cat was on one side of the road, afterwards it is on the other side of the road), but it has no bigger and longer-lasting direct affects. People who think "volatile" really matters are just fooling themselves. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/