Re: [PATCH] ring_buffer: fix ring_buffer_event_length()

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 8 Jan 2009 12:55:30 +0100 Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 7 Jan 2009 23:58:39 -0500 (EST) Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > >  kernel/trace/ring_buffer.c |    8 +++++++-
> > 
> > <looks>
> > 
> > heavens, what a lot of inlining.  Looks like something from 1997 :)
> > 
> > Prove me wrong!
> > 
> > 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > 
> >           text    data     bss     dec     hex filename
> > before:  11320     228       8   11556    2d24 kernel/trace/ring_buffer.o
> > after:   10592     228       8   10828    2a4c kernel/trace/ring_buffer.o
> 
> You are wrong :-)

Not.

> With x86 defconfig and gcc 4.3.2 i get zero change in size:

With my config and my gcc I see a large change in size.  So those
`inline' statements in that C file are *wrong*.

>   kernel/trace/ring_buffer.o:
> 
>      text	   data	    bss	    dec	    hex	filename
>     11485	    228	      8	  11721	   2dc9	ring_buffer.o.before
>     11485	    228	      8	  11721	   2dc9	ring_buffer.o.after
> 
>   md5:
>      55447563cd459bbb02c6234b2544fcc2  ring_buffer.o.before.asm
>      55447563cd459bbb02c6234b2544fcc2  ring_buffer.o.after.asm
> 
> (i took out the free_page() bit to only measure the inlining)
> 
> That is the same with and without CONFIG_OPTIMIZE_INLINING - i.e. recent 
> GCC gets the inlining right.
> 
> Really, we should stop bothering about inlines on the source code level 
> (the kernel has 20,000 inlines and around 100,000 functions - do we really 
> want to maintain inlining information on a per function basis?) - and we 
> should tell the GCC folks when the compiler messes up some detail.
> 
> Or if GCC messes up inlining so much in the future that we cannot live 
> with it, we can go back to "always inline" and manual annotations again. 
> Or write a new compiler. (the latter is probably less work ;-)

None of that makes the inline statements in ring_buffer.c less wrong. 
It says that with some configs and some gcc versions, their damage is
lessened.