[PATCH] irqtrace-option-off-compile-fix

[PATCH] irqtrace-option-off-compile-fix

[Tim Chen]

When CONFIG_TRACE_IRQFLAGS_SUPPORT is turned off, the latest kernel has
compile errors.  The patch below fix the problems.

Regards,
Tim Chen


Signed-off-by: Tim Chen <tim.c.chen@intel.com>
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 412e025..2dd865f 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -41,10 +41,10 @@
 # define INIT_TRACE_IRQFLAGS
 #endif
 
-#ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
-
 #include <asm/irqflags.h>
 
+#ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
+
 #define local_irq_enable() \
 	do { trace_hardirqs_on(); raw_local_irq_enable(); } while (0)
 #define local_irq_disable() \
@@ -62,24 +62,24 @@
 			raw_local_irq_restore(flags);		\
 		}						\
 	} while (0)
+#define safe_halt()						\
+	do {							\
+		trace_hardirqs_on();				\
+		raw_safe_halt();				\
+	} while (0)
+
 #else /* !CONFIG_TRACE_IRQFLAGS_SUPPORT */
 /*
  * The local_irq_*() APIs are equal to the raw_local_irq*()
  * if !TRACE_IRQFLAGS.
  */
-# define raw_local_irq_disable()	local_irq_disable()
-# define raw_local_irq_enable()		local_irq_enable()
-# define raw_local_irq_save(flags)	local_irq_save(flags)
-# define raw_local_irq_restore(flags)	local_irq_restore(flags)
+#define local_irq_disable()		raw_local_irq_disable()
+#define local_irq_enable()		raw_local_irq_enable()
+#define local_irq_save(flags)		raw_local_irq_save(flags)
+#define local_irq_restore(flags)	raw_local_irq_restore(flags)
+#define safe_halt()			raw_safe_halt()
 #endif /* CONFIG_TRACE_IRQFLAGS_SUPPORT */
 
-#ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
-#define safe_halt()						\
-	do {							\
-		trace_hardirqs_on();				\
-		raw_safe_halt();				\
-	} while (0)
-
 #define local_save_flags(flags)		raw_local_save_flags(flags)
 
 #define irqs_disabled()						\
@@ -91,6 +91,5 @@
 })
 
 #define irqs_disabled_flags(flags)	raw_irqs_disabled_flags(flags)
-#endif		/* CONFIG_X86 */
 
 #endif

Re: [PATCH] irqtrace-option-off-compile-fix

[Arjan van de Ven]

On Mon, 2006-07-10 at 17:18 -0700, Tim Chen wrote:
> When CONFIG_TRACE_IRQFLAGS_SUPPORT is turned off, the latest kernel has
> compile errors.  The patch below fix the problems.


Hi,

which architecture did you see this on? (asking because IA64 and PPC
compile just fine without this, and for x86 and x86-64 this is not an
option you can turn off as user, it's not a user selectable config
option but it's a "I have this feature in arch" option)

Greetings,
   Arjan van de Ven

Re: [PATCH] irqtrace-option-off-compile-fix

[Tim Chen]

I was testing on x86_64 and turned off the option in
arch/x86_64/Kconfig.debug. 

When the option is turned off, the following functions become undefined:
local_irq_disable()           
local_irq_enable()             
local_irq_save(flags)          
local_irq_restore(flags)       
safe_halt() 
local_save_flags()
irqs_disabled()
irqs_disabled_flags(flags)                   

It seems plausible that some users may want to avoid the overhead of
tracing IRQFLAGS by turning the option off.

Regards,
Tim Chen

On Tue, 2006-07-11 at 09:13 +0200, Arjan van de Ven wrote:
> On Mon, 2006-07-10 at 17:18 -0700, Tim Chen wrote:
> > When CONFIG_TRACE_IRQFLAGS_SUPPORT is turned off, the latest kernel has
> > compile errors.  The patch below fix the problems.
> 
> 
> Hi,
> 
> which architecture did you see this on? (asking because IA64 and PPC
> compile just fine without this, and for x86 and x86-64 this is not an
> option you can turn off as user, it's not a user selectable config
> option but it's a "I have this feature in arch" option)
> 
> Greetings,
>    Arjan van de Ven

Re: [PATCH] irqtrace-option-off-compile-fix

[Tim Chen]

On Tue, 2006-07-11 at 19:13 +0200, Arjan van de Ven wrote:
> On Tue, 2006-07-11 at 09:23 -0700, Tim Chen wrote:
> > I was testing on x86_64 and turned off the option in
> > arch/x86_64/Kconfig.debug. 
> > 
> > When the option is turned off, the following functions become undefined:
> > local_irq_disable()           
> > local_irq_enable()             
> > local_irq_save(flags)          
> > local_irq_restore(flags)       
> > safe_halt() 
> > local_save_flags()
> > irqs_disabled()
> > irqs_disabled_flags(flags)                   
> > 
> > It seems plausible that some users may want to avoid the overhead of
> > tracing IRQFLAGS by turning the option off.
> 
> eh that is a different config option!

My typo, it is TRACE_IRQFLAGS_SUPPORT in arch/x86_64/Kconfig.debug.

Re: [PATCH] irqtrace-option-off-compile-fix

[Arjan van de Ven]

On Tue, 2006-07-11 at 09:23 -0700, Tim Chen wrote:
> I was testing on x86_64 and turned off the option in
> arch/x86_64/Kconfig.debug. 
> 
> When the option is turned off, the following functions become undefined:
> local_irq_disable()           
> local_irq_enable()             
> local_irq_save(flags)          
> local_irq_restore(flags)       
> safe_halt() 
> local_save_flags()
> irqs_disabled()
> irqs_disabled_flags(flags)                   
> 
> It seems plausible that some users may want to avoid the overhead of
> tracing IRQFLAGS by turning the option off.

eh that is a different config option!

Re: [PATCH] irqtrace-option-off-compile-fix

[Arjan van de Ven]

On Tue, 2006-07-11 at 09:36 -0700, Tim Chen wrote:
> On Tue, 2006-07-11 at 19:13 +0200, Arjan van de Ven wrote:
> > On Tue, 2006-07-11 at 09:23 -0700, Tim Chen wrote:
> > > I was testing on x86_64 and turned off the option in
> > > arch/x86_64/Kconfig.debug. 
> > > 
> > > When the option is turned off, the following functions become undefined:
> > > local_irq_disable()           
> > > local_irq_enable()             
> > > local_irq_save(flags)          
> > > local_irq_restore(flags)       
> > > safe_halt() 
> > > local_save_flags()
> > > irqs_disabled()
> > > irqs_disabled_flags(flags)                   
> > > 
> > > It seems plausible that some users may want to avoid the overhead of
> > > tracing IRQFLAGS by turning the option off.
> > 
> > eh that is a different config option!
> 
> My typo, it is TRACE_IRQFLAGS_SUPPORT in arch/x86_64/Kconfig.debug.


that should never ever be user setable. That just says if you have
support for the api.

The one you want is CONFIG_TRACE_IRQFLAGS .. which is the one that
actually turns the tracing on

Re: [PATCH] irqtrace-option-off-compile-fix

[Tim Chen]

On Tue, 2006-07-11 at 20:05 +0200, Arjan van de Ven wrote: 
> On Tue, 2006-07-11 at 09:36 -0700, Tim Chen wrote:
> > On Tue, 2006-07-11 at 19:13 +0200, Arjan van de Ven wrote:
> > > On Tue, 2006-07-11 at 09:23 -0700, Tim Chen wrote:
> > > > I was testing on x86_64 and turned off the option in
> > > > arch/x86_64/Kconfig.debug. 
> > > > 
> > > > When the option is turned off, the following functions become undefined:
> > > > local_irq_disable()           
> > > > local_irq_enable()             
> > > > local_irq_save(flags)          
> > > > local_irq_restore(flags)       
> > > > safe_halt() 
> > > > local_save_flags()
> > > > irqs_disabled()
> > > > irqs_disabled_flags(flags)                   
> > > > 
> > > > It seems plausible that some users may want to avoid the overhead of
> > > > tracing IRQFLAGS by turning the option off.
> > > 
> > > eh that is a different config option!
> > 
> > My typo, it is TRACE_IRQFLAGS_SUPPORT in arch/x86_64/Kconfig.debug.
> 
> 
> that should never ever be user setable. That just says if you have
> support for the api.
> 
> The one you want is CONFIG_TRACE_IRQFLAGS .. which is the one that
> actually turns the tracing on
> 

I could not turn off CONFIG_TRACE_IRQFLAGS_SUPPORT in .config directly. 
The command "scripts/kconfig/conf -s arch/x86_64/Kconfig" in Makefile
overwrites changes made to CONFIG_TRACE_IRQFLAGS_SUPPORT in .config
file.  So this is always turned on in .config if the option
TRACE_IRQFLAGS_SUPPORT is set in arch/x86_64/Kconfig.debug.  I may be
missing something.  Any suggestions?
  

Re: [PATCH] irqtrace-option-off-compile-fix

[Ingo Molnar]

* Tim Chen <tim.c.chen@linux.intel.com> wrote:

> > The one you want is CONFIG_TRACE_IRQFLAGS .. which is the one that 
> > actually turns the tracing on
> 
> I could not turn off CONFIG_TRACE_IRQFLAGS_SUPPORT in .config 
> directly. The command "scripts/kconfig/conf -s arch/x86_64/Kconfig" in 
> Makefile overwrites changes made to CONFIG_TRACE_IRQFLAGS_SUPPORT in 
> .config file.  So this is always turned on in .config if the option 
> TRACE_IRQFLAGS_SUPPORT is set in arch/x86_64/Kconfig.debug.  I may be 
> missing something.  Any suggestions?

correct, that flag is always set - it signals towards the core kernel 
that the architecture in question (x86_64) that it has trace-irqflags 
support. NOTE: this does not mean that irqflags tracing is turned on - 
that is another option: CONFIG_TRACE_IRQFLAGS.

unsetting the support flag makes no sense and will likely break the 
build. There is no overhead from irqflags tracing if it's turned off. 
(even if the CONFIG_TRACE_IRQFLAGS_SUPPORT option is set)

does this explain things? We could rename the boolean value to 
CONFIG_TRACE_IRQFLAGS_SUPPORT_AVAILABLE perhaps, to avoid future 
confusion.

	Ingo

Re: [PATCH] irqtrace-option-off-compile-fix

[Tim Chen]

On Tue, 2006-07-11 at 21:45 +0200, Ingo Molnar wrote:
> * Tim Chen <tim.c.chen@linux.intel.com> wrote:
> 
> > > The one you want is CONFIG_TRACE_IRQFLAGS .. which is the one that 
> > > actually turns the tracing on
> > 
> > I could not turn off CONFIG_TRACE_IRQFLAGS_SUPPORT in .config 
> > directly. The command "scripts/kconfig/conf -s arch/x86_64/Kconfig" in 
> > Makefile overwrites changes made to CONFIG_TRACE_IRQFLAGS_SUPPORT in 
> > .config file.  So this is always turned on in .config if the option 
> > TRACE_IRQFLAGS_SUPPORT is set in arch/x86_64/Kconfig.debug.  I may be 
> > missing something.  Any suggestions?
> 
> correct, that flag is always set - it signals towards the core kernel 
> that the architecture in question (x86_64) that it has trace-irqflags 
> support. NOTE: this does not mean that irqflags tracing is turned on - 
> that is another option: CONFIG_TRACE_IRQFLAGS.
> 
> unsetting the support flag makes no sense and will likely break the 
> build. There is no overhead from irqflags tracing if it's turned off. 
> (even if the CONFIG_TRACE_IRQFLAGS_SUPPORT option is set)
> 
> does this explain things? We could rename the boolean value to 
> CONFIG_TRACE_IRQFLAGS_SUPPORT_AVAILABLE perhaps, to avoid future 
> confusion.
> 
> 	Ingo

Thanks for clarifying.