Re: [PATCH 2/3] ftrace/x86: Add support for C version of recordmcount

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2010-10-15 at 04:50 +0200, Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: Steven Rostedt <srostedt@redhat.com>
> > 
> > This patch adds the support for the C version of recordmcount and
> > compile times show ~ 12% improvement.
> 
> I reported this recordmcount performance problem 2 years ago. Better 
> later than never i guess.

And I also remember saying after I posted this code that it would have a
compile time performance hit. Heck, it's a perl script running on every
object file. It was obvious what was at issue here. But it's better to
slow down the kernel build than to brick network cards. Also, perl was
much easier to do.

That said, the embarrassing thing is not that I knew (or you reported
it) about this performance problem. I'm actually quite embarrassed that
I had this code sitting in my inbox for over a year. I just kept having
other things that were more important coming up than lowering the
compile time of the kernel. Although, I did work to get streamline
config to offset this performance hit.

Finally, while at the End Users Summit, I decided to take a look at
John's code, and I was quite impressed.

But as you said, better late than never.


> 
> > +ifdef CONFIG_DYNAMIC_FTRACE
> > +	ifdef CONFIG_HAVE_C_MCOUNT_RECORD
> > +		BUILD_C_RECORDMCOUNT := y
> > +		export BUILD_C_RECORDMCOUNT
> 
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -33,6 +33,7 @@ config X86
> >  	select HAVE_KRETPROBES
> >  	select HAVE_OPTPROBES
> >  	select HAVE_FTRACE_MCOUNT_RECORD
> > +	select HAVE_C_MCOUNT_RECORD
> 
> The naming is inconsistent here - it should be HAVE_C_RECORDMCOUNT, like 
> the build variable has, and like the utility is called. If we are going 
> to add this flag to most architectures we should name it consistently.

Sure, want me to rebase it or just write a patch on top of it?

-- Steve