how to handle tracing .h loops

how to handle tracing .h loops

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]

Hi,

I'm working on a patch which needs to add a #include <linux/highmem.h>
to <linux/skbuff.h>. This is causing a bunch of fallout in the trace
headers which I can't figure out the right solution to. Simplifying the
patch to just:

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7b996ed..3f5ee74 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -29,6 +29,7 @@
 #include <linux/rcupdate.h>
 #include <linux/dmaengine.h>
 #include <linux/hrtimer.h>
+#include <linux/highmem.h>
 
 /* Don't change this without changing skb_csum_unnecessary! */
 #define CHECKSUM_NONE 0

reproduces on an i386 defconfig (v3.1-rc2). (actually just including
interrupt.h is sufficient too).

The spew is:

In file included from include/trace/ftrace.h:296:0,
                 from include/trace/define_trace.h:96,
                 from include/trace/events/irq.h:150,
                 from include/linux/interrupt.h:23,
                 from arch/x86/include/asm/highmem.h:23,
                 from include/linux/highmem.h:33,
                 from include/linux/skbuff.h:32,
                 from kernel/audit.h:24,
                 from kernel/signal.c:38:
include/trace/events/irq.h: In function 'ftrace_raw_output_softirq':
include/trace/events/irq.h:87:1: error: 'HI_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: note: each undeclared identifier is reported only once for each function it appears in
include/trace/events/irq.h:87:1: error: 'TIMER_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'NET_TX_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'NET_RX_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'BLOCK_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'BLOCK_IOPOLL_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'TASKLET_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'SCHED_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'HRTIMER_SOFTIRQ' undeclared (first use in this function)
include/trace/events/irq.h:87:1: error: 'RCU_SOFTIRQ' undeclared (first use in this function)

There are more following that but I suspect they are just secondary
fallout so I've omitted them in the interests of brevity. Full log is
attached.

The reason I can't see a solution is that it seems to be that
trace/events/irq.h in included in linux/interrupt.h before the
definition of the enum with all those values in it.

I looked in tip.git#perf/core (and master) but I can't see anything
which relates to this issue.

Clearly other places are including interrupt.h and highmem.h without
issue so what am I doing wrong here?

Thanks,
Ian.

[-- Attachment #2: log --]
[-- Type: application/octet-stream, Size: 8784 bytes --]

Re: how to handle tracing .h loops

[Ian Campbell]

On Thu, 2011-08-18 at 12:51 +0100, Ian Campbell wrote:
> Clearly other places are including interrupt.h and highmem.h without
> issue so what am I doing wrong here?

I guess I should have look at the .c file instead of concentrating on
the .h's because this works:

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7b996ed..3f5ee74 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -29,6 +29,7 @@
 #include <linux/rcupdate.h>
 #include <linux/dmaengine.h>
 #include <linux/hrtimer.h>
+#include <linux/highmem.h>
 
 /* Don't change this without changing skb_csum_unnecessary! */
 #define CHECKSUM_NONE 0
diff --git a/kernel/signal.c b/kernel/signal.c
index 291c970..ff432ec 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -30,6 +30,7 @@
 #include <linux/nsproxy.h>
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>
+#undef CREATE_TRACE_POINTS
 
 #include <asm/param.h>
 #include <asm/uaccess.h>


But that seems odd (noone else does it). Perhaps I should instead move
that include to last in the file? i.e.:

diff --git a/kernel/signal.c b/kernel/signal.c
index 291c970..d3cd4e7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -28,8 +28,6 @@
 #include <linux/freezer.h>
 #include <linux/pid_namespace.h>
 #include <linux/nsproxy.h>
-#define CREATE_TRACE_POINTS
-#include <trace/events/signal.h>
 
 #include <asm/param.h>
 #include <asm/uaccess.h>
@@ -37,6 +35,9 @@
 #include <asm/siginfo.h>
 #include "audit.h"	/* audit_signal_info() */
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/signal.h>
+
 /*
  * SLAB caches for signal bits.
  */

Ian.

Re: how to handle tracing .h loops

[Steven Rostedt]

On Thu, 2011-08-18 at 12:57 +0100, Ian Campbell wrote:
> On Thu, 2011-08-18 at 12:51 +0100, Ian Campbell wrote:
> > Clearly other places are including interrupt.h and highmem.h without
> > issue so what am I doing wrong here?
> 
> I guess I should have look at the .c file instead of concentrating on
> the .h's because this works:
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 7b996ed..3f5ee74 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -29,6 +29,7 @@
>  #include <linux/rcupdate.h>
>  #include <linux/dmaengine.h>
>  #include <linux/hrtimer.h>
> +#include <linux/highmem.h>
>  
>  /* Don't change this without changing skb_csum_unnecessary! */
>  #define CHECKSUM_NONE 0
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 291c970..ff432ec 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -30,6 +30,7 @@
>  #include <linux/nsproxy.h>
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/signal.h>
> +#undef CREATE_TRACE_POINTS

I think the better solution is to move this down below the asms. But as
this does work, I'm don't have any big preference against it.

>  
>  #include <asm/param.h>
>  #include <asm/uaccess.h>
> 
> 
> But that seems odd (noone else does it). Perhaps I should instead move
> that include to last in the file? i.e.:
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 291c970..d3cd4e7 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -28,8 +28,6 @@
>  #include <linux/freezer.h>
>  #include <linux/pid_namespace.h>
>  #include <linux/nsproxy.h>
> -#define CREATE_TRACE_POINTS
> -#include <trace/events/signal.h>
>  
>  #include <asm/param.h>
>  #include <asm/uaccess.h>
> @@ -37,6 +35,9 @@
>  #include <asm/siginfo.h>
>  #include "audit.h"	/* audit_signal_info() */
>  
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/signal.h>
> +

Yes, the trace events headers are suppose to be the last headers
included.

Either method works. Either undefine CREATE_TRACE_POINTS or move the
headers down. Only the C file should be depending on the header anyway.

Thanks!

-- Steve

>  /*
>   * SLAB caches for signal bits.
>   */
> 
> Ian.