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

2007-09-10 Thread Segher Boessenkool
"volatile" has nothing to do with reordering. atomic_dec() writes to memory, so it _does_ have "volatile semantics", implicitly, as long as the compiler cannot optimise the atomic variable away completely -- any store counts as a side effect. Stores can be reordered. Only x86 has (mostly) impli

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

2007-09-10 Thread Paul E. McKenney
On Mon, Sep 10, 2007 at 02:36:26PM -0700, Christoph Lameter wrote: > On Mon, 10 Sep 2007, Paul E. McKenney wrote: > > > The one exception to this being the case where process-level code is > > communicating to an interrupt handler running on that same CPU -- on > > all CPUs that I am aware of, a g

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

2007-09-10 Thread Christoph Lameter
On Mon, 10 Sep 2007, Paul E. McKenney wrote: > The one exception to this being the case where process-level code is > communicating to an interrupt handler running on that same CPU -- on > all CPUs that I am aware of, a given CPU always sees its own writes > in order. Yes but that is due to the c

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

2007-09-10 Thread Paul E. McKenney
On Mon, Sep 10, 2007 at 11:59:29AM -0700, Christoph Lameter wrote: > On Fri, 17 Aug 2007, Segher Boessenkool wrote: > > > "volatile" has nothing to do with reordering. atomic_dec() writes > > to memory, so it _does_ have "volatile semantics", implicitly, as > > long as the compiler cannot optimis

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

2007-09-10 Thread Kyle Moffett
On Sep 10, 2007, at 12:46:33, Denys Vlasenko wrote: My point is that people are confused as to what atomic_read() exactly means, and this is bad. Same for cpu_relax(). First one says "read", and second one doesn't say "barrier". Q&A: Q: When is it OK to use atomic_read()? A: You are ask

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

2007-09-10 Thread Christoph Lameter
On Mon, 10 Sep 2007, Linus Torvalds wrote: > The fact is, "volatile" *only* makes things worse. It generates worse > code, and never fixes any real bugs. This is a *fact*. Yes, lets just drop the volatiles now! We need a patch that gets rid of them Volunteers? - To unsubscribe from this l

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

2007-09-10 Thread Christoph Lameter
On Fri, 17 Aug 2007, Segher Boessenkool wrote: > "volatile" has nothing to do with reordering. atomic_dec() writes > to memory, so it _does_ have "volatile semantics", implicitly, as > long as the compiler cannot optimise the atomic variable away > completely -- any store counts as a side effect.

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

2007-09-10 Thread Denys Vlasenko
On Monday 10 September 2007 16:09, Linus Torvalds wrote: > On Mon, 10 Sep 2007, Denys Vlasenko wrote: > > static inline int > > qla2x00_wait_for_loop_ready(scsi_qla_host_t *ha) > > { > > int return_status = QLA_SUCCESS; > > unsigned long loop_timeout ; > > scsi_qla_host

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

2007-09-10 Thread Arjan van de Ven
On Mon, 10 Sep 2007 15:38:23 +0100 Denys Vlasenko <[EMAIL PROTECTED]> wrote: > On Monday 10 September 2007 15:51, Arjan van de Ven wrote: > > On Mon, 10 Sep 2007 11:56:29 +0100 > > Denys Vlasenko <[EMAIL PROTECTED]> wrote: > > > > > > > > Well, if you insist on having it again: > > > > > > Wait

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

2007-09-10 Thread Linus Torvalds
On Mon, 10 Sep 2007, Denys Vlasenko wrote: > > static inline int > qla2x00_wait_for_loop_ready(scsi_qla_host_t *ha) > { > int return_status = QLA_SUCCESS; > unsigned long loop_timeout ; > scsi_qla_host_t *pha = to_qla_parent(ha); > > /* wait for 5 min at the

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

2007-09-10 Thread Denys Vlasenko
On Monday 10 September 2007 15:51, Arjan van de Ven wrote: > On Mon, 10 Sep 2007 11:56:29 +0100 > Denys Vlasenko <[EMAIL PROTECTED]> wrote: > > > > > Well, if you insist on having it again: > > > > Waiting for atomic value to be zero: > > > > while (atomic_read(&x)) > >

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

2007-09-10 Thread Denys Vlasenko
On Monday 10 September 2007 14:38, Denys Vlasenko wrote: > You are basically trying to educate me how to use atomic properly. > You don't need to do it, as I am (currently) not a driver author. > > I am saying that people who are already using atomic_read() > (and who unfortunately did not read yo

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

2007-09-10 Thread Arjan van de Ven
On Mon, 10 Sep 2007 11:56:29 +0100 Denys Vlasenko <[EMAIL PROTECTED]> wrote: > > Well, if you insist on having it again: > > Waiting for atomic value to be zero: > > while (atomic_read(&x)) > continue; > and this I would say is buggy code all the way. Not from a pure

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

2007-09-10 Thread Denys Vlasenko
On Monday 10 September 2007 13:22, Kyle Moffett wrote: > On Sep 10, 2007, at 06:56:29, Denys Vlasenko wrote: > > On Sunday 09 September 2007 19:18, Arjan van de Ven wrote: > >> On Sun, 9 Sep 2007 19:02:54 +0100 > >> Denys Vlasenko <[EMAIL PROTECTED]> wrote: > >> > >>> Why is all this fixation on "v

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

2007-09-10 Thread Kyle Moffett
On Sep 10, 2007, at 06:56:29, Denys Vlasenko wrote: On Sunday 09 September 2007 19:18, Arjan van de Ven wrote: On Sun, 9 Sep 2007 19:02:54 +0100 Denys Vlasenko <[EMAIL PROTECTED]> wrote: Why is all this fixation on "volatile"? I don't think people want "volatile" keyword per se, they want ato

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

2007-09-10 Thread Herbert Xu
On Mon, Sep 10, 2007 at 11:56:29AM +0100, Denys Vlasenko wrote: > > Expecting every driver writer to remember that atomic_read is not in fact > a "read from memory" is naive. That won't happen. Face it, majority of > driver authors are a bit less talented than Ingo Molnar or Arjan van de Ven ;) >

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

2007-09-10 Thread Denys Vlasenko
On Sunday 09 September 2007 19:18, Arjan van de Ven wrote: > On Sun, 9 Sep 2007 19:02:54 +0100 > Denys Vlasenko <[EMAIL PROTECTED]> wrote: > > > Why is all this fixation on "volatile"? I don't think > > people want "volatile" keyword per se, they want atomic_read(&x) to > > _always_ compile into a

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

2007-09-09 Thread Arjan van de Ven
On Sun, 9 Sep 2007 19:02:54 +0100 Denys Vlasenko <[EMAIL PROTECTED]> wrote: > Why is all this fixation on "volatile"? I don't think > people want "volatile" keyword per se, they want atomic_read(&x) to > _always_ compile into an memory-accessing instruction, not register > access. and ... why is

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

2007-09-09 Thread Denys Vlasenko
On Friday 17 August 2007 17:48, Linus Torvalds wrote: > > 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 generat

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

2007-08-24 Thread Denys Vlasenko
On Friday 24 August 2007 18:15, Christoph Lameter wrote: > On Fri, 24 Aug 2007, Denys Vlasenko wrote: > > On Thursday 16 August 2007 00:22, Paul Mackerras wrote: > > > Satyam Sharma writes: > > > In the kernel we use atomic variables in precisely those situations > > > where a variable is potential

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

2007-08-24 Thread Linus Torvalds
On Fri, 24 Aug 2007, Denys Vlasenko wrote: > > > No, you don't use "x.counter++". But you *do* use > > > > if (atomic_read(&x) <= 1) > > > > and loading into a register is stupid and pointless, when you could just > > do it as a regular memory-operand to the cmp instruction. > > It doesn't m

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

2007-08-24 Thread Christoph Lameter
On Fri, 24 Aug 2007, Denys Vlasenko wrote: > On Thursday 16 August 2007 00:22, Paul Mackerras wrote: > > Satyam Sharma writes: > > 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

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

2007-08-24 Thread Denys Vlasenko
On Thursday 16 August 2007 00:22, Paul Mackerras wrote: > Satyam Sharma writes: > 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

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

2007-08-24 Thread Denys Vlasenko
On Saturday 18 August 2007 05:13, Linus Torvalds wrote: > On Sat, 18 Aug 2007, Satyam Sharma wrote: > > No code does (or would do, or should do): > > > > x.counter++; > > > > on an "atomic_t x;" anyway. > > That's just an example of a general problem. > > No, you don't use "x.counter++". But yo

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

2007-08-22 Thread Adrian Bunk
On Tue, Aug 21, 2007 at 06:51:16PM -0400, [EMAIL PROTECTED] wrote: > On Tue, 21 Aug 2007 09:16:43 PDT, "Paul E. McKenney" said: > > > I agree that instant gratification is hard to come by when synching > > up compiler and kernel versions. Nonetheless, it should be possible > > to create APIs that

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

2007-08-21 Thread Paul E. McKenney
On Tue, Aug 21, 2007 at 06:51:16PM -0400, [EMAIL PROTECTED] wrote: > On Tue, 21 Aug 2007 09:16:43 PDT, "Paul E. McKenney" said: > > > I agree that instant gratification is hard to come by when synching > > up compiler and kernel versions. Nonetheless, it should be possible > > to create APIs that

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

2007-08-21 Thread Valdis . Kletnieks
On Tue, 21 Aug 2007 09:16:43 PDT, "Paul E. McKenney" said: > I agree that instant gratification is hard to come by when synching > up compiler and kernel versions. Nonetheless, it should be possible > to create APIs that are are conditioned on the compiler version. We've tried that, sort of. Se

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

2007-08-21 Thread Linus Torvalds
On Tue, 21 Aug 2007, Chris Snook wrote: > > Moore's law is definitely working against us here. Register counts, pipeline > depths, core counts, and clock multipliers are all increasing in the long run. > At some point in the future, barrier() will be universally regarded as a > hammer too big f

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

2007-08-21 Thread Satyam Sharma
On Tue, 21 Aug 2007, Chris Snook wrote: > David Miller wrote: > > From: Linus Torvalds <[EMAIL PROTECTED]> > > Date: Mon, 20 Aug 2007 22:46:47 -0700 (PDT) > > > > > Ie a "barrier()" is likely _cheaper_ than the code generation downside > > > from using "volatile". > > > > Assuming GCC were eve

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

2007-08-21 Thread Paul E. McKenney
On Tue, Aug 21, 2007 at 04:48:51PM +0200, Segher Boessenkool wrote: > >>Let me say it more clearly: On ARM, it is impossible to perform atomic > >>operations on MMIO space. > > > >Actually, no one is suggesting that we try to do that at all. > > > >The discussion about RMW ops on MMIO space started

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

2007-08-21 Thread Segher Boessenkool
At some point in the future, barrier() will be universally regarded as a hammer too big for most purposes. Whether or not removing it now You can't just remove it, it is needed in some places; you want to replace it in most places with a more fine-grained "compiler barrier", I presume? constit

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

2007-08-21 Thread Segher Boessenkool
Let me say it more clearly: On ARM, it is impossible to perform atomic operations on MMIO space. Actually, no one is suggesting that we try to do that at all. The discussion about RMW ops on MMIO space started with a comment attributed to the gcc developers that one reason why gcc on x86 doesn'

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

2007-08-21 Thread Segher Boessenkool
And no, RMW on MMIO isn't "problematic" at all, either. An RMW op is a read op, a modify op, and a write op, all rolled into one opcode. But three actual operations. Maybe for some CPUs, but not all. ARM for instance can't use the load exclusive and store exclusive instructions to MMIO space.

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

2007-08-21 Thread Chris Snook
David Miller wrote: From: Linus Torvalds <[EMAIL PROTECTED]> Date: Mon, 20 Aug 2007 22:46:47 -0700 (PDT) Ie a "barrier()" is likely _cheaper_ than the code generation downside from using "volatile". Assuming GCC were ever better about the code generation badness with volatile that has been di

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

2007-08-21 Thread Andi Kleen
On Tue, Aug 21, 2007 at 07:33:49PM +1000, Paul Mackerras wrote: > So the whole discussion is irrelevant to ARM, PowerPC and any other > architecture except x86[-64]. It's even irrelevant on x86 because all modifying operations on atomic_t are coded in inline assembler and will always be RMW no ma

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

2007-08-21 Thread Paul Mackerras
Russell King writes: > Let me say it more clearly: On ARM, it is impossible to perform atomic > operations on MMIO space. Actually, no one is suggesting that we try to do that at all. The discussion about RMW ops on MMIO space started with a comment attributed to the gcc developers that one reas

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

2007-08-21 Thread Russell King
On Mon, Aug 20, 2007 at 05:05:18PM -0700, Paul E. McKenney wrote: > On Tue, Aug 21, 2007 at 01:02:01AM +0200, Segher Boessenkool wrote: > > >>And no, RMW on MMIO isn't "problematic" at all, either. > > >> > > >>An RMW op is a read op, a modify op, and a write op, all rolled > > >>into one opcode.

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

2007-08-21 Thread Russell King
On Tue, Aug 21, 2007 at 01:02:01AM +0200, Segher Boessenkool wrote: > >>And no, RMW on MMIO isn't "problematic" at all, either. > >> > >>An RMW op is a read op, a modify op, and a write op, all rolled > >>into one opcode. But three actual operations. > > > >Maybe for some CPUs, but not all. ARM f

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

2007-08-21 Thread David Miller
From: Linus Torvalds <[EMAIL PROTECTED]> Date: Mon, 20 Aug 2007 22:46:47 -0700 (PDT) > Ie a "barrier()" is likely _cheaper_ than the code generation downside > from using "volatile". Assuming GCC were ever better about the code generation badness with volatile that has been discussed here, I muc

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

2007-08-20 Thread Linus Torvalds
On Mon, 20 Aug 2007, Chris Snook wrote: > > What about barrier removal? With consistent semantics we could optimize a > fair amount of code. Whether or not that constitutes "premature" optimization > is open to debate, but there's no question we could reduce our register wiping > in some places

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

2007-08-20 Thread Paul E. McKenney
On Tue, Aug 21, 2007 at 01:02:01AM +0200, Segher Boessenkool wrote: > >>And no, RMW on MMIO isn't "problematic" at all, either. > >> > >>An RMW op is a read op, a modify op, and a write op, all rolled > >>into one opcode. But three actual operations. > > > >Maybe for some CPUs, but not all. ARM f

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

2007-08-20 Thread Segher Boessenkool
And no, RMW on MMIO isn't "problematic" at all, either. An RMW op is a read op, a modify op, and a write op, all rolled into one opcode. But three actual operations. Maybe for some CPUs, but not all. ARM for instance can't use the load exclusive and store exclusive instructions to MMIO space.

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

2007-08-20 Thread Russell King
On Tue, Aug 21, 2007 at 12:04:17AM +0200, Segher Boessenkool wrote: > And no, RMW on MMIO isn't "problematic" at all, either. > > An RMW op is a read op, a modify op, and a write op, all rolled > into one opcode. But three actual operations. Maybe for some CPUs, but not all. ARM for instance ca

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

2007-08-20 Thread Segher Boessenkool
Such code generally doesn't care precisely when it gets the update, just that the update is atomic, and it doesn't loop forever. Yes, it _does_ care that it gets the update _at all_, and preferably as early as possible. Regardless, I'm convinced we just need to do it all in assembly. So do y

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

2007-08-20 Thread Segher Boessenkool
Right. ROTFL... volatile actually breaks atomic_t instead of making it safe. x++ becomes a register load, increment and a register store. Without volatile we can increment the memory directly. It seems that volatile requires that the variable is loaded into a register first and then operated up

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

2007-08-20 Thread Chris Snook
Herbert Xu wrote: On Mon, Aug 20, 2007 at 09:15:11AM -0400, Chris Snook wrote: Linus Torvalds wrote: 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 t

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

2007-08-20 Thread Chris Snook
Christoph Lameter wrote: On Fri, 17 Aug 2007, Paul E. McKenney wrote: On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote: On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: gcc bugzilla bug #33102, for whatever that ends up being worth. ;-) I had totally forgotten tha

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

2007-08-20 Thread Herbert Xu
On Mon, Aug 20, 2007 at 09:15:11AM -0400, Chris Snook wrote: > Linus Torvalds wrote: > >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

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

2007-08-20 Thread Chris Snook
Stefan Richter wrote: Nick Piggin wrote: Stefan Richter wrote: Nick Piggin wrote: I don't know why people would assume volatile of atomics. AFAIK, most of the documentation is pretty clear that all the atomic stuff can be reordered etc. except for those that modify and return a value. Which

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

2007-08-20 Thread Chris Snook
Linus Torvalds wrote: 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. What about barrier removal? With consistent semantics we c

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

2007-08-18 Thread Paul E. McKenney
On Sat, Aug 18, 2007 at 03:41:13PM -0700, Linus Torvalds wrote: > > > On Sat, 18 Aug 2007, Paul E. McKenney wrote: > > > > One of the gcc guys claimed that he thought that the two-instruction > > sequence would be faster on some x86 machines. I pointed out that > > there might be a concern abou

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

2007-08-18 Thread Linus Torvalds
On Sat, 18 Aug 2007, Paul E. McKenney wrote: > > One of the gcc guys claimed that he thought that the two-instruction > sequence would be faster on some x86 machines. I pointed out that > there might be a concern about code size. I chose not to point out > that people might also care about the

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

2007-08-18 Thread Paul E. McKenney
On Fri, Aug 17, 2007 at 06:24:15PM -0700, Christoph Lameter wrote: > On Fri, 17 Aug 2007, Paul E. McKenney wrote: > > > On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote: > > > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: > > > > > > > > gcc bugzilla bug #33102, for w

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

2007-08-18 Thread Paul E. McKenney
On Fri, Aug 17, 2007 at 09:13:35PM -0700, Linus Torvalds wrote: > > > On Sat, 18 Aug 2007, Satyam Sharma wrote: > > > > No code does (or would do, or should do): > > > > x.counter++; > > > > on an "atomic_t x;" anyway. > > That's just an example of a general problem. > > No, you don't us

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

2007-08-18 Thread Stefan Richter
Nick Piggin wrote: > Stefan Richter wrote: >> Nick Piggin wrote: >> >>> I don't know why people would assume volatile of atomics. AFAIK, most >>> of the documentation is pretty clear that all the atomic stuff can be >>> reordered etc. except for those that modify and return a value. >> >> >> Which

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

2007-08-18 Thread Satyam Sharma
On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > > GCC manual, section 6.1, "When ^^ > > > is a Volatile Object Accessed?" doesn't say anything of the ^^^ > > > kind. ^ > > True, "impleme

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

2007-08-18 Thread Satyam Sharma
On Fri, 17 Aug 2007, Linus Torvalds wrote: > On Sat, 18 Aug 2007, Satyam Sharma wrote: > > > > No code does (or would do, or should do): > > > > x.counter++; > > > > on an "atomic_t x;" anyway. > > That's just an example of a general problem. > > No, you don't use "x.counter++". But you

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

2007-08-18 Thread Satyam Sharma
[ LOL, you _are_ shockingly petty! ] On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > > The documentation simply doesn't say "+m" is allowed. The code to > > > allow it was added for the benefit of people who do not read the > > > documentation. Documentation for "+m" might get added later i

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

2007-08-17 Thread Segher Boessenkool
The documentation simply doesn't say "+m" is allowed. The code to allow it was added for the benefit of people who do not read the documentation. Documentation for "+m" might get added later if it is decided this [the code, not the documentation] is a sane thing to have (which isn't directly obv

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

2007-08-17 Thread Linus Torvalds
On Sat, 18 Aug 2007, Satyam Sharma wrote: > > No code does (or would do, or should do): > > x.counter++; > > on an "atomic_t x;" anyway. That's just an example of a general problem. No, you don't use "x.counter++". But you *do* use if (atomic_read(&x) <= 1) and loading into a

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

2007-08-17 Thread Satyam Sharma
On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > > > > The "asm volatile" implementation does have exactly the same > > > > > reordering guarantees as the "volatile cast" thing, > > > > > > > > I don't think so. > > > > > > "asm volatile" creates a side effect. > > > > Yeah. > > > > > Side

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

2007-08-17 Thread Segher Boessenkool
The "asm volatile" implementation does have exactly the same reordering guarantees as the "volatile cast" thing, I don't think so. "asm volatile" creates a side effect. Yeah. Side effects aren't allowed to be reordered wrt sequence points. Yeah. This is exactly the same reason as why "

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Nick Piggin wrote: > Satyam Sharma wrote: > > > I didn't quite understand what you said here, so I'll tell what I think: > > > > * foo() is a compiler barrier if the definition of foo() is invisible to > > the compiler at a callsite. > > > > * foo() is also a compiler ba

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

2007-08-17 Thread Satyam Sharma
On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > > > > atomic_dec() writes > > > > > to memory, so it _does_ have "volatile semantics", implicitly, as > > > > > long as the compiler cannot optimise the atomic variable away > > > > > completely -- any store counts as a side effect. > > > > > >

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Christoph Lameter wrote: > On Fri, 17 Aug 2007, Paul E. McKenney wrote: > > > On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote: > > > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: > > > > > > > > gcc bugzilla bug #33102, for whatever that ends

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

2007-08-17 Thread Christoph Lameter
On Fri, 17 Aug 2007, Paul E. McKenney wrote: > On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote: > > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: > > > > > > gcc bugzilla bug #33102, for whatever that ends up being worth. ;-) > > > > I had totally forgotten that I'

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

2007-08-17 Thread Paul E. McKenney
On Sat, Aug 18, 2007 at 08:09:13AM +0800, Herbert Xu wrote: > On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: > > > > gcc bugzilla bug #33102, for whatever that ends up being worth. ;-) > > I had totally forgotten that I'd already filed that bug more > than six years ago until t

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

2007-08-17 Thread Segher Boessenkool
No it does not have any volatile semantics. atomic_dec() can be reordered at will by the compiler within the current basic unit if you do not add a barrier. "volatile" has nothing to do with reordering. If you're talking of "volatile" the type-qualifier keyword, then http://lkml.org/lkml/200

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

2007-08-17 Thread Herbert Xu
On Fri, Aug 17, 2007 at 04:59:12PM -0700, Paul E. McKenney wrote: > > gcc bugzilla bug #33102, for whatever that ends up being worth. ;-) I had totally forgotten that I'd already filed that bug more than six years ago until they just closed yours as a duplicate of mine :) Good luck in getting it

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

2007-08-17 Thread Segher Boessenkool
atomic_dec() writes to memory, so it _does_ have "volatile semantics", implicitly, as long as the compiler cannot optimise the atomic variable away completely -- any store counts as a side effect. I don't think an atomic_dec() implemented as an inline "asm volatile" or one that uses a "forget" m

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

2007-08-17 Thread Paul E. McKenney
On Thu, Aug 16, 2007 at 08:50:30PM -0700, Linus Torvalds wrote: > Just try it yourself: > > volatile int i; > int j; > > int testme(void) > { > return i <= 1; > } > > int testme2(void) > { > return j <= 1; > } > > and c

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

2007-08-17 Thread Segher Boessenkool
#define forget(a) __asm__ __volatile__ ("" :"=m" (a) :"m" (a)) [ This is exactly equivalent to using "+m" in the constraints, as recently explained on a GCC list somewhere, in response to the patch in my bitops series a few weeks back where I thought "+m" was bogus. ] [It wasn't ex

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

2007-08-17 Thread Satyam Sharma
On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > > > No it does not have any volatile semantics. atomic_dec() can be > > > > reordered > > > > at will by the compiler within the current basic unit if you do not add > > > > a > > > > barrier. > > > > > > "volatile" has nothing to do with reord

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

2007-08-17 Thread Satyam Sharma
On Sat, 18 Aug 2007, Segher Boessenkool wrote: > > #define forget(a) __asm__ __volatile__ ("" :"=m" (a) :"m" (a)) > > > > [ This is exactly equivalent to using "+m" in the constraints, as recently > > explained on a GCC list somewhere, in response to the patch in my bitops > > series a fe

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

2007-08-17 Thread Segher Boessenkool
#define forget(a) __asm__ __volatile__ ("" :"=m" (a) :"m" (a)) [ This is exactly equivalent to using "+m" in the constraints, as recently explained on a GCC list somewhere, in response to the patch in my bitops series a few weeks back where I thought "+m" was bogus. ] [It wasn't ex

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

2007-08-17 Thread Segher Boessenkool
Here, I should obviously admit that the semantics of *(volatile int *)& aren't any neater or well-defined in the _language standard_ at all. The standard does say (verbatim) "precisely what constitutes as access to object of volatile-qualified type is implementation-defined", but GCC does help u

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

2007-08-17 Thread Segher Boessenkool
Now the second wording *IS* technically correct, but come on, it's 24 words long whereas the original one was 3 -- and hopefully anybody reading the shorter phrase *would* have known anyway what was meant, without having to be pedantic about it :-) Well you were talking pretty formal (and detail

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

2007-08-17 Thread Segher Boessenkool
In a reasonable world, gcc should just make that be (on x86) addl $1,i(%rip) on x86-64, which is indeed what it does without the volatile. But with the volatile, the compiler gets really nervous, and doesn't dare do it in one instruction, and thus generates crap like movl

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

2007-08-17 Thread Segher Boessenkool
(and yes, it is perfectly legitimate to want a non-volatile read for a data type that you also want to do atomic RMW operations on) ...which is undefined behaviour in C (and GCC) when that data is declared volatile, which is a good argument against implementing atomics that way in itself. Segh

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

2007-08-17 Thread Segher Boessenkool
Of course, since *normal* accesses aren't necessarily limited wrt re-ordering, the question then becomes one of "with regard to *what* does it limit re-ordering?". A C compiler that re-orders two different volatile accesses that have a sequence point in between them is pretty clearly a buggy co

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

2007-08-17 Thread Paul E. McKenney
On Fri, Aug 17, 2007 at 12:49:00PM -0700, Arjan van de Ven wrote: > > On Fri, 2007-08-17 at 12:49 -0700, Paul E. McKenney wrote: > > > > What about reading values modified in interrupt handlers, as in your > > > > "random" case? Or is this a bug where the user of atomic_read() is > > > > invali

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

2007-08-17 Thread Arjan van de Ven
On Fri, 2007-08-17 at 12:49 -0700, Paul E. McKenney wrote: > > > What about reading values modified in interrupt handlers, as in your > > > "random" case? Or is this a bug where the user of atomic_read() is > > > invalidly expecting a read each time it is called? > > > > the interrupt handler

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

2007-08-17 Thread Paul E. McKenney
On Fri, Aug 17, 2007 at 11:54:33AM -0700, Arjan van de Ven wrote: > > On Fri, 2007-08-17 at 12:50 -0600, Chris Friesen wrote: > > Linus Torvalds wrote: > > > > > - in other words, the *only* possible meaning for "volatile" is a purely > > >single-CPU meaning. And if you only have a single C

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

2007-08-17 Thread Linus Torvalds
On Fri, 17 Aug 2007, Chris Friesen wrote: > > I assume you mean "except for IO-related code and 'random' values like > jiffies" as you mention later on? Yes. There *are* valid uses for "volatile", but they have remained the same for the last few years: - "jiffies" - internal per-architecture

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

2007-08-17 Thread Arjan van de Ven
On Fri, 2007-08-17 at 12:50 -0600, Chris Friesen wrote: > Linus Torvalds wrote: > > > - 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

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

2007-08-17 Thread Paul E. McKenney
On Sat, Aug 18, 2007 at 12:01:38AM +0530, Satyam Sharma wrote: > > > On Fri, 17 Aug 2007, Paul E. McKenney wrote: > > > On Fri, Aug 17, 2007 at 01:09:08PM +0530, Satyam Sharma wrote: > > > > > > On Thu, 16 Aug 2007, Paul E. McKenney wrote: > > > > > > > On Fri, Aug 17, 2007 at 07:59:02AM +0800

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

2007-08-17 Thread Chris Friesen
Linus Torvalds wrote: - 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 t

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Segher Boessenkool wrote: > > > atomic_dec() already has volatile behavior everywhere, so this is > > > semantically > > > okay, but this code (and any like it) should be calling cpu_relax() each > > > iteration through the loop, unless there's a compelling reason not to. >

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Paul E. McKenney wrote: > On Fri, Aug 17, 2007 at 01:09:08PM +0530, Satyam Sharma wrote: > > > > On Thu, 16 Aug 2007, Paul E. McKenney wrote: > > > > > On Fri, Aug 17, 2007 at 07:59:02AM +0800, Herbert Xu wrote: > > > > > > > > First of all, I think this illustrates that

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

2007-08-17 Thread Segher Boessenkool
atomic_dec() already has volatile behavior everywhere, so this is semantically okay, but this code (and any like it) should be calling cpu_relax() each iteration through the loop, unless there's a compelling reason not to. I'll allow that for some hardware drivers (possibly this one) such a co

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

2007-08-17 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-17 Thread Linus Torvalds
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 p

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

2007-08-17 Thread Paul E. McKenney
On Fri, Aug 17, 2007 at 01:09:08PM +0530, Satyam Sharma wrote: > > > On Thu, 16 Aug 2007, Paul E. McKenney wrote: > > > On Fri, Aug 17, 2007 at 07:59:02AM +0800, Herbert Xu wrote: > > > On Thu, Aug 16, 2007 at 09:34:41AM -0700, Paul E. McKenney wrote: > > > > > > > > The compiler can also reorde

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Nick Piggin wrote: > Satyam Sharma wrote: > > > On Fri, 17 Aug 2007, Nick Piggin wrote: > > > > > Because they should be thinking about them in terms of barriers, over > > > which the compiler / CPU is not to reorder accesses or cache memory > > > operations, rather than "

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

2007-08-17 Thread Nick Piggin
Satyam Sharma wrote: On Fri, 17 Aug 2007, Nick Piggin wrote: Satyam Sharma wrote: On Fri, 17 Aug 2007, Nick Piggin wrote: Satyam Sharma wrote: It is very obvious. msleep calls schedule() (ie. sleeps), which is always a barrier. Probably you didn't mean that, but no, schedule() is not

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Nick Piggin wrote: > Satyam Sharma wrote: > [...] > > You think both these are equivalent in terms of "looks": > > > > | > > while (!atomic_read(&v)) { | while (!atomic_read_xxx(&v)) { > > ...

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

2007-08-17 Thread Satyam Sharma
On Fri, 17 Aug 2007, Nick Piggin wrote: > Satyam Sharma wrote: > > On Fri, 17 Aug 2007, Nick Piggin wrote: > > > Satyam Sharma wrote: > > > > > > It is very obvious. msleep calls schedule() (ie. sleeps), which is > > > always a barrier. > > > > Probably you didn't mean that, but no, schedule()

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

2007-08-17 Thread Nick Piggin
Satyam Sharma wrote: On Fri, 17 Aug 2007, Nick Piggin wrote: Because they should be thinking about them in terms of barriers, over which the compiler / CPU is not to reorder accesses or cache memory operations, rather than "special" "volatile" accesses. This is obviously just a taste thin

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

2007-08-17 Thread Nick Piggin
Satyam Sharma wrote: On Fri, 17 Aug 2007, Nick Piggin wrote: 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)) { ...

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

2007-08-17 Thread Nick Piggin
Satyam Sharma wrote: On Fri, 17 Aug 2007, Nick Piggin wrote: Satyam Sharma wrote: It is very obvious. msleep calls schedule() (ie. sleeps), which is always a barrier. Probably you didn't mean that, but no, schedule() is not barrier because it sleeps. It's a barrier because it's invisible

  1   2   3   >