uninline init_waitqueue_*() functions

Subject: uninline init_waitqueue_*() functions
From: Ingo Molnar <[email protected]>

some wait.h inlines are way too large: init_waitqueue_entry() and init_waitqueue_func_entry() generate 20-30 bytes of inlined code per call site, and init_waitqueue_head() is 30-40 bytes (on x86).

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

Quoted 2 lines · Andrew Morton

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Quoted 7 lines

Quoted 2 lines

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Yes, but that lumps all three together. init_waitqueue_head() is obviously the porky one. And it's porkier with CONFIG_DEBUG_SPINLOCK and CONFIG_LOCKDEP, which isn't the case to optimise for.

With the debug options turned off, even init_waitqueue_head() becomes just three assignments, similar to init_waitqueue_entry() and init_waitqueue_func_entry(). All pretty marginal.

Quoted 11 lines · Andrew Morton

Quoted 7 lines

Quoted 2 lines

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Yes, but that lumps all three together. init_waitqueue_head() is obviously the porky one. And it's porkier with CONFIG_DEBUG_SPINLOCK and CONFIG_LOCKDEP, which isn't the case to optimise for.

true. I redid my tests with both lockdep and debug-spinlocks turned off:

  text            data    bss     dec             filename
  21172153        6077270 3081864 30331287        vmlinux.x32.after
  21198222        6077106 3081864 30357192        vmlinux.x32.before

with 851 callsites that's a 30.6 bytes win per call site (total 26K) - still not bad at all.

Quoted 8 lines

true. I redid my tests with both lockdep and debug-spinlocks turned off:

  text            data    bss     dec             filename
  21172153        6077270 3081864 30331287        vmlinux.x32.after
  21198222        6077106 3081864 30357192        vmlinux.x32.before

with 851 callsites that's a 30.6 bytes win per call site (total 26K) - still not bad at all.

and that was with CONFIG_CC_OPTIMIZE_FOR_SIZE enabled. With optimize-for-size disabled the win goes up to 32.6 bytes (total 28K).

Quoted 11 lines · Ingo Molnar

Quoted 8 lines

true. I redid my tests with both lockdep and debug-spinlocks turned off:

  text            data    bss     dec             filename
  21172153        6077270 3081864 30331287        vmlinux.x32.after
  21198222        6077106 3081864 30357192        vmlinux.x32.before

with 851 callsites that's a 30.6 bytes win per call site (total 26K) - still not bad at all.

and that was with CONFIG_CC_OPTIMIZE_FOR_SIZE enabled. With optimize-for-size disabled the win goes up to 32.6 bytes (total 28K).

i also tried a config with the best size settings (disabling FRAME_POINTER, enabling CC_OPTIMIZE_FOR_SIZE), and this gives:

  text            data    bss     dec         filename
  20777768        6076042 3081864 29935674    vmlinux.x32.size.before
  20748140        6076178 3081864 29906182    vmlinux.x32.size.after

or a 34.8 bytes win per callsite (29K total).

Quoted 17 lines

Quoted 9 lines

Quoted 6 lines

  text            data    bss     dec             filename
  21172153        6077270 3081864 30331287        vmlinux.x32.after
  21198222        6077106 3081864 30357192        vmlinux.x32.before

with 851 callsites that's a 30.6 bytes win per call site (total 26K) - still not bad at all.

and that was with CONFIG_CC_OPTIMIZE_FOR_SIZE enabled. With optimize-for-size disabled the win goes up to 32.6 bytes (total 28K).

i also tried a config with the best size settings (disabling FRAME_POINTER, enabling CC_OPTIMIZE_FOR_SIZE), and this gives:

  text            data    bss     dec         filename
  20777768        6076042 3081864 29935674    vmlinux.x32.size.before
  20748140        6076178 3081864 29906182    vmlinux.x32.size.after

Quoted 2 lines

or a 34.8 bytes win per callsite (29K total).

With gcc-4.1.0 on i686, uninlining those three functions as per the below patch _increases_ the allnoconfig vmlinux's .text from 833456 bytes to 833728.

Quoted 8 lines

Quoted 8 lines

i also tried a config with the best size settings (disabling FRAME_POINTER, enabling CC_OPTIMIZE_FOR_SIZE), and this gives:

  text            data    bss     dec         filename
  20777768        6076042 3081864 29935674    vmlinux.x32.size.before
  20748140        6076178 3081864 29906182    vmlinux.x32.size.after

Quoted 6 lines

Quoted 2 lines

or a 34.8 bytes win per callsite (29K total).

With gcc-4.1.0 on i686, uninlining those three functions as per the below patch _increases_ the allnoconfig vmlinux's .text from 833456 bytes to 833728.

that's just the effect of CONFIG_REGPARM and CONFIG_CC_OPTIMIZE_FOR_SIZE not being set in an allnoconfig. Once i set them the text size evens out:

 431348   60666   27276  519290   7ec7a vmlinux.x32.mini.before
 431359   60666   27276  519301   7ec85 vmlinux.x32.mini.after

compiling without CONFIG_REGPARM on i686 (if you care about text size) makes little sense. It penalizes function calls artificially.

OK, but what happened to the 35-bytes-per-callsite saving?

Quoted 1 line · Andrew Morton

Quoted 1 line

OK, but what happened to the 35-bytes-per-callsite saving?

I really don't think it existed.

Maybe there's something else going on. In particular, I wonder if sections like the "debug_loc" fection end up being counted towards text-size? They never actually get _loaded_, but they can be absolutely enormous if CONFIG_DEBUG_INFO is enabled.

Quoted 8 lines · Linus Torvalds

Quoted 2 lines · Andrew Morton

OK, but what happened to the 35-bytes-per-callsite saving?

I really don't think it existed.

Quoted 4 lines

Maybe there's something else going on. In particular, I wonder if sections like the "debug_loc" fection end up being counted towards text-size? They never actually get _loaded_, but they can be absolutely enormous if CONFIG_DEBUG_INFO is enabled.

i had CONFIG_DEBUG_INFO (and UNWIND_INFO) disabled in all these build tests.

i had CONFIG_DEBUG_INFO (and UNWIND_INFO) disabled in all these build tests.

Quoted 3 lines · Ingo Molnar

i had CONFIG_DEBUG_INFO (and UNWIND_INFO) disabled in all these build tests.

Good, because I just verified: those two together will on their own increase "text size" by about 17% for me.

I still think Andrew is right: I don't see how an initializer that should basically be three instructions can possibly be 35 bytes larger than a function call that should be a minimum of two instructions (argument setup in %eax and the actual call - and that's totally ignoring the deleterious effects of a function call on register liveness).

The fact that with allnoconfig the kernel is _smaller_ (but, quite franlkly, within the noise) with the inlined version would seem to back up Andrews position that it really shouldn't matter.

So I'm left wondering why it matters for you, and what triggers it. Maybe there is some secondary issue that could show us an even more interesting optimization (or some compiler behaviour that we should try to encourage).

Quoted 15 lines · Linus Torvalds

Quoted 3 lines · Ingo Molnar

i had CONFIG_DEBUG_INFO (and UNWIND_INFO) disabled in all these build tests.

Good, because I just verified: those two together will on their own increase "text size" by about 17% for me.

I still think Andrew is right: I don't see how an initializer that should basically be three instructions can possibly be 35 bytes larger than a function call that should be a minimum of two instructions (argument setup in %eax and the actual call - and that's totally ignoring the deleterious effects of a function call on register liveness).

Quoted 3 lines

The fact that with allnoconfig the kernel is _smaller_ (but, quite franlkly, within the noise) with the inlined version would seem to back up Andrews position that it really shouldn't matter.

well, the allnoconfig thing is artificial (and the uninteresting) for a number of reasons:

- it has REGPARM disabled which penalizes function calls

- it's UP and hence the inlining cost of init_wait_queue_head() is significantly smaller.

- allnoconfig has smaller average function size - increasing the cost of uninlining

Quoted 4 lines

So I'm left wondering why it matters for you, and what triggers it. Maybe there is some secondary issue that could show us an even more interesting optimization (or some compiler behaviour that we should try to encourage).

yeah, i'd not want to skip over some interesting and still unexplained effect either, but 35 bytes isnt all that outlandish and from everything i've seen it's a real win. Here is an actual example:

 c0fb6137:       c7 44 24 08 00 00 00    movl   $0x0,0x8(%esp)
 c0fb613e:       00
 c0fb613f:       c7 44 24 08 01 00 00    movl   $0x1,0x8(%esp)
 c0fb6146:       00
 c0fb6147:       c7 43 60 00 00 00 00    movl   $0x0,0x60(%ebx)
 c0fb614e:       8b 44 24 08             mov    0x8(%esp),%eax
 c0fb6152:       89 43 5c                mov    %eax,0x5c(%ebx)
 c0fb6155:       8d 43 64                lea    0x64(%ebx),%eax
 c0fb6158:       89 40 04                mov    %eax,0x4(%eax)
 c0fb615b:       89 43 64                mov    %eax,0x64(%ebx)

versus:

 c0fb070e:       8d 43 5c                lea    0x5c(%ebx),%eax
 c0fb0711:       e8 94 98 18 ff          call   c0139faa <init_waitqueue_head>

so 39 bytes versus 8 bytes - 31 bytes saved. It's a similar win in other cases i checked too. (the only exception is for smaller functions that i mentioned before: where the parameters are not pre-calculated yet so there's no good integration for the function call. In that case it's break even, or in some cases a 3-4 bytes loss.)

Quoted 2 lines

well, the allnoconfig thing is artificial (and the uninteresting) for a number of reasons:

hm, I'd have to say that allyesconfig is also artificial and the savings numbers are somewhat uninteresting in that case too.

Quoted 3 lines · Randy.Dunlap

Quoted 2 lines

well, the allnoconfig thing is artificial (and the uninteresting) for a number of reasons:

Quoted 2 lines

hm, I'd have to say that allyesconfig is also artificial and the savings numbers are somewhat uninteresting in that case too.

well the 'allyesconfig' isnt the true allyesconfig but one with most debugging options disabled. It is quite close to a typical distro config
- hence very much relevant. (I wanted to use something that is easy to reproduce.) Believe me, for large configs the savings are real.

Quoted 4 lines

well the 'allyesconfig' isnt the true allyesconfig but one with most debugging options disabled. It is quite close to a typical distro config
- hence very much relevant. (I wanted to use something that is easy to reproduce.) Believe me, for large configs the savings are real.

I would expect distros to use something close to all*mod*config (with some debug options disabled).

Quoted 3 lines · Randy.Dunlap

Quoted 2 lines

well, the allnoconfig thing is artificial (and the uninteresting) for a number of reasons:

Quoted 2 lines

hm, I'd have to say that allyesconfig is also artificial and the savings numbers are somewhat uninteresting in that case too.

well the 'allyesconfig' isnt the true allyesconfig but one with most debugging options disabled. It is quite close to a typical distro config
- hence very much relevant. (I wanted to use something that is easy to reproduce.) Believe me, for large configs the savings are real.

Ingo

yeah, i'd not want to skip over some interesting and still unexplained effect either, but 35 bytes isnt all that outlandish and from everything i've seen it's a real win. Here is an actual example:

 c0fb6137:       c7 44 24 08 00 00 00    movl   $0x0,0x8(%esp)
 c0fb613e:       00
 c0fb613f:       c7 44 24 08 01 00 00    movl   $0x1,0x8(%esp)
 c0fb6146:       00
 c0fb6147:       c7 43 60 00 00 00 00    movl   $0x0,0x60(%ebx)
 c0fb614e:       8b 44 24 08             mov    0x8(%esp),%eax
 c0fb6152:       89 43 5c                mov    %eax,0x5c(%ebx)
 c0fb6155:       8d 43 64                lea    0x64(%ebx),%eax
 c0fb6158:       89 40 04                mov    %eax,0x4(%eax)
 c0fb615b:       89 43 64                mov    %eax,0x64(%ebx)

Quoted 5 lines · Ingo Molnar

yeah, i'd not want to skip over some interesting and still unexplained effect either, but 35 bytes isnt all that outlandish and from everything i've seen it's a real win. Here is an actual example:

Quoted 10 lines

 c0fb6137:       c7 44 24 08 00 00 00    movl   $0x0,0x8(%esp)
 c0fb613e:       00
 c0fb613f:       c7 44 24 08 01 00 00    movl   $0x1,0x8(%esp)
 c0fb6146:       00
 c0fb6147:       c7 43 60 00 00 00 00    movl   $0x0,0x60(%ebx)
 c0fb614e:       8b 44 24 08             mov    0x8(%esp),%eax
 c0fb6152:       89 43 5c                mov    %eax,0x5c(%ebx)
 c0fb6155:       8d 43 64                lea    0x64(%ebx),%eax
 c0fb6158:       89 40 04                mov    %eax,0x4(%eax)
 c0fb615b:       89 43 64                mov    %eax,0x64(%ebx)

Ahh, it's _that_ old gcc problem.

That's actually a different thing.

Gcc is HORRIBLY BAD at doing the simple

	some_structure = (struct somestruct) { INITIAL };

assignments. It is so ludicrously bad that it's sad. It tends to do that as a local "struct somestruct" on the stack that gets initialized, followed by a memcpy().

In this case, the problem appears to be the spinlock initialization code.

In other words, I suspect 90% of your improvement was because you got that braindamage out of line.

It would be _much_ better to just fix "spin_lock_init()" instead. That would help a lot of _other_ users too, not just the waitqueue initializations.

Making that a real function (and inline only for the non-debug case, at which point it's just a simple and small store) would be much better.

Quoted 8 lines

 c0fb6137:       c7 44 24 08 00 00 00    movl   $0x0,0x8(%esp)
 c0fb613e:       00
 c0fb613f:       c7 44 24 08 01 00 00    movl   $0x1,0x8(%esp)
 c0fb6146:       00
 c0fb6147:       c7 43 60 00 00 00 00    movl   $0x0,0x60(%ebx)
 c0fb614e:       8b 44 24 08             mov    0x8(%esp),%eax
 c0fb6152:       89 43 5c                mov    %eax,0x5c(%ebx)

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

Quoted 4 lines · Linus Torvalds

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

yeah. I tried this and it indeed slashed 42K off text size (0.2%):

 text            data    bss     dec             filename
 20779489        6073834 3075176 29928499        vmlinux.volatile
 20736884        6073834 3075176 29885894        vmlinux.non-volatile

i booted the resulting allyesconfig bzImage and everything seems to be working fine. Find patch below.

Subject: spinlocks: remove 'volatile'
From: Ingo Molnar <[email protected]>

remove 'volatile' from the spinlock types, it causes gcc to generate really bad code. (and it's pointless anyway)

this reduces the non-debug SMP kernel's size by 0.2% (!).

Signed-off-by Ingo Molnar <[email protected]>

---
 include/asm-i386/spinlock_types.h   |    4 ++--
 include/asm-x86_64/spinlock_types.h |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Index: linux/include/asm-i386/spinlock_types.h
===================================================================

include/asm-i386/spinlock_types.h +0 -0

@@ -6,13 +6,13 @@
 #endif
 
 typedef struct {

Shouldn't the __raw_read_can_lock and __raw_write_can_lock macros be changed too, just to make sure the value gets read every single time if it's used in a loop? Just like the __raw_spin_is_locked already has a (volatile signed char * cast)? -

Quoted 4 lines · Linus Torvalds

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

yeah. I tried this and it indeed slashed 42K off text size (0.2%):

text            data    bss     dec             filename
20779489        6073834 3075176 29928499        vmlinux.volatile
20736884        6073834 3075176 29885894        vmlinux.non-volatile

i booted the resulting allyesconfig bzImage and everything seems to be working fine. Find patch below.

Ingo

------------------>
Subject: spinlocks: remove 'volatile'
From: Ingo Molnar <[email protected]>

remove 'volatile' from the spinlock types, it causes gcc to generate really bad code. (and it's pointless anyway)

this reduces the non-debug SMP kernel's size by 0.2% (!).

Signed-off-by Ingo Molnar <[email protected]>

---

include/asm-i386/spinlock_types.h   |    4 ++--
include/asm-x86_64/spinlock_types.h |    4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

Index: linux/include/asm-i386/spinlock_types.h
===================================================================

include/asm-i386/spinlock_types.h +2 -2

@@ -6,13 +6,13 @@
 #endif
 
 typedef struct {
-	volatile unsigned int slock;
+	unsigned int slock;
 } raw_spinlock_t;
 
 #define __RAW_SPIN_LOCK_UNLOCKED	{ 1 }
 
 typedef struct {
-	volatile unsigned int lock;
+	unsigned int lock;
 } raw_rwlock_t;
 
 #define __RAW_RW_LOCK_UNLOCKED		{ RW_LOCK_BIAS }

Index: linux/include/asm-x86_64/spinlock_types.h
===================================================================

include/asm-x86_64/spinlock_types.h +2 -3

@@ -6,13 +6,13 @@
 #endif
 
 typedef struct {
-	volatile unsigned int slock;
+	unsigned int slock;
 } raw_spinlock_t;
 
 #define __RAW_SPIN_LOCK_UNLOCKED	{ 1 }
 
 typedef struct {
-	volatile unsigned int lock;
+	unsigned int lock;
 } raw_rwlock_t;
 
 #define __RAW_RW_LOCK_UNLOCKED		{ RW_LOCK_BIAS }
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 3 lines · Ingo Molnar

* Linus Torvalds <[email protected]> wrote:

Quoted 4 lines

Quoted 4 lines

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

Then GCC must be fixed. The keyword volatile is correct. It should force the compiler to read the variable every time it's used.

This is not pointless. If GCC generates bad code, tell the GCC people. The volatile keyword is essential.

Quoted 5 lines · linux-os (Dick Johnson)

Quoted 3 lines · Ingo Molnar

* Linus Torvalds <[email protected]> wrote:

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

Then GCC must be fixed. The keyword volatile is correct. It should force the compiler to read the variable every time it's used.

this is not really what the C standard says.

Quoted 2 lines

This is not pointless. If GCC generates bad code, tell the GCC people. The volatile keyword is essential.

no the "volatile" semantics are vague, trecherous and evil. It's a LOT better to insert the well defined "barrier()" in the right places.

Quoted 18 lines · Arjan van de Ven

Quoted 5 lines · linux-os (Dick Johnson)

Quoted 3 lines · Ingo Molnar

* Linus Torvalds <[email protected]> wrote:

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

Then GCC must be fixed. The keyword volatile is correct. It should force the compiler to read the variable every time it's used.

this is not really what the C standard says.

Quoted 5 lines

Quoted 2 lines

This is not pointless. If GCC generates bad code, tell the GCC people. The volatile keyword is essential.

no the "volatile" semantics are vague, trecherous and evil. It's a LOT better to insert the well defined "barrier()" in the right places.

Look at:

http://en.wikipedia.org/wiki/Volatile_variable

This is just what is needed to prevent the compiler from making non working code during optimization.

Also look at:

http://en.wikipedia.org/wiki/Memory_barrier

This is used to prevent out-of-order execution, not at all what is necessary.

Quoted 26 lines · linux-os (Dick Johnson)

Quoted 18 lines · Arjan van de Ven

Quoted 5 lines · linux-os (Dick Johnson)

Quoted 3 lines · Ingo Molnar

* Linus Torvalds <[email protected]> wrote:

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

I wonder if we should remove the "volatile". There really isn't anything _good_ that gcc can do with it, but we've seen gcc code generation do stupid things before just because "volatile" seems to just disable even proper normal working.

Then GCC must be fixed. The keyword volatile is correct. It should force the compiler to read the variable every time it's used.

this is not really what the C standard says.

Quoted 5 lines

Quoted 2 lines

This is not pointless. If GCC generates bad code, tell the GCC people. The volatile keyword is essential.

no the "volatile" semantics are vague, trecherous and evil. It's a LOT better to insert the well defined "barrier()" in the right places.

Quoted 6 lines

Look at:

http://en.wikipedia.org/wiki/Volatile_variable

This is just what is needed to prevent the compiler from making non working code during optimization.

and an entry level document at wikipedia is more important than the C standard ;)

Quoted 3 lines

Also look at:

Quoted 4 lines

http://en.wikipedia.org/wiki/Memory_barrier

This is used to prevent out-of-order execution, not at all what is necessary.

I did not talk about memory barriers. In fact, barrier() is NOT a memory barrier. It's a compiler optimization barrier!

Quoted 4 lines

I did not talk about memory barriers. In fact, barrier() is NOT a memory barrier. It's a compiler optimization barrier!

// Read 10 samples from 2 A/D converters.

int* ina;
int a[10];
int* inb;
int b[10];

for (int i=0; i<10; i++)

{
	a[i] = *ina;
	barrier();
	b[i] = *inb;
}

The barrier prevents the compiler of translating this to:

for (int i=0; i<10; i++)

{
	b[i] = *inb;
	a[i] = *ina;
}

or even to:

for (int i=0; i<10; i++)

	a[i] = *ina;
for (int i=0; i<10; i++)
	b[i] = *inb;

but does not prevent it to do this:

// Read 10 samples from 2 A/D converters.

int* ina;
int a[10];
int* inb;
int b[10];

for (int i=0; i<10; i++)

{
	a[i] = *ina;
	barrier();
	b[i] = *inb;
}

The barrier prevents the compiler of translating this to:

for (int i=0; i<10; i++)

{
	b[i] = *inb;
	a[i] = *ina;
}

or even to:

for (int i=0; i<10; i++)

	a[i] = *ina;
for (int i=0; i<10; i++)
	b[i] = *inb;

but does not prevent it to do this:

register int tmp_a = *ina;
register int tmp_b = *inb;

for (int i=0; i<10; i++)

{
	a[i] = tmp_a;
	b[i] = tmp_b;
}

because nor 'ina' nor 'inb' change under what the compiler sees inside the loop. 'volatile' prevents the compiler of do a high level cache of *ina or *inb.

If your assembler instructions access memory in an unpredictable fashion, add `memory' to the list of clobbered registers. This will cause GCC to not keep memory values cached in registers across the assembler instruction and not optimize stores or loads to that memory. You will also want to add the volatile keyword if the memory affected is not listed in the inputs or outputs of the asm, as the memory' clobber does not count as a side-effect of the asm. If you know how large the accessed memory is, you can add it as input or output but if this is not known, you should add memory'. As an example, if you access ten bytes of a string, you can use a memory input like:

The barrier prevents the compiler of translating this to:

for (int i=0; i<10; i++)

{
	b[i] = *inb;
	a[i] = *ina;
}

or even to:

for (int i=0; i<10; i++)

	a[i] = *ina;
for (int i=0; i<10; i++)
	b[i] = *inb;

but does not prevent it to do this:

register int tmp_a = *ina;
register int tmp_b = *inb;

for (int i=0; i<10; i++)

{
	a[i] = tmp_a;
	b[i] = tmp_b;
}

because nor 'ina' nor 'inb' change under what the compiler sees inside the loop. 'volatile' prevents the compiler of do a high level cache of *ina or *inb.

http://en.wikipedia.org/wiki/Memory_barrier

This is used to prevent out-of-order execution, not at all what is necessary.

Look at:

http://en.wikipedia.org/wiki/Volatile_variable

This is just what is needed to prevent the compiler from making non working
code during optimization.

this is not really what the C standard says.

I'm still browsing a copy here, but so far have only really found this:

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Quoted 5 lines · Mark Lord

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Note that the "reordered" is totally pointless.

The _hardware_ will re-order accesses. Which is the whole point. "volatile" is basically never sufficient in itself.

The "reordered" thing really only matters on SMP machines, no?

Quoted 2 lines · Chris Friesen

The "reordered" thing really only matters on SMP machines, no?

Also (very much!) for device drivers.

As long as you're not talking to external devices, each cpu must be coherent with respect to itself, no? It's allowed to execute out-of-order, but it needs to make sure that by doing so it doesn't cause changes that are visible to software.

Quoted 9 lines · Linus Torvalds

Quoted 5 lines · Mark Lord

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Quoted 4 lines

Note that the "reordered" is totally pointless.

The _hardware_ will re-order accesses. Which is the whole point. "volatile" is basically never sufficient in itself.

The "reordered" thing really only matters on SMP machines, no? In which case (for userspace) the locking mechanisms (mutexes, etc.) should do The Right Thing to ensure visibility between cpus.

The C standard requires the use of volatile for signal handlers and setjmp.

For userspace at least the whole discussion of "barriers" is sort of moot--there are no memory barriers defined in the C language, which makes it kind of hard to write portable code that uses them.

Quoted 7 lines · Linus Torvalds

Quoted 5 lines · Mark Lord

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Quoted 3 lines

Note that the "reordered" is totally pointless. The _hardware_ will re-order accesses. Which is the whole point. "volatile" is basically never sufficient in itself.

The "reordered" thing really only matters on SMP machines, no?

The "reordered" thing really only matters on SMP machines, no? In which

The C standard requires the use of volatile for signal handlers and setjmp.

For userspace at least the whole discussion of "barriers" is sort of moot--there are no memory barriers defined in the C language, which makes it kind of hard to write portable code that uses them.

Quoted 1 line · Chris Friesen

The C standard requires the use of volatile for signal handlers and setjmp.

Actually, the C standard requires "sigatomic_t".

Quoted 1 line · Chris Friesen

The C standard requires the use of volatile for signal handlers and setjmp.

Actually, the C standard requires "sigatomic_t".

Quoted 3 lines

For userspace at least the whole discussion of "barriers" is sort of moot--there are no memory barriers defined in the C language, which makes it kind of hard to write portable code that uses them.

Any locking primitive BY DEFINITION has a barrier in it.

If it doesn't, it's not a locking primitive, it's a random sequence of code that does something pointless.

But for someone coding to POSIX, is there any reason why they would use barriers? If so, what API would they use?

Quoted 8 lines · Mark Lord

I'm still browsing a copy here, but so far have only really found this:

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Note that the "reordered" is totally pointless.

The _hardware_ will re-order accesses. Which is the whole point. "volatile" is basically never sufficient in itself.

So the definition of "volatile" literally made sense three or four decades ago. It's not sensible any more.

Quoted 11 lines · Linus Torvalds

Quoted 8 lines · Mark Lord

I'm still browsing a copy here, but so far have only really found this:

A volatile declaration may be used to describe an object corresponding to a memory-mapped input/output port or an object accessed by an aysnchronously interrupting function. Actions on objects so declared shall not be "optimized out" by an implementation or reordered except as permitted by the rules for evaluating expressions.

Quoted 9 lines

Note that the "reordered" is totally pointless.

The _hardware_ will re-order accesses. Which is the whole point. "volatile" is basically never sufficient in itself.

So the definition of "volatile" literally made sense three or four decades
ago. It's not sensible any more.

It could be argued that gcc's implementation of volatile is wrong, and that gcc should add the appropriate serializing instructions before and after volatile accesses.

Of course, that would make volatile even more suboptimal, but at least correct.

Quoted 6 lines

It could be argued that gcc's implementation of volatile is wrong, and that gcc should add the appropriate serializing instructions before and after volatile accesses.

Quoted 2 lines

Of course, that would make volatile even more suboptimal, but at least correct.

with PCI, and the PCI posting rules, there is no "one" serializing instruction, you need to know the specifics of the device in question to cause the flush. So at least there is no universal possible implementation of volatile as you suggest ;-)

Quoted 6 lines · Arjan van de Ven

Quoted 6 lines

It could be argued that gcc's implementation of volatile is wrong, and that gcc should add the appropriate serializing instructions before and after volatile accesses.

Quoted 8 lines

Quoted 2 lines

Of course, that would make volatile even more suboptimal, but at least correct.

with PCI, and the PCI posting rules, there is no "one" serializing instruction, you need to know the specifics of the device in question to cause the flush. So at least there is no universal possible implementation of volatile as you suggest ;-)

A serializing volatile makes it possible to write portable code to access pci mmio. You'd just follow a write with a read or whatever the rules say.

Quoted 4 lines

Quoted 4 lines

with PCI, and the PCI posting rules, there is no "one" serializing instruction, you need to know the specifics of the device in

question to

Quoted 4 lines

Quoted 3 lines

cause the flush. So at least there is no universal possible implementation of volatile as you suggest ;-)

Quoted 3 lines

A serializing volatile makes it possible to write portable code to access pci mmio. You'd just follow a write with a read or whatever the rules say.

yeah except that the compiler cannot know what to read; reading back the same memory location is NOT correct nor safe. It's device specific, for some devices it'll be safe, for others you have to read some OTHER location.

I didn't suggest the compiler could or should do it, just that it would be possible (for the _user_) to write portable ISO C code to access PCI mmio registers, if volatile's implementation serialized access.

I didn't suggest the compiler could or should do it, just that it would be possible (for the _user_) to write portable ISO C code to access PCI mmio registers, if volatile's implementation serialized access.

With the current implementation of volatile in gcc, it is impossible - you need to resort to inline assembly for some architectures, which is not an ISO C feature.

And I'm not suggesting that it would be a good idea to use volatile even if it was corrected - it would have to take a worst-case approach and thus would generate very bad code.

Quoted 4 lines

I didn't suggest the compiler could or should do it, just that it would be possible (for the _user_) to write portable ISO C code to access PCI mmio registers, if volatile's implementation serialized access.

That isn't possible neither. How to actually serialize access can require different set of primitives depending on the storage class of the memory you are accessing, which the C compiler has 0 knowledge about. (For example, cacheable storage vs. write-through vs. non-cacheable guarded vs. non-cacheable non-guarded on powerpc, there are different issues on other architectures).

Quoted 3 lines

With the current implementation of volatile in gcc, it is impossible - you need to resort to inline assembly for some architectures, which is not an ISO C feature.

ISO C has never been about writing device drivers. There is simply no choice here. You need an atchitecture specific set of accessors. If you want portable code, then pick a library like libpci and make sure it contains all you need on all the architectures you need. Then write portable code on top of it.

Quoted 1 line

And I'm not suggesting that it would be a good idea to use volatile

even

Quoted 2 lines

if it was corrected - it would have to take a worst-case approach and thus would generate very bad code.

So what is the point ?

Volatile is useful for non device driver work, for example VJ-style channels. A portable volatile can help to code such things in a compiler-neutral and platform-neutral way. Linux doesn't care about compiler neutrality, being coded in GNU C, and about platform neutrality, having a per-arch abstraction layer, but other programs may wish to run on multiple compilers and multiple platforms without per-platform glue layers.

Quoted 7 lines

Volatile is useful for non device driver work, for example VJ-style channels. A portable volatile can help to code such things in a compiler-neutral and platform-neutral way. Linux doesn't care about compiler neutrality, being coded in GNU C, and about platform neutrality, having a per-arch abstraction layer, but other programs may wish to run on multiple compilers and multiple platforms without per-platform glue layers.

There is a portable volatile, it's called 'pthread_mutex_lock'. It allows
you to code such things in a compiler-neutral and platform-neutral way. You don't have to worry about what the compiler might do, what the hardware might do, what atomic operations the CPU supports, or anything like that. The atomicity issues I've mentioned in my other posts make any attempt at creating a 'portable volatile' for shared memory more or less doomed from the start.

Quoted 8 lines · David Schwartz

Quoted 7 lines

Volatile is useful for non device driver work, for example VJ-style channels. A portable volatile can help to code such things in a compiler-neutral and platform-neutral way. Linux doesn't care about compiler neutrality, being coded in GNU C, and about platform neutrality, having a per-arch abstraction layer, but other programs may wish to run on multiple compilers and multiple platforms without per-platform glue layers.

Quoted 7 lines

There is a portable volatile, it's called 'pthread_mutex_lock'. It allows
you to code such things in a compiler-neutral and platform-neutral way. You don't have to worry about what the compiler might do, what the hardware might do, what atomic operations the CPU supports, or anything like that. The atomicity issues I've mentioned in my other posts make any attempt at creating a 'portable volatile' for shared memory more or less doomed from the start.

The other thing to add here is that if you're outside of the kernel, you *don't* want to be implementing your own spinlock if for no other reason than it's a performance disaster. The scheduler doesn't know that you are spinning waiting for a lock, so you will be scheduled and burning cpu time (and energy, and heat budget) while waiting for the the lock. I once spent over a week on-site at a customer complaining about Linux performance problems, and it turned out the reason why was because they thought they were being "smart" by implementing their own spinlock in userspace. Bad bad bad.

So if a userspace progam ever uses volatile, it's almost certainly a bug, one way or another.

Quoted 2 lines

So if a userspace progam ever uses volatile, it's almost certainly a bug, one way or another.

Without 'volatile' and disabling optimizations altogether, how do we prevent gcc from optimizing away pointers? As can be seen on
http://wiki.rubygarden.org/Ruby/page/show/GCAndExtensions (at 'Compiler over-optimisations and "volatile"'), volatile is used to prevent a specific type of optimization. This is because of the garbage collector, which scans the stack and registers to find referenced objects. So you don't want local variables containing references to objects optimized away.

So this would tend to confirm the rule of thumb: use of "volatile" in a userspace progam tends to indicate a bug.

- Ted

Yes, I use vfork. So far, the only way I have found for the parent to know whether or not the child's exec() failed is this way:

Quoted 2 lines · Joshua Hudson

Yes, I use vfork. So far, the only way I have found for the parent to know whether or not the child's exec() failed is this way:

What I do in UML is make a pipe between the parent and child, set it CLOEXEC, and write a byte down it if the exec fails.

The parent reads it - if it gets no bytes, the exec succeeded, if it gets one byte, it failed.

I usually do the same, except that I write the errno in case of failure. This way the parent knows *why* exec failed ;-)

Quoted 5 lines

So this would tend to confirm the rule of thumb: use of "volatile" in a userspace progam tends to indicate a bug.

- Ted

No. vfork(), setjmp(), signal().

Yes, I use vfork. So far, the only way I have found for the parent to know whether or not the child's exec() failed is this way:

volatile int failed;
pid_t pid;

failed = 0;
if (0 == (pid = vfork())) {
execve(argv[0], argv, envp);
failed = errno;
_exit(0);
}
if (pid < 0) {
/* can't fork */
}
if (failed) {
/* wait for pid (clean up zombie) */
errno = failed;
/* can't exec: update state */
}

What's wrong with _exit(exec() == -1 ? 0 : errno); and picking up the status with wait(2) ?

Quoted 3 lines

What's wrong with _exit(exec() == -1 ? 0 : errno); and picking up the status with wait(2) ?

The exec'd application may return regular error codes, which would interfere. IIRC /usr/sbin/useradd has different exit codes depending on what failed (providing some option, failure to create account, failure to create home dir, etc.). Now if you exit(errno) instead, you have an overlap.

And your code is somewhat wrong. Given that exec() would stand for execve(someprogram_and_args_here), if it returned -1 you would return 0, indicating success. Can't be. And if exec() does not return -1, which it never should, you return errno, which never reaches anyone.

Quoted 9 lines · Jan Engelhardt

Quoted 3 lines

What's wrong with _exit(exec() == -1 ? 0 : errno); and picking up the status with wait(2) ?

The exec'd application may return regular error codes, which would interfere. IIRC /usr/sbin/useradd has different exit codes depending on what failed (providing some option, failure to create account, failure to create home dir, etc.). Now if you exit(errno) instead, you have an overlap.

You're right. Maybe you could return -ve or with a high bit set, but I guess you may not know what the app will return.

But I don't see how the volatile or pipe solutions are any better
though: it would seem that both result in undefined behaviour according to my vfork man page. At least the wait() solution is defined (and workable, if you know what the target might return).

But I don't see how the volatile or pipe solutions are any better
though: it would seem that both result in undefined behaviour according to my vfork man page.

Then GCC must be fixed. The keyword volatile is correct. It should force the compiler to read the variable every time it's used.

Any other use of "volatile" is almost certainly a bug, or just useless.

Quoted 1 line · Linus Torvalds

Any other use of "volatile" is almost certainly a bug, or just useless.

Side note: it's also totally possible that a volatiles _hides_ a bug, ie removing the volatile ends up having bad effects, but that's because the software itself isn't actually following the rules (or, more commonly, the rules are broken, and somebody added "volatile" to hide the problem).

That's not just a theoretical notion, btw. We had _tons_ of these kinds of "volatile"s in the original old networking code. They were _all_ wrong. Every single one.

Quoted 2 lines · Linus Torvalds

Any other use of "volatile" is almost certainly a bug, or just useless.

Side note: it's also totally possible that a volatiles _hides_ a bug, ie removing the volatile ends up having bad effects, but that's because the software itself isn't actually following the rules (or, more commonly, the rules are broken, and somebody added "volatile" to hide the problem).

That's not just a theoretical notion, btw. We had _tons_ of these kinds of "volatile"s in the original old networking code. They were _all_ wrong. Every single one.

Linus

Linus, you may have been reading too many novels.

If I have some code that does:

extern int spinner;

funct(){
     while(spinner)
         ;

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

Quoted 11 lines · linux-os (Dick Johnson)

Linus, you may have been reading too many novels.

If I have some code that does:

extern int spinner;

funct(){
     while(spinner)
         ;

Quoted 3 lines

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

You don't know how C works, do you?

You also have no idea of what out-of-order memory accesses do to OS code, right?

I think you are mixing apples and oranges. Using volatile to control o-o-o memory accesses is sure wrong.

int spinner=0;

funct(){
while(spinner)

Quoted 11 lines · Linus Torvalds

Quoted 2 lines · Linus Torvalds

Any other use of "volatile" is almost certainly a bug, or just useless.

Side note: it's also totally possible that a volatiles _hides_ a bug, ie removing the volatile ends up having bad effects, but that's because the software itself isn't actually following the rules (or, more commonly, the rules are broken, and somebody added "volatile" to hide the problem).

Quoted 6 lines

That's not just a theoretical notion, btw. We had _tons_ of these kinds of "volatile"s in the original old networking code. They were _all_ wrong. Every single one.

Linus

Linus, you may have been reading too many novels.

If I have some code that does:

extern int spinner;

funct(){
    while(spinner)
        ;

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

However, if I have the same code, but the variable is visible during compile time, i.e.,

int spinner=0;

funct(){
    while(spinner)
        ;

... the compiler may eliminate that code altogether because it 'knows' that spinner is FALSE, having initialized it to zero itself.

Since spinner is global in scope, somebody surely could have changed it before funct() was even called, but the current gcc 'C' compiler doesn't care and may optimize it away entirely. To prevent this, you must declare the variable volatile. To do otherwise is a bug.

Reading between the lines of your text, you are trying to say that object 'spinner' should remain an integer, but any access should be cast, like:

funct() {
    while((volatile)spinner)
        ;

This is just a matter of style. It substitutes a number of casts for a simple declaration. That said, I think that the current implementation of 'volatile' is broken because the compiler seems to believe that the variable has moved! It recalculates the address of the variable as well as accessing its value. This is what makes the code generation problematical.

extern int spinner;

funct(){
     while(spinner)
         ;

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

However, if I have the same code, but the variable is visible during compile time, i.e.,

int spinner=0;

funct(){
     while(spinner)
         ;

... the compiler may eliminate that code altogether because it 'knows' that spinner is FALSE, having initialized it to zero itself.

Since spinner is global in scope, somebody surely could have changed it before funct() was even called, but the current gcc 'C' compiler doesn't care and may optimize it away entirely.

To
prevent this, you must declare the variable volatile. To do otherwise is a bug.

That said, I think that the current
implementation of 'volatile' is broken because the compiler seems to believe that the variable has moved! It recalculates the address of the variable as well as accessing its value. This is what makes the code generation problematical.

Quoted 9 lines · linux-os \(Dick Johnson\)

extern int spinner;

funct(){
     while(spinner)
         ;

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

defiant:/tmp/khc$ gcc --version
gcc (GCC) 4.1.1 20060525 (Red Hat 4.1.1-1)
defiant:/tmp/khc$ cat test.c
extern int spinner;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

test.o: file format elf32-i386

Disassembly of section .text:

00000000 <funct>:

  0:   a1 00 00 00 00          mov    0x0,%eax
  5:   55                      push   %ebp
  6:   89 e5                   mov    %esp,%ebp
  8:   85 c0                   test   %eax,%eax
  a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
 10:   75 fe                   jne    10 <funct+0x10>
 12:   5d                      pop    %ebp
 13:   c3                      ret

"0x0" is, of course, for relocation.

Quoted 6 lines

However, if I have the same code, but the variable is visible during compile time, i.e.,

int spinner=0;

Quoted 7 lines

funct(){
     while(spinner)
         ;

... the compiler may eliminate that code altogether because it 'knows' that spinner is FALSE, having initialized it to zero itself.

defiant:/tmp/khc$ cat test.c
int spinner = 0;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

00000000 <funct>:

  0:   a1 00 00 00 00          mov    0x0,%eax
  5:   55                      push   %ebp
  6:   89 e5                   mov    %esp,%ebp
  8:   85 c0                   test   %eax,%eax
  a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
 10:   75 fe                   jne    10 <funct+0x10>
 12:   5d                      pop    %ebp
 13:   c3                      ret

Quoted 3 lines

Since spinner is global in scope, somebody surely could have changed it before funct() was even called, but the current gcc 'C' compiler doesn't care and may optimize it away entirely.

Personally I don't think such C compiler even existed. HISOFT C on ZX Spectrum could be a good candidate but I think it didn't have any optimizer :-)

Quoted 3 lines

To
prevent this, you must declare the variable volatile. To do otherwise is a bug.

Nope. Volatile just means that every read (and write) must actually access the variable. Note that the compiler optimized out accesses to the variable in the loop - while it has to check at the beginning of funct(), it knows that the variable is constant through funct().

Note that "volatile" is not exactly what we usually want, but it does the job (i.e., the program doesn't crash, but the code is probably suboptimal).

Quoted 5 lines

That said, I think that the current
implementation of 'volatile' is broken because the compiler seems to believe that the variable has moved! It recalculates the address of the variable as well as accessing its value. This is what makes the code generation problematical.

You must be using a heavily broken compiler:

defiant:/tmp/khc$ cat test.c
volatile int spinner = 0;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

00000000 <funct>:

  0:   55                      push   %ebp
  1:   89 e5                   mov    %esp,%ebp
  3:   a1 00 00 00 00          mov    0x0,%eax
  8:   85 c0                   test   %eax,%eax
  a:   75 f7                   jne    3 <funct+0x3>
  c:   5d                      pop    %ebp
  d:   c3                      ret

--
Krzysztof Halasa

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Reference:
/usr/src/linux-2.6.16.4/include/linux/compiler-gcc.h:
#define barrier() __asm__ __volatile__("": : :"memory")

Quoted 5 lines · linux-os (Dick Johnson)

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Actually, it's not just me.

Read things like the Intel CPU documentation.

IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU potentially over-heat, causing degreaded performance, and you're simply not supposed to do it.

Quoted 4 lines

Reference:
/usr/src/linux-2.6.16.4/include/linux/compiler-gcc.h:
#define barrier() __asm__ __volatile__("": : :"memory")

Actually, for your kind of stupid loop, you should use

- include/asm-i386/processor.h:

	#define cpu_relax()        rep_nop()

where rep_nop() is

	/* REP NOP (PAUSE) is a a good thing to insert into busy-wait loops. */
	static inline void rep_nop(void)
	{
		__asm__ __volatile__("rep;nop": : :"memory");
	}

on x86, and can be other things on other CPU's. On ppc64 it is

	#define cpu_relax()     do { HMT_low(); HMT_medium(); barrier(); } while (0)

where those HMT macros adjust thread priority.

In other words, you just don't know what you're talking about. "volatile" is simply not useful, and the fact that you cannot seem to grasp that is _your_ problem.

Repeat after me (or just shut up about things that you not only don't know about, but are apparently not willing to even learn):

"'volatile' is useless. The things it did 30 years ago are much
more complex these days, and need to be tied to much more detailed rules that depend on the actual particular problem, rather than one keyword to the compiler that doesn't actually give enough information for the compiler to do anything useful"

Comprende?

Linus

Again, I didn't propose to do that. In fact, your spin-lock code already inserts "rep nops" and I never implied that they should be removed. I said only that "volatile" still needs to be used, not some macro that tells the compiler that everything in memory probably got trashed. Read what I said, not what you think some idiot might have said.

Locks are supposed to be syncronization points, which is why they ALREADY HAVE "memory" on the clobber list! "memory" IS NECESSARY. The fact that "=m" is changing to "+m" in Linus's patches is because "=m" is in fact insufficient, because it would let the compiler believe we're just going to over-write the value. "volatile" merely hides that bug -- once that bug is _fixed_ (by going to "+m"), "volatile" is no longer useful.

If "volatile" is in use elsewhere (other than locks), it's still probably wrong. In these cases, you can use a barrier, a volatile cast, or an inline asm with a specific clobber.

Quoted 7 lines · Chase Venters

Locks are supposed to be syncronization points, which is why they ALREADY HAVE "memory" on the clobber list! "memory" IS NECESSARY. The fact that "=m" is changing to "+m" in Linus's patches is because "=m" is in fact insufficient, because it would let the compiler believe we're just going to over-write the value. "volatile" merely hides that bug -- once that bug is _fixed_ (by going to "+m"), "volatile" is no longer useful.

This is a completely different story. "volatile", barrier() and "+m"/"=m" aren't sync points. If the variable access isn't atomic you must use locking even with volatiles, barriers etc.

Quoted 3 lines

If "volatile" is in use elsewhere (other than locks), it's still probably wrong. In these cases, you can use a barrier, a volatile cast, or an inline asm with a specific clobber.

A volatile cast is just a volatile, moved from data declaration to all access points. It doesn't buy you anything.
barrier() is basically "all-volatile". All of them operate on the same, compiler level.

If the "volatile" is used the wrong way (which is probably true for most cases), then volatile cast and barrier() will be wrong as well. You need locks or atomic access, meaningful on hardware level.

You're mincing my words. The reason "memory" is on the clobber list is because
a lock is supposed to synchronize all memory accesses up to that point. It's a fence / a barrier, because if the compiler re-orders your loads/stores across the lock, you're in trouble. That's exactly what I was pointing out.

A volatile cast lets you prevent the compiler from always treating the variable as volatile.

Quoted 3 lines

If the "volatile" is used the wrong way (which is probably true for most cases), then volatile cast and barrier() will be wrong as well. You need locks or atomic access, meaningful on hardware level.

No. Linus already described what some example invalid uses of "volatile" are.
One example is the very one this whole thread is about - using 'volatile' on the declaration of the spinlock counter. That usage is _wrong_, and barrier()
would not be.

Volatile originally existed to tell the compiler a variable could change at will. Because of reordering, it's almost never sufficient with our modern compilers and CPUs. That's precisely where barrier() (and/or its hardware equivalents) help in places where 'volatile' is wrong.

Your statement is
additionally wrong because one use-case of memory barriers is to safely write
lock-free code.

Quoted 6 lines · Chase Venters

You're mincing my words. The reason "memory" is on the clobber list is because
a lock is supposed to synchronize all memory accesses up to that point. It's a fence / a barrier, because if the compiler re-orders your loads/stores across the lock, you're in trouble. That's exactly what I was pointing out.

Sure, but a barrier alone isn't enough. You have to use assembler and it's beyond scope of C volatile.

Quoted 2 lines

A volatile cast lets you prevent the compiler from always treating the variable as volatile.

Yes, if that's what you really want.

Quoted 10 lines

Quoted 3 lines

If the "volatile" is used the wrong way (which is probably true for most cases), then volatile cast and barrier() will be wrong as well. You need locks or atomic access, meaningful on hardware level.

No. Linus already described what some example invalid uses of "volatile" are.
One example is the very one this whole thread is about - using 'volatile' on the declaration of the spinlock counter. That usage is _wrong_, and barrier()
would not be.

That's a special case, because you want to invalidate all variables, but you still need locking. I.e., barrier() alone doesn't buy you anything WRT to hardware.

Quoted 4 lines

Volatile originally existed to tell the compiler a variable could change at will. Because of reordering, it's almost never sufficient with our modern compilers and CPUs. That's precisely where barrier() (and/or its hardware equivalents) help in places where 'volatile' is wrong.

How does barrier() help here? Some example, maybe?
What do you consider a barrier() hardware equivalent? Don't you think you're mixing compiler optimization and operation of the hardware?

Quoted 4 lines

Your statement is
additionally wrong because one use-case of memory barriers is to safely write
lock-free code.

You can't safely write lock-free code in C, if you have to deal with hardware or SMP. C don't know about hardware.

You need the barrier for both the CPU and the compiler. The CPU barrier comes from an instruction like '*fence' on x86 (or a locked bus op), while the compiler barrier comes from the memory clobber. Because the spinlocks already _must_ have both of these (including the other constraints in the inline asm), 'volatile' on the spinlock ctr is useless.

Thanks,
Chase

Quoted 2 lines

Sure, but a barrier alone isn't enough. You have to use assembler and it's beyond scope of C volatile.

Right, which is why volatile is wrong.

You need the barrier for both the CPU and the compiler.

This is a bait and switch argument. The code was displayed to show the compiler output, not an example of good coding practice.

Quoted 5 lines

"'volatile' is useless. The things it did 30 years ago are much
more complex these days, and need to be tied to much more detailed rules that depend on the actual particular problem, rather than one keyword to the compiler that doesn't actually give enough information for the compiler to do anything useful"

It still makes a difference for code generation, OF COURSE. But it's the wrong thing to use.

Quoted 2 lines

This is a bait and switch argument. The code was displayed to show the compiler output, not an example of good coding practice.

NO IT IS NOT.

The whole point of my argument is simple:

Quoted 5 lines

Quoted 5 lines

"'volatile' is useless. The things it did 30 years ago are much
more complex these days, and need to be tied to much more detailed rules that depend on the actual particular problem, rather than one keyword to the compiler that doesn't actually give enough information for the compiler to do anything useful"

And dammit, if you cannot admit that, then you're not worth discussing with.

"volatile" is useless. It's a big hammer in a world where the nails aren't nails any more, they are screws, thumb-tacks, and spotwelding.

Actually, because volatile is big hammer, it can be used to work around compiler bugs. If compiler dies at internal error in function foo, just sprinkle few volatiles into it, and you can usually work around that compiler problem.

Quoted 11 lines · Linus Torvalds

Quoted 5 lines · linux-os (Dick Johnson)

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Actually, it's not just me.

Quoted 5 lines

Read things like the Intel CPU documentation.

IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU potentially over-heat, causing degreaded performance, and you're simply not supposed to do it.

This is a bait and switch argument. The code was displayed to show the compiler output, not an example of good coding practice.

#include <stdint.h>

//volatile
uint32_t spinvar = 1;
uint32_t mtx;

void lock(uint32_t* l)

{
   *l = 1;
}

void unlock(uint32_t* l)

{
   *l = 0;
}

void spin()

{
   uint32_t local;

   for (;;)
   {
       lock(&mtx);
       local = spinvar;
       unlock(&mtx);
       if (!local)
           break;
   }
}

without the volatile:

spin:

   pushl   %ebp
   movl    spinvar, %eax
   movl    %esp, %ebp
   testl   %eax, %eax
   je  .L7
.L10:
   jmp .L10

.L7:
movl $0, mtx
popl %ebp
ret

so the compiler did something like

local = spinvar;
if (local)

	for (;;);

(notice the dead lock/unlock inlined code elimination).

With the volatile, the code is correct:

spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:
movl spinvar, %eax
testl %eax, %eax
jne .L7
movl $0, mtx
popl %ebp
ret

So think about all you inlined spinlocks, mutexes and so on.

And if you do

void lock(volatile uint32_t* l)
...
void unlock(volatile uint32_t* l)
...

the code is even better:

spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:

   movl    $1, mtx    <=========
   movl    spinvar, %eax
   movl    $0, mtx    <=========
   testl   %eax, %eax

jne .L7
popl %ebp
ret

So volatile just means 'dont trust this does not change even you don't see why'.

Quoted 17 lines · J.A. Magallón

#include <stdint.h>

//volatile
uint32_t spinvar = 1;
uint32_t mtx;

void lock(uint32_t* l)

{
   *l = 1;
}

void unlock(uint32_t* l)

{
   *l = 0;
}

void spin()

Quoted 12 lines

{
   uint32_t local;

   for (;;)
   {
       lock(&mtx);
       local = spinvar;
       unlock(&mtx);
       if (!local)
           break;
   }
}

This is _totally_ incorrect. Your "lock" functions are broken, because they do not introduce syncronization points or locked bus operations.

Due
to this huge failure, the compiler and/or processor is free to re-order your loads and stores, resulting in totally unpredictable runtime behavior.

Quoted 18 lines

without the volatile:

spin:

   pushl   %ebp
   movl    spinvar, %eax
   movl    %esp, %ebp
   testl   %eax, %eax
   je  .L7
.L10:
   jmp .L10

.L7:
movl $0, mtx
popl %ebp
ret

so the compiler did something like

Quoted 5 lines

local = spinvar;
if (local)

	for (;;);

(notice the dead lock/unlock inlined code elimination).

...which indicates that your code is wrong.

Quoted 14 lines

With the volatile, the code is correct:

spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:
movl spinvar, %eax
testl %eax, %eax
jne .L7
movl $0, mtx
popl %ebp
ret

Actually, it's not. It's never setting "mtx" to 1,

and it's certainly not
doing any sync or locked ops.

Quoted 1 line

So think about all you inlined spinlocks, mutexes and so on.

Yes, you got it wrong, and the current code gets it right. (Linus's patch of =m to +m, combined with -volatile, is best)

Quoted 15 lines

And if you do

void lock(volatile uint32_t* l)
...
void unlock(volatile uint32_t* l)
...

the code is even better:

spin:
pushl %ebp
movl %esp, %ebp
.p2align 4,,7
.L7:

Quoted 7 lines

   movl    $1, mtx    <=========
   movl    spinvar, %eax
   movl    $0, mtx    <=========
   testl   %eax, %eax

jne .L7
popl %ebp
ret

NO! It's not better. You're still not syncing or locking the bus! If you refer to the fact that the "movl $1" has magically appeared, that's because you've just PAPERED OVER THE PROBLEM WITH "volatile", which is _exactly_ what Linus is telling you NOT TO DO.

Quoted 3 lines

So volatile just means 'dont trust this does not change even you don't see why'.

No.

Thanks,
Chase

Quoted 3 lines

This is _totally_ incorrect. Your "lock" functions are broken, because they do not introduce syncronization points or locked bus operations.

But why should I do that ? I write C, a high level language, and I don't mind about buses or whatever. I just have to know that a 32 bit store is atomic in a 32bit arch. There is a nice high level and portable language feature to say 'reload this variable here'. And it lets the compiler do its
optimization work as it best can asuming it has to reload that variable. Instead, you prefer to lock all the bus for that.
Kernel developers spent a full release to get rid of
the 'big kernel lock'. Perhaps there is another release needed to get rid of the 'big memory barrier'.

BTW, I really don't mind if a given architecnture has to lock the bus or say a prayer to Budha to reload a variable. I want it to be reloaded at every (or a certain, in case of a (volatile)mtx cast) usage. The compiler is the responsible of knowing what to do. What if nextgen P4 Xeon do not need a bus lock ? Will you rewrite the kernel ?

Quoted 8 lines

.L7:

   movl    $1, mtx    <=========
   movl    spinvar, %eax
   movl    $0, mtx    <=========
   testl   %eax, %eax

jne .L7
popl %ebp
ret

NO! It's not better. You're still not syncing or locking the bus! If you refer to the fact that the "movl $1" has magically appeared, that's because you've just PAPERED OVER THE PROBLEM WITH "volatile", which is _exactly_ what Linus is telling you NOT TO DO.

Quoted 9 lines · Chase Venters

Quoted 8 lines

.L7:

   movl    $1, mtx    <=========
   movl    spinvar, %eax
   movl    $0, mtx    <=========
   testl   %eax, %eax

jne .L7
popl %ebp
ret

Quoted 5 lines

NO! It's not better. You're still not syncing or locking the bus! If you refer to the fact that the "movl $1" has magically appeared, that's because you've just PAPERED OVER THE PROBLEM WITH "volatile", which is _exactly_ what Linus is telling you NOT TO DO.

BTW, I really don't mind if a given architecnture has to lock the bus or say a prayer to Budha to reload a variable. I want it to be reloaded at every (or a certain, in case of a (volatile)mtx cast) usage. The compiler is the responsible of knowing what to do. What if nextgen P4 Xeon do not need a bus lock ? Will you rewrite the kernel ?

--
J.A. Magallon <jamagallon()ono!com> \ Software is like sex:
\ It's better when it's free
Mandriva Linux release 2007.0 (Cooker) for i586
Linux 2.6.17-jam01 (gcc 4.1.1 20060518 (prerelease)) #2 SMP PREEMPT Wed

On Fri, 7 Jul 2006 17:22:22 -0500 (CDT), Chase Venters

Quoted 9 lines

Quoted 8 lines

.L7:

   movl    $1, mtx    <=========
   movl    spinvar, %eax
   movl    $0, mtx    <=========
   testl   %eax, %eax

jne .L7
popl %ebp
ret

Quoted 4 lines

NO! It's not better. You're still not syncing or locking the bus! If you refer to the fact that the "movl $1" has magically appeared, that's because you've just PAPERED OVER THE PROBLEM WITH "volatile", which is _exactly_ what Linus is telling you NOT TO DO.

BTW, I really don't mind if a given architecnture has to lock the bus or say a prayer to Budha to reload a variable. I want it to be reloaded at every (or a certain, in case of a (volatile)mtx cast) usage. The compiler is the responsible of knowing what to do. What if nextgen P4 Xeon do not need a bus lock ? Will you rewrite the kernel ?

Quoted 19 lines · linux-os \(Dick Johnson\)

Quoted 15 lines · Linus Torvalds

Quoted 5 lines · linux-os (Dick Johnson)

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Actually, it's not just me.

Read things like the Intel CPU documentation.

IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU potentially over-heat, causing degreaded performance, and you're simply not supposed to do it.

Quoted 3 lines

This is a bait and switch argument. The code was displayed to show the compiler output, not an example of good coding practice.

volatile means what it means, is usefull and is right. If it is used in kernel for other things apart from what it was designed for it is kernel or programmer responsibility. It does not mention nothing about locking.

Quoted 7 lines

Quoted 4 lines

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Actually, it's not just me.

Quoted 5 lines

Read things like the Intel CPU documentation.

IT IS ACTIVELY WRONG to busy-loop on a variable. It will make the CPU potentially over-heat, causing degreaded performance, and you're simply not supposed to do it.

This is a bait and switch argument. [...]

Quoted 10 lines

Quoted 10 lines

If I have some code that does:

extern int spinner;

funct(){
     while(spinner)
           ;

The 'C' compiler has no choice but to actually make that memory access and read the variable [...]

[...] In fact, your spin-lock code already inserts "rep nops" and I never implied that they should be removed. I said only that "volatile" still needs to be used, not some macro that tells the compiler that everything in memory probably got trashed. [...]

Well, in fact I do know what I'm talking about. Your bait-and-switch arguments just won't work with me.

Quoted 10 lines

Repeat after me (or just shut up about things that you not only don't know about, but are apparently not willing to even learn):

"'volatile' is useless. The things it did 30 years ago are much
more complex these days, and need to be tied to much more detailed rules that depend on the actual particular problem, rather than one keyword to the compiler that doesn't actually give enough information for the compiler to do anything useful"

Comprende?

Quoted 4 lines

[...] In fact, your spin-lock code already inserts "rep nops" and I never implied that they should be removed. I said only that "volatile" still needs to be used, not some macro that tells the compiler that everything in memory probably got trashed. [...]

your position here does seem to make much sense to me, so please help me

understand it. You suggest that the assembly code should be left alone. But then why do you need the volatile keyword to begin with?

Quoted 2 lines

00000000 <funct>:
0: a1 00 00 00 00 mov 0x0,%eax

Quoted 3 lines

  5:   55                      push   %ebp
  6:   89 e5                   mov    %esp,%ebp
  8:   85 c0                   test   %eax,%eax

Quoted 6 lines

  a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
 10:   75 fe                   jne    10 <funct+0x10>
 12:   5d                      pop    %ebp
 13:   c3                      ret

"0x0" is, of course, for relocation.

So read the code; you have "10: jne 10", jumping to itself
forever, without even doing anything else to set the flags, much less reading a variable.

Quoted 8 lines

00000000 <funct>:

  0:   55                      push   %ebp
  1:   89 e5                   mov    %esp,%ebp
  3:   a1 00 00 00 00          mov    0x0,%eax
  8:   85 c0                   test   %eax,%eax
  a:   75 f7                   jne    3 <funct+0x3>
  c:   5d                      pop    %ebp
  d:   c3                      ret

This is the only code that works. Guess why it worked? Because you declared the variable volatile.

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Quoted 39 lines · Krzysztof Halasa

Quoted 9 lines · linux-os \(Dick Johnson\)

extern int spinner;

funct(){
     while(spinner)
         ;

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module (a.k.a. file).

defiant:/tmp/khc$ gcc --version
gcc (GCC) 4.1.1 20060525 (Red Hat 4.1.1-1)
defiant:/tmp/khc$ cat test.c
extern int spinner;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

test.o: file format elf32-i386

Disassembly of section .text:

00000000 <funct>:

  0:   a1 00 00 00 00          mov    0x0,%eax
  5:   55                      push   %ebp
  6:   89 e5                   mov    %esp,%ebp
  8:   85 c0                   test   %eax,%eax
  a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
 10:   75 fe                   jne    10 <funct+0x10>
 12:   5d                      pop    %ebp
 13:   c3                      ret

Quoted 1 line

"0x0" is, of course, for relocation.

So read the code; you have "10: jne 10", jumping to itself
forever, without even doing anything else to set the flags, much less reading a variable.

Quoted 25 lines

Quoted 5 lines

However, if I have the same code, but the variable is visible during compile time, i.e.,

int spinner=0;

Quoted 7 lines

funct(){
     while(spinner)
         ;

... the compiler may eliminate that code altogether because it 'knows' that spinner is FALSE, having initialized it to zero itself.

defiant:/tmp/khc$ cat test.c
int spinner = 0;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

00000000 <funct>:

Quoted 9 lines

  0:   a1 00 00 00 00          mov    0x0,%eax
  5:   55                      push   %ebp
  6:   89 e5                   mov    %esp,%ebp
  8:   85 c0                   test   %eax,%eax
  a:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
 10:   75 fe                   jne    10 <funct+0x10>
 12:   5d                      pop    %ebp
 13:   c3                      ret

Then, you have exactly the same thing here:

   10:   75 fe                   jne    10 <funct+0x10>

Same bad code.

Quoted 40 lines

Quoted 3 lines

Since spinner is global in scope, somebody surely could have changed it before funct() was even called, but the current gcc 'C' compiler doesn't care and may optimize it away entirely.

Personally I don't think such C compiler even existed. HISOFT C on ZX Spectrum could be a good candidate but I think it didn't have any optimizer :-)

Quoted 3 lines

To
prevent this, you must declare the variable volatile. To do otherwise is a bug.

Nope. Volatile just means that every read (and write) must actually access the variable. Note that the compiler optimized out accesses to the variable in the loop - while it has to check at the beginning of funct(), it knows that the variable is constant through funct().

Note that "volatile" is not exactly what we usually want, but it does the job (i.e., the program doesn't crash, but the code is probably suboptimal).

Quoted 5 lines

That said, I think that the current
implementation of 'volatile' is broken because the compiler seems to believe that the variable has moved! It recalculates the address of the variable as well as accessing its value. This is what makes the code generation problematical.

You must be using a heavily broken compiler:

defiant:/tmp/khc$ cat test.c
volatile int spinner = 0;

void funct(void)

{
    while(spinner)
        ;
}
defiant:/tmp/khc$ gcc -Wall -O2 -c test.c
defiant:/tmp/khc$ objdump -d test.o

Quoted 8 lines

00000000 <funct>:

  0:   55                      push   %ebp
  1:   89 e5                   mov    %esp,%ebp
  3:   a1 00 00 00 00          mov    0x0,%eax
  8:   85 c0                   test   %eax,%eax
  a:   75 f7                   jne    3 <funct+0x3>
  c:   5d                      pop    %ebp
  d:   c3                      ret

This is the only code that works. Guess why it worked? Because you declared the variable volatile.

Now Linus declares that instead of declaring an object volatile so that it is actually accessed every time it is referenced, he wants to use a GNU-ism with assembly that tells the compiler to re-read __every__ variable existing im memory, instead of just one. Go figure!

Reference:
/usr/src/linux-2.6.16.4/include/linux/compiler-gcc.h:
#define barrier() __asm__ __volatile__("": : :"memory")

Quoted 3 lines

Quoted 3 lines

The 'C' compiler has no choice but to actually make that memory access and read the variable because the variable is in another module

(a.k.a.

Quoted 1 line

Quoted 1 line

file).

So read the code; you have "10: jne 10", jumping to itself
forever, without even doing anything else to set the flags, much less reading a variable.

Quoted 6 lines

Quoted 6 lines

However, if I have the same code, but the variable is visible during compile time, i.e.,

int spinner=0;

Quoted 8 lines

Quoted 7 lines

funct(){
     while(spinner)
         ;

... the compiler may eliminate that code altogether because it 'knows' that spinner is FALSE, having initialized it to zero itself.

Then, you have exactly the same thing here:

   10:   75 fe                   jne    10 <funct+0x10>

Same bad code.

Quoted 1 line

Any other use of "volatile" is almost certainly a bug, or just useless.

That's not just a theoretical notion, btw. We had _tons_ of these kinds of "volatile"s in the original old networking code. They were _all_ wrong. Every single one.

$ find linux-2.6.17 -type f -iname '*.[ch]' -print0 | xargs -0 grep volatile | wc -l
13948

Tough job.

Quoted 5 lines

$ find linux-2.6.17 -type f -iname '*.[ch]' -print0 | xargs -0 grep
volatile | wc -l
13948

Tough job.

You need to exclude "asm volatile", which is a completely different thing.

Quoted 2 lines

You need to exclude "asm volatile", which is a completely different thing.

10077.

yeah. I tried this and it indeed slashed 42K off text size (0.2%):

 text            data    bss     dec             filename
 20779489        6073834 3075176 29928499        vmlinux.volatile
 20736884        6073834 3075176 29885894        vmlinux.non-volatile

i booted the resulting allyesconfig bzImage and everything seems to be working fine. Find patch below.

So I _think_ that we should change the "=m" to the much more correct "+m" at the same time (or before - it's really a bug-fix regardless) as removing the "volatile".

Quoted 4 lines · Linus Torvalds

So I _think_ that we should change the "=m" to the much more correct "+m" at the same time (or before - it's really a bug-fix regardless) as removing the "volatile".

Here's a first cut (UNTESTED!) for x86. I didn't check any other architectures, I bet they have similar problems.

I tried the same on MIPS, for lazyness sake at first only in atomic.h. With gcc 3.3 the code size is exactly the same with both "=m" and "+m", so I didn't look into details of the generated code. With gcc 4.1 "+m" results in a size increase of about 1K for the ip27_defconfig kernel. For example:

<unlock_kernel>:

       df830000        ld      v1,0(gp)
       8c620028        lw      v0,40(v1)
       04400014        bltz    v0,a80000000029944c <unlock_kernel+0x5c>
       00000000        nop
       2442ffff        subiu   v0,v0,1
       ac620028        sw      v0,40(v1)	# current->lock_depth
       8c630028        lw      v1,40(v1)	# current->lock_depth
       0461000b        bgez    v1,a80000000029943c <unlock_kernel+0x4c>

The poinless load isn't generated with "=m". The interesting thing is that in all the instances of bloat I looked at it was actually happening not as part of the asm statement itself, so maybe gcc's reload is getting a little confused.

Quoted 4 lines · Linus Torvalds

So I _think_ that we should change the "=m" to the much more correct "+m" at the same time (or before - it's really a bug-fix regardless) as removing the "volatile".

Here's a first cut (UNTESTED!) for x86. I didn't check any other architectures, I bet they have similar problems.

Linus

----

include/asm-i386/atomic.h +0 -0

index 4f061fa..51a1662 100644

This disagrees with the gcc (4.1.0) docs. info --index-search='Extended Asm' gcc

The ordinary output operands must be write-only; GCC will assume that the values in these operands before the instruction are dead and need not be generated. Extended asm supports input-output or read-write operands. Use the constraint character `+' to indicate such an operand and list it with the output operands. You should only use read-write operands when the constraints for the operand (or the operand in which only some of the bits are to be changed) allow a register.

Quoted 11 lines · Keith Owens

This disagrees with the gcc (4.1.0) docs. info --index-search='Extended Asm' gcc

The ordinary output operands must be write-only; GCC will assume that the values in these operands before the instruction are dead and need not be generated. Extended asm supports input-output or read-write operands. Use the constraint character `+' to indicate such an operand and list it with the output operands. You should only use read-write operands when the constraints for the operand (or the operand in which only some of the bits are to be changed) allow a register.

I'm fairly sure the docs are outdated (but may well be correct for older gcc versions - as I already discussed elsewhere, that "+" thing was not historically useful).

We've been using "+m" for some time in the kernel on several architectures.

git aficionados can do

	git grep -1 '"+m"' v2.6.17

to see the pre-existing usage of this (most of them go back a lot further, although some of them are newer - the <asm-i386/bitops.h> ones were added in January.

So if "+m" didn't work, we've been in trouble for at least the last year.

"... Extended asm supports input-output or
read-write operands. Use the constraint character `+' to indicate such an operand and list it with the output operands. You should only use read-write operands when the constraints for the operand (or the operand in which only some of the bits are to be changed) allow a register."

We can certainly write

	...
	:"=m" (*ptr)
	:"m" (*ptr)
	...

instead of the much simpler

	:"+m" (*ptr)

but we've been using that "+m" format for a long time already (and I _think_ we did so at the suggestion of gcc people), and it would be much better if the gcc documentation was just fixed here.

but we've been using that "+m" format for a long time already (and I _think_ we did so at the suggestion of gcc people), and it would be much better if the gcc documentation was just fixed here.

Quoted 4 lines · Linus Torvalds

but we've been using that "+m" format for a long time already (and I _think_ we did so at the suggestion of gcc people), and it would be much better if the gcc documentation was just fixed here.

IIRC early 4.0 had problems with "+m" and did what the documentation says, but that got quickly fixed.
Here is the old thread http://marc.theaimsgroup.com/?t=108384517900001&r=1&w=2

uninline more wait.h inline functions.

allyesconfig vmlinux size delta:

  text            data    bss     dec          filename
  20736884        6073834 3075176 29885894     vmlinux.before
  20721009        6073966 3075176 29870151     vmlinux.after

On my x86_64 typicalconfig
(http://www.zip.com.au/~akpm/linux/patches/stuff/config-x)

everything inlined:

text data bss dec hex filename
4079169 702440 280184 5061793 4d3ca1 vmlinux

uninline init_waitqueue_head:

4076921 702456 280184 5059561 4d33e9 vmlinux

uninline init_waitqueue_head+init_waitqueue_entry

box:/usr/src/25> size vmlinux
text data bss dec hex filename
4077017 702472 280184 5059673 4d3459 vmlinux

uninline init_waitqueue_head+init_waitqueue_entry+init_waitqueue_func_entry

box:/usr/src/25> size vmlinux
text data bss dec hex filename
4077128 702496 280184 5059808 4d34e0 vmlinux

So we only want to uninline init_waitqueue_head().

Quoted 19 lines · Ingo Molnar

Quoted 8 lines

Quoted 8 lines

i also tried a config with the best size settings (disabling FRAME_POINTER, enabling CC_OPTIMIZE_FOR_SIZE), and this gives:

  text            data    bss     dec         filename
  20777768        6076042 3081864 29935674    vmlinux.x32.size.before
  20748140        6076178 3081864 29906182    vmlinux.x32.size.after

Quoted 6 lines

Quoted 2 lines

or a 34.8 bytes win per callsite (29K total).

With gcc-4.1.0 on i686, uninlining those three functions as per the below patch _increases_ the allnoconfig vmlinux's .text from 833456 bytes to 833728.

that's just the effect of CONFIG_REGPARM and CONFIG_CC_OPTIMIZE_FOR_SIZE not being set in an allnoconfig. Once i set them the text size evens out:

Quoted 5 lines

 431348   60666   27276  519290   7ec7a vmlinux.x32.mini.before
 431359   60666   27276  519301   7ec85 vmlinux.x32.mini.after

compiling without CONFIG_REGPARM on i686 (if you care about text size) makes little sense. It penalizes function calls artificially.

OK, but what happened to the 35-bytes-per-callsite saving?

Quoted 2 lines · Andrew Morton

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 6 lines · Ingo Molnar

Quoted 2 lines · Andrew Morton

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

Quoted 3 lines

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

And more importantly it's a function that's called in slowpathes per definition. So saving text sounds like a good idea, how minimal it may be.

Quoted 11 lines · On Wed, 5 Jul 2006 11:37:56 +0100 Christoph Hellwig <[email protected]> wrote:

Quoted 9 lines · Ingo Molnar

Quoted 2 lines · Andrew Morton

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Quoted 4 lines

And more importantly it's a function that's called in slowpathes per definition. So saving text sounds like a good idea, how minimal it may be.

Well yes - as I said, it's a net win. But 31 bytes per callsite seems weird and makes one wonder what's going on.

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

shrinks fs/select.o by eight bytes. (More than I expected). So it does appear to be a space win, but a pretty slim one.

there are 855 calls to these functions in the allyesconfig vmlinux i did, and i measured a combined size reduction of 34791 bytes. That averages to a 40 bytes win per call site. (on i386.)

Quoted 3 lines

And more importantly it's a function that's called in slowpathes per definition. So saving text sounds like a good idea, how minimal it may be.

Well yes - as I said, it's a net win. But 31 bytes per callsite seems weird and makes one wonder what's going on.