Re: [patch] uninline init_waitqueue_*() functions

[Ingo Molnar <mingo@elte.hu>]

* Linus Torvalds <torvalds@osdl.org> wrote:

> On Wed, 5 Jul 2006, Ingo Molnar wrote:
> > 
> > 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)
> 
> 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.

in the debug case it's already a function. (by virtue of lockdep) But 
what happens here is CONFIG_PREEMPT and break_lock and thus we get two 
fields which get initialized in that stupid way. I'll try a better 
initialization sequence. This was with gcc 4.0.1.

	Ingo