Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934458AbXHORqd (ORCPT ); Wed, 15 Aug 2007 13:46:33 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756542AbXHORqI (ORCPT ); Wed, 15 Aug 2007 13:46:08 -0400 Received: from pentafluge.infradead.org ([213.146.154.40]:35720 "EHLO pentafluge.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755535AbXHORqG (ORCPT ); Wed, 15 Aug 2007 13:46:06 -0400 Date: Wed, 15 Aug 2007 23:25:05 +0530 (IST) From: Satyam Sharma X-X-Sender: satyam@enigma.security.iitk.ac.in To: "Paul E. McKenney" cc: Stefan Richter , Christoph Lameter , 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, Herbert Xu Subject: Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures In-Reply-To: <20070815142516.GB9645@linux.vnet.ibm.com> Message-ID: References: <20070809131423.GA9927@shell.boston.redhat.com> <46C2D6F3.3070707@s5r6.in-berlin.de> <46C2FADB.7020407@s5r6.in-berlin.de> <20070815142516.GB9645@linux.vnet.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: 4347 Lines: 182 Hi Paul, On Wed, 15 Aug 2007, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 07:17:29PM +0530, Satyam Sharma wrote: > > [...] > > No, I'd actually prefer something like what Christoph Lameter suggested, > > i.e. users (such as above) who want "volatile"-like behaviour from atomic > > ops can use alternative functions. How about something like: > > > > #define atomic_read_volatile(v) \ > > ({ \ > > forget(&(v)->counter); \ > > ((v)->counter); \ > > }) > > Wouldn't the above "forget" the value, throw it away, then forget > that it forgot it, giving non-volatile semantics? Nope, I don't think so. I wrote the following trivial testcases: [ See especially tp4.c and tp4.s (last example). ] ============================================================================== $ cat tp1.c # Using volatile access casts #define atomic_read(a) (*(volatile int *)&a) int a; void func(void) { a = 0; while (atomic_read(a)) ; } ============================================================================== $ gcc -Os -S tp1.c; cat tp1.s func: pushl %ebp movl %esp, %ebp movl $0, a .L2: movl a, %eax testl %eax, %eax jne .L2 popl %ebp ret ============================================================================== $ cat tp2.c # Using nothing; gcc will optimize the whole loop away #define forget(x) #define atomic_read(a) \ ({ \ forget(&(a)); \ (a); \ }) int a; void func(void) { a = 0; while (atomic_read(a)) ; } ============================================================================== $ gcc -Os -S tp2.c; cat tp2.s func: pushl %ebp movl %esp, %ebp popl %ebp movl $0, a ret ============================================================================== $ cat tp3.c # Using a full memory clobber barrier #define forget(x) asm volatile ("":::"memory") #define atomic_read(a) \ ({ \ forget(&(a)); \ (a); \ }) int a; void func(void) { a = 0; while (atomic_read(a)) ; } ============================================================================== $ gcc -Os -S tp3.c; cat tp3.s func: pushl %ebp movl %esp, %ebp movl $0, a .L2: cmpl $0, a jne .L2 popl %ebp ret ============================================================================== $ cat tp4.c # Using a forget(var) macro #define forget(a) __asm__ __volatile__ ("" :"=m" (a) :"m" (a)) #define atomic_read(a) \ ({ \ forget(a); \ (a); \ }) int a; void func(void) { a = 0; while (atomic_read(a)) ; } ============================================================================== $ gcc -Os -S tp4.c; cat tp4.s func: pushl %ebp movl %esp, %ebp movl $0, a .L2: cmpl $0, a jne .L2 popl %ebp ret ============================================================================== Possibly these were too trivial to expose any potential problems that you may have been referring to, so would be helpful if you could write a more concrete example / sample code. > > Or possibly, implement these "volatile" atomic ops variants in inline asm > > like the patch that Sebastian Siewior has submitted on another thread just > > a while back. > > Given that you are advocating a change (please keep in mind that > atomic_read() and atomic_set() had volatile semantics on almost all > platforms), care to give some example where these historical volatile > semantics are causing a problem? > [...] > Can you give even one example > where the pre-existing volatile semantics are causing enough of a problem > to justify adding yet more atomic_*() APIs? Will take this to the other sub-thread ... > > Of course, if we find there are more callers in the kernel who want the > > volatility behaviour than those who don't care, we can re-define the > > existing ops to such variants, and re-name the existing definitions to > > somethine else, say "atomic_read_nonvolatile" for all I care. > > Do we really need another set of APIs? Well, if there's one set of users who do care about volatile behaviour, and another set that doesn't, it only sounds correct to provide both those APIs, instead of forcing one behaviour on all users. Thanks, 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/