Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764140AbXHQKAL (ORCPT ); Fri, 17 Aug 2007 06:00:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756285AbXHQJ7y (ORCPT ); Fri, 17 Aug 2007 05:59:54 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:52307 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756174AbXHQJ7w (ORCPT ); Fri, 17 Aug 2007 05:59:52 -0400 Date: Fri, 17 Aug 2007 15:42:26 +0530 (IST) From: Satyam Sharma X-X-Sender: satyam@enigma.security.iitk.ac.in To: Nick Piggin cc: Linus Torvalds , Paul Mackerras , Segher Boessenkool , heiko.carstens@de.ibm.com, horms@verge.net.au, Linux Kernel Mailing List , rpjday@mindspring.com, ak@suse.de, netdev@vger.kernel.org, cfriesen@nortel.com, Andrew Morton , jesper.juhl@gmail.com, linux-arch@vger.kernel.org, zlynx@acm.org, clameter@sgi.com, schwidefsky@de.ibm.com, Chris Snook , Herbert Xu , davem@davemloft.net, wensong@linux-vs.org, wjiang@resilience.com Subject: Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures In-Reply-To: <46C56725.6000207@cyberone.com.au> Message-ID: References: <46C32618.2080108@redhat.com> <20070815234021.GA28775@gondor.apana.org.au> <3694fb2e4ed1e4d9bf873c0d050c911e@kernel.crashing.org> <46C3B50E.7010702@yahoo.com.au> <194369f4c96ea0e24decf8f9197d5bad@kernel.crashing.org> <46C505B2.6030704@yahoo.com.au> <18117.4848.695269.72976@cargo.ozlabs.ibm.com> <46C54D94.5080803@yahoo.com.au> <46C56725.6000207@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: 3524 Lines: 123 On Fri, 17 Aug 2007, Nick Piggin wrote: > Satyam Sharma wrote: > > > > On Fri, 17 Aug 2007, Nick Piggin wrote: > > > > Also, why would you want to make these insane accessors for atomic_t > > > types? Just make sure everybody knows the basics of barriers, and they > > > can apply that knowledge to atomic_t and all other lockless memory > > > accesses as well. > > > > > > Code that looks like: > > > > while (!atomic_read(&v)) { > > ... > > cpu_relax_no_barrier(); > > forget(v.counter); > > ^^^^^^^^ > > } > > > > would be uglier. Also think about code such as: > > I think they would both be equally ugly, You think both these are equivalent in terms of "looks": | while (!atomic_read(&v)) { | while (!atomic_read_xxx(&v)) { ... | ... cpu_relax_no_barrier(); | cpu_relax_no_barrier(); order_atomic(&v); | } } | (where order_atomic() is an atomic_t specific wrapper as you mentioned below) ? Well, taste varies, but ... > but the atomic_read_volatile > variant would be more prone to subtle bugs because of the weird > implementation. What bugs? > And it would be more ugly than introducing an order(x) statement for > all memory operations, and adding an order_atomic() wrapper for it > for atomic types. Oh, that order() / forget() macro [forget() was named such by Chuck Ebbert earlier in this thread where he first mentioned it, btw] could definitely be generically introduced for any memory operations. > > a = atomic_read(); > > if (!a) > > do_something(); > > > > forget(); > > a = atomic_read(); > > ... /* some code that depends on value of a, obviously */ > > > > forget(); > > a = atomic_read(); > > ... > > > > So much explicit sprinkling of "forget()" looks ugly. > > Firstly, why is it ugly? It's nice because of those nice explicit > statements there that give us a good heads up and would have some > comments attached to them atomic_read_xxx (where xxx = whatever naming sounds nice to you) would obviously also give a heads up, and could also have some comments attached to it. > (also, lack of the word "volatile" is always a plus). Ok, xxx != volatile. > Secondly, what sort of code would do such a thing? See the nodemgr_host_thread() that does something similar, though not exactly same. > > on the other hand, looks neater. The "_volatile()" suffix makes it also > > no less explicit than an explicit barrier-like macro that this primitive > > is something "special", for code clarity purposes. > > Just don't use the word volatile, That sounds amazingly frivolous, but hey, why not. As I said, ok, xxx != volatile. > and have barriers both before and after the memory operation, How could that lead to bugs? (if you can point to existing code, but just some testcase / sample code would be fine as well). > [...] I don't see > the point though, when you could just have a single barrier(x) > barrier function defined for all memory locations, As I said, barrier() is too heavy-handed. > rather than > this odd thing that only works for atomics Why would it work only for atomics? You could use that generic macro for anything you well damn please. > (and would have to > be duplicated for atomic_set. #define atomic_set_xxx for something similar. Big deal ... NOT. - 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/