Re: [patch] uninline init_waitqueue_*() functions

Linus Torvalds
2006-07-05T22:09:51Z

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.

Linus