Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759525AbXHQIQn (ORCPT ); Fri, 17 Aug 2007 04:16:43 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752313AbXHQIQU (ORCPT ); Fri, 17 Aug 2007 04:16:20 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:34535 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752282AbXHQIQQ (ORCPT ); Fri, 17 Aug 2007 04:16:16 -0400 Date: Fri, 17 Aug 2007 13:58:14 +0530 (IST) From: Satyam Sharma X-X-Sender: satyam@enigma.security.iitk.ac.in To: Paul Mackerras cc: Herbert Xu , Stefan Richter , Christoph Lameter , "Paul E. McKenney" , Chris Snook , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Linus Torvalds , netdev@vger.kernel.org, Andrew Morton , ak@suse.de, heiko.carstens@de.ibm.com, davem@davemloft.net, 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: <18117.13576.93583.222760@cargo.ozlabs.ibm.com> Message-ID: References: <18115.45316.702491.681906@cargo.ozlabs.ibm.com> <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> <46C40587.7050708@s5r6.in-berlin.de> <20070816081049.GA1431@gondor.apana.org.au> <18117.11685.431347.996767@cargo.ozlabs.ibm.com> <20070817053200.GA15457@gondor.apana.org.au> <18117.13576.93583.222760@cargo.ozlabs.ibm.com> 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: 3095 Lines: 78 On Fri, 17 Aug 2007, Paul Mackerras wrote: > Herbert Xu writes: > > > On Fri, Aug 17, 2007 at 03:09:57PM +1000, Paul Mackerras wrote: > > > Herbert Xu writes: > > > > > > > Can you find an actual atomic_read code snippet there that is > > > > broken without the volatile modifier? > > > > > > There are some in arch-specific code, for example line 1073 of > > > arch/mips/kernel/smtc.c. On mips, cpu_relax() is just barrier(), so > > > the empty loop body is ok provided that atomic_read actually does the > > > load each time around the loop. > > > > A barrier() is all you need to force the compiler to reread > > the value. > > > > The people advocating volatile in this thread are talking > > about code that doesn't use barrier()/cpu_relax(). > > Did you look at it? Here it is: > > /* Someone else is initializing in parallel - let 'em finish */ > while (atomic_read(&idle_hook_initialized) < 1000) > ; Honestly, this thread is suffering from HUGE communication gaps. What Herbert (obviously) meant there was that "this loop could've been okay _without_ using volatile-semantics-atomic_read() also, if only it used cpu_relax()". That does work, because cpu_relax() is _at least_ barrier() on all archs (on some it also emits some arch-dependent "pause" kind of instruction). Now, saying that "MIPS does not have such an instruction so I won't use cpu_relax() for arch-dependent-busy-while-loops in arch/mips/" sounds like a wrong argument, because: tomorrow, such arch's _may_ introduce such an instruction, so naturally, at that time we'd change cpu_relax() appropriately (in reality, we would actually *re-define* cpu_relax() and ensure that the correct version gets pulled in depending on whether the callsite code is legacy or only for the newer such CPUs of said arch, whatever), but loops such as this would remain un-changed, because they never used cpu_relax()! OTOH an argument that said the following would've made a stronger case: "I don't want to use cpu_relax() because that's a full memory clobber barrier() and I have loop-invariants / other variables around in that code that I *don't* want the compiler to forget just because it used cpu_relax(), and hence I will not use cpu_relax() but instead make my atomic_read() itself have "volatility" semantics. Not just that, but I will introduce a cpu_relax_no_barrier() on MIPS, that would be a no-op #define for now, but which may not be so forever, and continue to use that in such busy loops." In general, please read the thread-summary I've tried to do at: http://lkml.org/lkml/2007/8/17/25 Feel free to continue / comment / correct stuff from there, there's too much confusion and circular-arguments happening on this thread otherwise. [ I might've made an incorrect statement there about "volatile" w.r.t. cache on non-x86 archs, I think. ] Satyam - 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/