[PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-09 Thread Chris Snook
As recent discussions[1], and bugs[2] have shown, there is a great deal of confusion about the expected behavior of atomic_read(), compounded by the fact that it is not the same on all architectures. Since users expect calls to atomic_read() to actually perform a read, it is not desirable to allow

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-09 Thread Arnd Bergmann
On Thursday 09 August 2007, Chris Snook wrote: > This patchset makes the behavior of atomic_read uniform by removing the > volatile keyword from all atomic_t and atomic64_t definitions that currently > have it, and instead explicitly casts the variable as volatile in > atomic_read().  This leaves l

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-09 Thread Chris Snook
Arnd Bergmann wrote: On Thursday 09 August 2007, Chris Snook wrote: This patchset makes the behavior of atomic_read uniform by removing the volatile keyword from all atomic_t and atomic64_t definitions that currently have it, and instead explicitly casts the variable as volatile in atomic_read()

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-09 Thread Arnd Bergmann
On Thursday 09 August 2007, Chris Snook wrote: > a) chicken and egg: asm-generic/atomic.h depends on definitions in > asm/atomic.h Ok, I see. > If you can find a way to reshuffle the code and make it simpler, I personally > am > all for it. I'm skeptical that you'll get much to show for the e

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Christoph Lameter
On Thu, 9 Aug 2007, Chris Snook wrote: > This patchset makes the behavior of atomic_read uniform by removing the > volatile keyword from all atomic_t and atomic64_t definitions that currently > have it, and instead explicitly casts the variable as volatile in > atomic_read(). This leaves little r

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Chris Snook
Christoph Lameter wrote: On Thu, 9 Aug 2007, Chris Snook wrote: This patchset makes the behavior of atomic_read uniform by removing the volatile keyword from all atomic_t and atomic64_t definitions that currently have it, and instead explicitly casts the variable as volatile in atomic_read().

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Christoph Lameter
On Tue, 14 Aug 2007, Chris Snook wrote: > But barriers force a flush of *everything* in scope, which we generally don't > want. On the other hand, we pretty much always want to flush atomic_* > operations. One way or another, we should be restricting the volatile > behavior to the thing that nee

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Satyam Sharma
On Tue, 14 Aug 2007, Christoph Lameter wrote: > On Thu, 9 Aug 2007, Chris Snook wrote: > > > This patchset makes the behavior of atomic_read uniform by removing the > > volatile keyword from all atomic_t and atomic64_t definitions that currently > > have it, and instead explicitly casts the var

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Chris Snook
Satyam Sharma wrote: On Tue, 14 Aug 2007, Christoph Lameter wrote: On Thu, 9 Aug 2007, Chris Snook wrote: This patchset makes the behavior of atomic_read uniform by removing the volatile keyword from all atomic_t and atomic64_t definitions that currently have it, and instead explicitly casts

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Christoph Lameter
On Tue, 14 Aug 2007, Chris Snook wrote: > Because atomic operations are generally used for synchronization, which > requires volatile behavior. Most such codepaths currently use an inefficient > barrier(). Some forget to and we get bugs, because people assume that > atomic_read() actually reads

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 04:38:54AM +0530, Satyam Sharma wrote: > > > On Tue, 14 Aug 2007, Christoph Lameter wrote: > > > On Thu, 9 Aug 2007, Chris Snook wrote: > > > > > This patchset makes the behavior of atomic_read uniform by removing the > > > volatile keyword from all atomic_t and atomic64

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-14 Thread Herbert Xu
Chris Snook <[EMAIL PROTECTED]> wrote: > > Because atomic operations are generally used for synchronization, which > requires > volatile behavior. Most such codepaths currently use an inefficient > barrier(). > Some forget to and we get bugs, because people assume that atomic_read() > actua

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Heiko Carstens
On Wed, Aug 15, 2007 at 02:49:03PM +0800, Herbert Xu wrote: > Chris Snook <[EMAIL PROTECTED]> wrote: > > > > Because atomic operations are generally used for synchronization, which > > requires > > volatile behavior. Most such codepaths currently use an inefficient > > barrier(). > > Some fo

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Stefan Richter
Satyam Sharma wrote: > [ BTW, why do we want the compiler to not optimize atomic_read()'s in > the first place? Atomic ops guarantee atomicity, which has nothing > to do with "volatility" -- users that expect "volatility" from > atomic ops are the ones who must be fixed instead, IMHO. ] LDD3

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 12:35:31PM +0200, Stefan Richter wrote: > > LDD3 says on page 125: "The following operations are defined for the > type [atomic_t] and are guaranteed to be atomic with respect to all > processors of an SMP computer." > > Doesn't "atomic WRT all processors" require volatil

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Stefan Richter wrote: > Satyam Sharma wrote: > > [ BTW, why do we want the compiler to not optimize atomic_read()'s in > > the first place? Atomic ops guarantee atomicity, which has nothing > > to do with "volatility" -- users that expect "volatility" from > > atomic o

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Stefan Richter
Satyam Sharma wrote: > On Wed, 15 Aug 2007, Stefan Richter wrote: >> Doesn't "atomic WRT all processors" require volatility? > > No, it definitely doesn't. Why should it? > > "Atomic w.r.t. all processors" is just your normal, simple "atomicity" > for SMP systems (ensure that that object is modif

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Stefan Richter
I wrote: > static inline void A(atomic_t *a) > { > int b = atomic_read(a); > if (b) > do_something_time_consuming(); > } > > static inline void B(atomic_t *a) > { > int b = atomic_read(a); > if (b) > do_something_more(); > } > > static void C(at

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Stefan Richter
On 8/15/2007 10:18 AM, Heiko Carstens wrote: > On Wed, Aug 15, 2007 at 02:49:03PM +0800, Herbert Xu wrote: >> Chris Snook <[EMAIL PROTECTED]> wrote: >> > >> > Because atomic operations are generally used for synchronization, which >> > requires >> > volatile behavior. Most such codepaths curren

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Stefan Richter wrote: > Satyam Sharma wrote: > > On Wed, 15 Aug 2007, Stefan Richter wrote: > >> Doesn't "atomic WRT all processors" require volatility? > > > > No, it definitely doesn't. Why should it? > > > > "Atomic w.r.t. all processors" is just your normal, simple "at

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
Hi Stefan, On Wed, 15 Aug 2007, Stefan Richter wrote: > On 8/15/2007 10:18 AM, Heiko Carstens wrote: > > On Wed, Aug 15, 2007 at 02:49:03PM +0800, Herbert Xu wrote: > >> Chris Snook <[EMAIL PROTECTED]> wrote: > >> > > >> > Because atomic operations are generally used for synchronization, which

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 07:17:29PM +0530, Satyam Sharma wrote: > On Wed, 15 Aug 2007, Stefan Richter wrote: > > Satyam Sharma wrote: > > > On Wed, 15 Aug 2007, Stefan Richter wrote: > > >> Doesn't "atomic WRT all processors" require volatility? > > > > > > No, it definitely doesn't. Why should it?

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 08:05:38PM +0530, Satyam Sharma wrote: > > > I don't know if this here is affected: > > Yes, I think it is. You're clearly expecting the read to actually happen > when you call get_hpsb_generation(). It's clearly not a busy-loop, so > cpu_relax() sounds pointless / wrong so

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 07:25:16AM -0700, Paul E. McKenney wrote: > > Do we really need another set of APIs? 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? Let's turn this around. Can you gi

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 11:33:36PM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 07:25:16AM -0700, Paul E. McKenney wrote: > > > > Do we really need another set of APIs? Can you give even one example > > where the pre-existing volatile semantics are causing enough of a problem > > to justify

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Stefan Richter
Herbert Xu wrote: > On Wed, Aug 15, 2007 at 08:05:38PM +0530, Satyam Sharma wrote: >>> I don't know if this here is affected: [...something like] b = atomic_read(a); for (i = 0; i < 4; i++) { msleep_interruptible(63); c = atomic_read(a);

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Chris Snook
Herbert Xu wrote: Chris Snook <[EMAIL PROTECTED]> wrote: Because atomic operations are generally used for synchronization, which requires volatile behavior. Most such codepaths currently use an inefficient barrier(). Some forget to and we get bugs, because people assume that atomic_read() ac

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 06:09:35PM +0200, Stefan Richter wrote: > Herbert Xu wrote: > > On Wed, Aug 15, 2007 at 08:05:38PM +0530, Satyam Sharma wrote: > >>> I don't know if this here is affected: > > [...something like] > b = atomic_read(a); > for (i = 0; i < 4; i++) { >

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 06:09:35PM +0200, Stefan Richter wrote: > > Herbert Xu wrote: > > > On Wed, Aug 15, 2007 at 08:05:38PM +0530, Satyam Sharma wrote: > > >>> I don't know if this here is affected: > > > > [...something like] > > b = atomic

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 11:33:36PM +0800, Herbert Xu wrote: > > On Wed, Aug 15, 2007 at 07:25:16AM -0700, Paul E. McKenney wrote: > > > > > > Do we really need another set of APIs? Can you give even one example > > > where the pre-existing volatil

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 10:48:28PM +0530, Satyam Sharma wrote: > > [...] > > Not for i386 and x86_64 -- those have atomic ops without any "volatile" > > semantics (currently as per existing definitions). > > I claim unit volumes with arm, and the m

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
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

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 10:48:28PM +0530, Satyam Sharma wrote: > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > On Wed, Aug 15, 2007 at 11:33:36PM +0800, Herbert Xu wrote: > > > On Wed, Aug 15, 2007 at 07:25:16AM -0700, Paul E. McKenney wrote: > > > > > > > > Do we really need another set of API

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread David Howells
Herbert Xu <[EMAIL PROTECTED]> wrote: > Let's turn this around. Can you give a single example where > the volatile semantics is needed in a legitimate way? Accessing H/W registers? But apart from that... David - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 07:19:57PM +0100, David Howells wrote: > Herbert Xu <[EMAIL PROTECTED]> wrote: > > > Let's turn this around. Can you give a single example where > > the volatile semantics is needed in a legitimate way? > > Accessing H/W registers? But apart from that... Communicating b

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
"Volatile behaviour" itself isn't consistently defined (at least definitely not consistently implemented in various gcc versions across platforms), It should be consistent across platforms; if not, file a bug please. but it is /expected/ to mean something like: "ensure that every such access a

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 08:31:25PM +0200, Segher Boessenkool wrote: > >>How does the compiler know that msleep() has got barrier()s? > > > >Because msleep_interruptible() is in a separate compilation unit, > >the compiler has to assume that it might modify any arbitrary global. > > No; compilation

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 11:25:05PM +0530, Satyam Sharma wrote: > 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

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Segher Boessenkool wrote: > > "Volatile behaviour" itself isn't consistently defined (at least > > definitely not consistently implemented in various gcc versions across > > platforms), > > It should be consistent across platforms; if not, file a bug please. > > > but it i

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
[ The Cc: list scares me. Should probably trim it. ] On Wed, 15 Aug 2007, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 08:31:25PM +0200, Segher Boessenkool wrote: > > >>How does the compiler know that msleep() has got barrier()s? > > > > > >Because msleep_interruptible() is in a separate co

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 01:24:42AM +0530, Satyam Sharma wrote: > [ The Cc: list scares me. Should probably trim it. ] Trim away! ;-) > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > > On Wed, Aug 15, 2007 at 08:31:25PM +0200, Segher Boessenkool wrote: > > > >>How does the compiler know that m

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Wed, 15 Aug 2007, Stefan Richter wrote: > LDD3 says on page 125: "The following operations are defined for the > type [atomic_t] and are guaranteed to be atomic with respect to all > processors of an SMP computer." > > Doesn't "atomic WRT all processors" require volatility? Atomic operations

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
How does the compiler know that msleep() has got barrier()s? Because msleep_interruptible() is in a separate compilation unit, the compiler has to assume that it might modify any arbitrary global. No; compilation units have nothing to do with it, GCC can optimise across compilation unit bounda

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
I think this was just terminology confusion here again. Isn't "any code that it cannot currently see" the same as "another compilation unit", and wouldn't the "compilation unit" itself expand if we ask gcc to compile more than one unit at once? Or is there some more specific "definition" for "com

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
What volatile does are a) never optimise away a read (or write) to the object, since the data can change in ways the compiler cannot see; and b) never move stores to the object across a sequence point. This does not mean other accesses cannot be reordered wrt the volatile access. If the abstract

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
What you probably mean is that the compiler has to assume any code it cannot currently see can do anything (insofar as allowed by the relevant standards etc.) I think this was just terminology confusion here again. Isn't "any code that it cannot currently see" the same as "another compilation un

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
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 a

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
No; compilation units have nothing to do with it, GCC can optimise across compilation unit boundaries just fine, if you tell it to compile more than one compilation unit at once. Last I checked, the Linux kernel build system did compile each .c file as a separate compilation unit. I have some

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
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. The trick is to have a sufficiently complicated expression to force the compiler to run out of registers. You ca

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 10:52:53PM +0200, Segher Boessenkool wrote: > >>I think this was just terminology confusion here again. Isn't "any > >>code > >>that it cannot currently see" the same as "another compilation unit", > >>and wouldn't the "compilation unit" itself expand if we ask gcc to > >>c

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 11:05:35PM +0200, Segher Boessenkool wrote: > >>No; compilation units have nothing to do with it, GCC can optimise > >>across compilation unit boundaries just fine, if you tell it to > >>compile more than one compilation unit at once. > > > >Last I checked, the Linux kernel

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Satyam Sharma writes: > > Doesn't "atomic WRT all processors" require volatility? > > No, it definitely doesn't. Why should it? > > "Atomic w.r.t. all processors" is just your normal, simple "atomicity" > for SMP systems (ensure that that object is modified / set / replaced > in main memory atom

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 11:45:20AM -0700, Paul E. McKenney wrote: > On Wed, Aug 15, 2007 at 07:19:57PM +0100, David Howells wrote: > > Herbert Xu <[EMAIL PROTECTED]> wrote: > > > > > Let's turn this around. Can you give a single example where > > > the volatile semantics is needed in a legitimate

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 07:40:21AM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 12:13:12PM -0400, Chris Snook wrote: > > > > Part of the motivation here is to fix heisenbugs. If I knew where they > > By the same token we should probably disable optimisations > altogether since that too can

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 07:41:46AM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 11:45:20AM -0700, Paul E. McKenney wrote: > > On Wed, Aug 15, 2007 at 07:19:57PM +0100, David Howells wrote: > > > Herbert Xu <[EMAIL PROTECTED]> wrote: > > > > > > > Let's turn this around. Can you give a singl

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 12:13:12PM -0400, Chris Snook wrote: > > Part of the motivation here is to fix heisenbugs. If I knew where they By the same token we should probably disable optimisations altogether since that too can create heisenbugs. Cheers, -- Visit Openswan at http://www.openswan.o

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 04:53:35PM -0700, Paul E. McKenney wrote: > > > > Communicating between process context and interrupt/NMI handlers using > > > per-CPU variables. > > > > Remeber we're talking about atomic_read/atomic_set. Please > > cite the actual file/function name you have in mind. >

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 08:12:48AM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 04:53:35PM -0700, Paul E. McKenney wrote: > > > > > > Communicating between process context and interrupt/NMI handlers using > > > > per-CPU variables. > > > > > > Remeber we're talking about atomic_read/atomic_s

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Thu, 16 Aug 2007, Paul Mackerras wrote: > In the kernel we use atomic variables in precisely those situations > where a variable is potentially accessed concurrently by multiple > CPUs, and where each CPU needs to see updates done by other CPUs in a > timely fashion. That is what they are for.

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 05:23:10PM -0700, Paul E. McKenney wrote: > On Thu, Aug 16, 2007 at 08:12:48AM +0800, Herbert Xu wrote: > > On Wed, Aug 15, 2007 at 04:53:35PM -0700, Paul E. McKenney wrote: > > > > > > > > Communicating between process context and interrupt/NMI handlers using > > > > > per-

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Christoph Lameter writes: > On Thu, 16 Aug 2007, Paul Mackerras wrote: > > > In the kernel we use atomic variables in precisely those situations > > where a variable is potentially accessed concurrently by multiple > > CPUs, and where each CPU needs to see updates done by other CPUs in a > > time

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Thu, 16 Aug 2007, Paul Mackerras wrote: > Those barriers are for when we need ordering between atomic variables > and other memory locations. An atomic variable by itself doesn't and > shouldn't need any barriers for other CPUs to be able to see what's > happening to it. It does not need any

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > Seems to me that we face greater chance of confusion without the > volatile than with, particularly as compiler optimizations become > more aggressive. Yes, we could simply disable optimization, but > optimization can be quite helpful. A volatile de

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
[ Sorry for empty subject line in previous mail. I intended to make a patch so cleared it to change it, but ultimately neither made a patch nor restored subject line. Done that now. ] On Thu, 16 Aug 2007, Herbert Xu wrote: > On Thu, Aug 16, 2007 at 06:06:00AM +0530, Satyam Sharma wrote: > >

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 08:30:23AM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 05:23:10PM -0700, Paul E. McKenney wrote: > > On Thu, Aug 16, 2007 at 08:12:48AM +0800, Herbert Xu wrote: > > > On Wed, Aug 15, 2007 at 04:53:35PM -0700, Paul E. McKenney wrote: > > > > > > > > > > Communicating b

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Thu, Aug 16, 2007 at 06:28:42AM +0530, Satyam Sharma wrote: > > > The udelay itself certainly should have some form of cpu_relax in it. > > Yes, a form of barrier() must be present in mdelay() or udelay() itself > as you say, having it in __const_udelay() is *not* enough (superflous > actually,

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Wed, Aug 15, 2007 at 05:49:50PM -0700, Paul E. McKenney wrote: > On Thu, Aug 16, 2007 at 08:30:23AM +0800, Herbert Xu wrote: > > > Thanks. But I don't need a summary of the thread, I'm asking > > for an extant code snippet in our kernel that benefits from > > the volatile change and is not part

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 05:42:07PM -0700, Christoph Lameter wrote: > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > > Seems to me that we face greater chance of confusion without the > > volatile than with, particularly as compiler optimizations become > > more aggressive. Yes, we could simply

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 05:26:34PM -0700, Christoph Lameter wrote: > On Thu, 16 Aug 2007, Paul Mackerras wrote: > > > In the kernel we use atomic variables in precisely those situations > > where a variable is potentially accessed concurrently by multiple > > CPUs, and where each CPU needs to see

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Segher Boessenkool wrote: > [...] > > BTW: > > > > #define atomic_read(a) (*(volatile int *)&(a)) > > #define atomic_set(a,i) (*(volatile int *)&(a) = (i)) > > > > int a; > > > > void func(void) > > { > > int b; > > > > b = atomic_read(a); > > atomic

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
No; compilation units have nothing to do with it, GCC can optimise across compilation unit boundaries just fine, if you tell it to compile more than one compilation unit at once. Last I checked, the Linux kernel build system did compile each .c file as a separate compilation unit. I have som

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 08:53:16AM +0800, Herbert Xu wrote: > On Wed, Aug 15, 2007 at 05:49:50PM -0700, Paul E. McKenney wrote: > > On Thu, Aug 16, 2007 at 08:30:23AM +0800, Herbert Xu wrote: > > > > > Thanks. But I don't need a summary of the thread, I'm asking > > > for an extant code snippet in

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
Hi Herbert, On Thu, 16 Aug 2007, Herbert Xu wrote: > On Thu, Aug 16, 2007 at 06:28:42AM +0530, Satyam Sharma wrote: > > > > > The udelay itself certainly should have some form of cpu_relax in it. > > > > Yes, a form of barrier() must be present in mdelay() or udelay() itself > > as you say, hav

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > The volatile cast should not disable all that many optimizations, > for example, it is much less hurtful than barrier(). Furthermore, > the main optimizations disabled (pulling atomic_read() and atomic_set() > out of loops) really do need to be disab

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 05:59:41PM -0700, Christoph Lameter wrote: > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > > The volatile cast should not disable all that many optimizations, > > for example, it is much less hurtful than barrier(). Furthermore, > > the main optimizations disabled (pull

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Christoph Lameter writes: > A volatile default would disable optimizations for atomic_read. > atomic_read without volatile would allow for full optimization by the > compiler. Seems that this is what one wants in many cases. Name one such case. An atomic_read should do a load from memory. If

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Wed, 15 Aug 2007, Paul E. McKenney wrote: > Understood. My point is not that the impact is precisely zero, but > rather that the impact on optimization is much less hurtful than the > problems that could arise otherwise, particularly as compilers become > more aggressive in their optimizations

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
Part of the motivation here is to fix heisenbugs. If I knew where they By the same token we should probably disable optimisations altogether since that too can create heisenbugs. Almost everything is a tradeoff; and so is this. I don't believe most people would find disabling all compiler op

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
Part of the motivation here is to fix heisenbugs. If I knew where they By the same token we should probably disable optimisations altogether since that too can create heisenbugs. Precisely the point -- use of volatile (whether in casts or on asms) in these cases are intended to disable those

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Thu, Aug 16, 2007 at 11:51:42AM +1000, Paul Mackerras wrote: > > Name one such case. See sk_stream_mem_schedule in net/core/stream.c: /* Under limit. */ if (atomic_read(sk->sk_prot->memory_allocated) < sk->sk_prot->sysctl_mem[0]) { if (*sk->sk_prot->memory_pre

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Wed, 15 Aug 2007, Christoph Lameter wrote: > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > > Understood. My point is not that the impact is precisely zero, but > > rather that the impact on optimization is much less hurtful than the > > problems that could arise otherwise, particularly a

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Herbert Xu writes: > See sk_stream_mem_schedule in net/core/stream.c: > > /* Under limit. */ > if (atomic_read(sk->sk_prot->memory_allocated) < > sk->sk_prot->sysctl_mem[0]) { > if (*sk->sk_prot->memory_pressure) > *sk->sk_prot->memory_pres

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Segher Boessenkool
A volatile default would disable optimizations for atomic_read. atomic_read without volatile would allow for full optimization by the compiler. Seems that this is what one wants in many cases. Name one such case. An atomic_read should do a load from memory. If the programmer puts an atomic_rea

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Thu, Aug 16, 2007 at 07:45:44AM +0530, Satyam Sharma wrote: > > Completely agreed, again. To summarize again (had done so about ~100 mails > earlier in this thread too :-) ... > > atomic_{read,set}_volatile() -- guarantees volatility also along with > atomicity (the two _are_ different concepts

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Thu, Aug 16, 2007 at 12:05:56PM +1000, Paul Mackerras wrote: > Herbert Xu writes: > > > See sk_stream_mem_schedule in net/core/stream.c: > > > > /* Under limit. */ > > if (atomic_read(sk->sk_prot->memory_allocated) < > > sk->sk_prot->sysctl_mem[0]) { > > if (*s

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Thu, 16 Aug 2007, Paul Mackerras wrote: > > We don't need to reload sk->sk_prot->memory_allocated here. > > Are you sure? How do you know some other CPU hasn't changed the value > in between? The cpu knows because the cacheline was not invalidated. - To unsubscribe from this list: send the

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Chris Friesen
Herbert Xu wrote: But I have to say that I still don't know of a single place where one would actually use the volatile variant. Given that many of the existing users do currently have "volatile", are you comfortable simply removing that behaviour from them? Are you sure that you will not i

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Thu, 16 Aug 2007, Paul Mackerras wrote: > Herbert Xu writes: > > > See sk_stream_mem_schedule in net/core/stream.c: > > > > /* Under limit. */ > > if (atomic_read(sk->sk_prot->memory_allocated) < > > sk->sk_prot->sysctl_mem[0]) { > > if (*sk->sk_prot->memory

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 03:23:28AM +0200, Segher Boessenkool wrote: > No; compilation units have nothing to do with it, GCC can optimise > across compilation unit boundaries just fine, if you tell it to > compile more than one compilation unit at once. > >>> > >>>Last I checked, the Lin

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Nick Piggin
Segher Boessenkool wrote: Part of the motivation here is to fix heisenbugs. If I knew where they By the same token we should probably disable optimisations altogether since that too can create heisenbugs. Almost everything is a tradeoff; and so is this. I don't believe most people would f

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Thu, 16 Aug 2007, Herbert Xu wrote: > > Do we have a consensus here? (hoping against hope, probably :-) > > I can certainly agree with this. I agree too. > But I have to say that I still don't know of a single place > where one would actually use the volatile variant. I suspect that what yo

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Christoph Lameter
On Wed, 15 Aug 2007, Christoph Lameter wrote: > On Thu, 16 Aug 2007, Paul Mackerras wrote: > > > > We don't need to reload sk->sk_prot->memory_allocated here. > > > > Are you sure? How do you know some other CPU hasn't changed the value > > in between? > > The cpu knows because the cacheline w

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 03:30:44AM +0200, Segher Boessenkool wrote: > >>>Part of the motivation here is to fix heisenbugs. If I knew where > >>>they > >> > >>By the same token we should probably disable optimisations > >>altogether since that too can create heisenbugs. > > > >Precisely the point

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Wed, Aug 15, 2007 at 06:41:40PM -0700, Christoph Lameter wrote: > On Wed, 15 Aug 2007, Paul E. McKenney wrote: > > > Understood. My point is not that the impact is precisely zero, but > > rather that the impact on optimization is much less hurtful than the > > problems that could arise otherwi

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 10:11:05AM +0800, Herbert Xu wrote: > On Thu, Aug 16, 2007 at 12:05:56PM +1000, Paul Mackerras wrote: > > Herbert Xu writes: > > > > > See sk_stream_mem_schedule in net/core/stream.c: > > > > > > /* Under limit. */ > > > if (atomic_read(sk->sk_prot->memory_

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Satyam Sharma
On Thu, 16 Aug 2007, Satyam Sharma wrote: > On Thu, 16 Aug 2007, Paul Mackerras wrote: > > Herbert Xu writes: > > > > > See sk_stream_mem_schedule in net/core/stream.c: > > > > > > /* Under limit. */ > > > if (atomic_read(sk->sk_prot->memory_allocated) < > > > sk->sk_prot->sys

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Satyam Sharma writes: > I can't speak for this particular case, but there could be similar code > examples elsewhere, where we do the atomic ops on an atomic_t object > inside a higher-level locking scheme that would take care of the kind of > problem you're referring to here. It would be useful f

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Herbert Xu writes: > > Are you sure? How do you know some other CPU hasn't changed the value > > in between? > > Yes I'm sure, because we don't care if others have increased > the reservation. But others can also reduce the reservation. Also, the code sets and clears *sk->sk_prot->memory_press

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Paul Mackerras
Christoph Lameter writes: > > But I have to say that I still don't know of a single place > > where one would actually use the volatile variant. > > I suspect that what you say is true after we have looked at all callers. It seems that there could be a lot of places where atomic_t is used in a n

Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures

2007-08-15 Thread Herbert Xu
On Thu, Aug 16, 2007 at 01:23:06PM +1000, Paul Mackerras wrote: > > In particular, atomic_read seems to lend itself to buggy uses. People > seem to do things like: > > atomic_add(&v, something); > if (atomic_read(&v) > something_else) ... If you're referring to the code in sk_stream_

  1   2   3   >