Re: [PATCH 2/2] jump label: disable due to compiler bug

From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@elte.hu>, Jason Baron <jbaron@redhat.com>,
	David Daney <ddaney@caviumnetworks.com>,
	rth@redhat.com, "H. Peter Anvin" <hpa@zytor.com>,
	tglx@linutronix.de, andi@firstfloor.org, roland@redhat.com,
	masami.hiramatsu.pt@hitachi.com, fweisbec@gmail.com,
	avi@redhat.com, davem@davemloft.net, vgoyal@redhat.com,
	sam@ravnborg.org, tony@bakeyournoodle.com, dsd@laptop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] jump label: disable due to compiler bug
Date: Fri, 29 Oct 2010 09:35:24 -0400	[thread overview]
Message-ID: <20101029133524.GA23916@Krystal> (raw)
In-Reply-To: <1288356418.18238.454.camel@gandalf.stny.rr.com>

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Fri, 2010-10-29 at 14:22 +0200, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Fri, 2010-10-29 at 08:34 +0200, Ingo Molnar wrote:
> 
> > > Do you mean to have it dependent on !EMBEDDED, or perhaps !CC_OPTIMIZE_FOR_SIZE ?
> > 
> > CC_OPTIMIZE_FOR_SIZE dependency probably not. Might even leave out !EMBEDDED, to 
> > make it generally configurable.
> > 
> > Also, we should make it default off - we need to see whether there are any compiler 
> > bugs lurking.
> 
> Ah, so we should then add a:
> 
> config JUMP_LABEL
> 	prompt "Optimize trace point call sites"
> 	depends on HAVE_ARCH_JUMP_LABEL
> 	help
> 	  If it is detected that the compiler has support for
> 	  "asm goto", the kernel will compile trace points locations 
> 	  with just a nop instruction. When trace points are enabled
> 	  the nop will be converted to a jump to the trace function.
> 	  This technique lowers overhead and stress on the branch
> 	  prediction of the processor.
> 
> 	  On i386, the options added to make this work may increase
> 	  the size of the kernel slightly.
> 
> 
> Then in the code have:
> 
> -#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL)
> +#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_HAVE_ARCH_JUMP_LABEL) \

Technically, you could remove the dependency on
"defined(CONFIG_HAVE_ARCH_JUMP_LABEL)", because it is now dealt with at
the Kconfig level (CONFIG_JUMP_LABEL depends on it).

> +	&& defined(JUMP_LABEL)

this would be defined(CONFIG_JUMP_LABEL)

The rest looks fine,

Thanks,

Mathieu

>  # include <asm/jump_label.h>
>  # define HAVE_JUMP_LABEL
>  #endif
> 
> 
> If you agree, I can write up a patch.
> 
> -- Steve
> 	  
> > 
> > > I can see people making a big stink about adding back a gcc option that makes the 
> > > kernel bigger.
> > 
> > Depends on exactly how much .text we are talking about here. Plus it would be 
> > default-off.
> > 
> > Thanks,
> > 
> > 	Ingo
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com