Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

[Peter Zijlstra]

On Sun, 2011-01-09 at 21:09 +0000, tip-bot for Steven Rostedt wrote:
> Commit-ID:  3a9f987b3141f086de27832514aad9f50a53f754
> Gitweb:     http://git.kernel.org/tip/3a9f987b3141f086de27832514aad9f50a53f754
> Author:     Steven Rostedt <srostedt@redhat.com>
> AuthorDate: Fri, 7 Jan 2011 15:40:10 -0500
> Committer:  Steven Rostedt <rostedt@goodmis.org>
> CommitDate: Fri, 7 Jan 2011 15:44:56 -0500
> 
> tracing: Include module.h in define_trace.h
> 
> While doing some developing, Peter Zijlstra and I have found
> that if a CREATE_TRACE_POINTS include is done before module.h
> is included, it can break the build.
> 
> We have been lucky so far that this has not broke the build
> since module.h is included in almost everything.
> 
> Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/trace/define_trace.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index b0b4eb2..da39b22 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -21,6 +21,16 @@
>  #undef CREATE_TRACE_POINTS
>  
>  #include <linux/stringify.h>
> +/*
> + * module.h includes tracepoints, and because ftrace.h
> + * pulls in module.h:
> + *  trace/ftrace.h -> linux/ftrace_event.h -> linux/perf_event.h ->
> + *  linux/ftrace.h -> linux/module.h
> + * we must include module.h here before we play with any of
> + * the TRACE_EVENT() macros, otherwise the tracepoints included
> + * by module.h may break the build.
> + */
> +#include <linux/module.h>
>  
>  #undef TRACE_EVENT
>  #define TRACE_EVENT(name, proto, args, tstruct, assign, print)	\

After I got everything building on that tree I was working on, the above
was found to compile but not to link, since it makes every:

 #define CREATE_TRACE_POINTS
 #include <trace/event/foo.h>

site also create the trace/event/module.h tracepoints, thus confusing
the linker with tons of duplicate symbols.

The below patch makes things compile and link:

---
 include/trace/define_trace.h |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index da39b22..9ebd0d3 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -21,16 +21,6 @@
 #undef CREATE_TRACE_POINTS
 
 #include <linux/stringify.h>
-/*
- * module.h includes tracepoints, and because ftrace.h
- * pulls in module.h:
- *  trace/ftrace.h -> linux/ftrace_event.h -> linux/perf_event.h ->
- *  linux/ftrace.h -> linux/module.h
- * we must include module.h here before we play with any of
- * the TRACE_EVENT() macros, otherwise the tracepoints included
- * by module.h may break the build.
- */
-#include <linux/module.h>
 
 #undef TRACE_EVENT
 #define TRACE_EVENT(name, proto, args, tstruct, assign, print)	\
@@ -120,4 +110,17 @@
 /* We may be processing more files */
 #define CREATE_TRACE_POINTS
 
+#else /* CREATE_TRACE_POINTS */
+
+/*
+ * module.h includes tracepoints, and because ftrace.h
+ * pulls in module.h:
+ *  trace/ftrace.h -> linux/ftrace_event.h -> linux/perf_event.h ->
+ *  linux/ftrace.h -> linux/module.h
+ * we must include module.h here before we play with any of
+ * the TRACE_EVENT() macros, otherwise the tracepoints included
+ * by module.h may break the build.
+ */
+#include <linux/module.h>
+
 #endif /* CREATE_TRACE_POINTS */

Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

[Steven Rostedt]

On Mon, 2011-01-10 at 14:49 +0100, Peter Zijlstra wrote:
> After I got everything building on that tree I was working on, the above
> was found to compile but not to link, since it makes every:
> 
>  #define CREATE_TRACE_POINTS
>  #include <trace/event/foo.h>
> 
> site also create the trace/event/module.h tracepoints, thus confusing
> the linker with tons of duplicate symbols.
> 
> The below patch makes things compile and link:
> 
> ---
>  include/trace/define_trace.h |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index da39b22..9ebd0d3 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -21,16 +21,6 @@
>  #undef CREATE_TRACE_POINTS

But here we undefine CREATE_TRACE_POINTS, why would this cause a problem
in module.h?

-- Steve

>  
>  #include <linux/stringify.h>
> -/*
> - * module.h includes tracepoints, and because ftrace.h
> - * pulls in module.h:
> - *  trace/ftrace.h -> linux/ftrace_event.h -> linux/perf_event.h ->
> - *  linux/ftrace.h -> linux/module.h
> - * we must include module.h here before we play with any of
> - * the TRACE_EVENT() macros, otherwise the tracepoints included
> - * by module.h may break the build.
> - */
> -#include <linux/module.h>
>  
>  #undef TRACE_EVENT
>  #define TRACE_EVENT(name, proto, args, tstruct, assign, print)	\
> @@ -120,4 +110,17 @@
>  /* We may be processing more files */
>  #define CREATE_TRACE_POINTS
>  
> +#else /* CREATE_TRACE_POINTS */
> +
> +/*
> + * module.h includes tracepoints, and because ftrace.h
> + * pulls in module.h:
> + *  trace/ftrace.h -> linux/ftrace_event.h -> linux/perf_event.h ->
> + *  linux/ftrace.h -> linux/module.h
> + * we must include module.h here before we play with any of
> + * the TRACE_EVENT() macros, otherwise the tracepoints included
> + * by module.h may break the build.
> + */
> +#include <linux/module.h>
> +
>  #endif /* CREATE_TRACE_POINTS */

Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

[Peter Zijlstra]

On Mon, 2011-01-10 at 09:14 -0500, Steven Rostedt wrote:
> > @@ -21,16 +21,6 @@
> >  #undef CREATE_TRACE_POINTS
> 
> But here we undefine CREATE_TRACE_POINTS, why would this cause a problem
> in module.h?

I'm not sure what's happening, all I know is that without this change I
get:

  LD      vmlinux.o
kernel/built-in.o:(__tracepoints+0xac0): multiple definition of `__tracepoint_module_request'
arch/x86/built-in.o:(__tracepoints+0x100): first defined here
kernel/built-in.o:(__tracepoints+0xa40): multiple definition of `__tracepoint_module_get'
arch/x86/built-in.o:(__tracepoints+0x80): first defined here
kernel/built-in.o:(__tracepoints+0x9c0): multiple definition of `__tracepoint_module_load'
arch/x86/built-in.o:(__tracepoints+0x0): first defined here
kernel/built-in.o:(__tracepoints+0xa80): multiple definition of `__tracepoint_module_put'
arch/x86/built-in.o:(__tracepoints+0xc0): first defined here
kernel/built-in.o:(__tracepoints+0xa00): multiple definition of `__tracepoint_module_free'
arch/x86/built-in.o:(__tracepoints+0x40): first defined here

Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

[Steven Rostedt]

On Mon, 2011-01-10 at 15:38 +0100, Peter Zijlstra wrote:
> On Mon, 2011-01-10 at 09:14 -0500, Steven Rostedt wrote:
> > > @@ -21,16 +21,6 @@
> > >  #undef CREATE_TRACE_POINTS
> > 
> > But here we undefine CREATE_TRACE_POINTS, why would this cause a problem
> > in module.h?
> 
> I'm not sure what's happening, all I know is that without this change I
> get:
> 
>   LD      vmlinux.o
> kernel/built-in.o:(__tracepoints+0xac0): multiple definition of `__tracepoint_module_request'
> arch/x86/built-in.o:(__tracepoints+0x100): first defined here
> kernel/built-in.o:(__tracepoints+0xa40): multiple definition of `__tracepoint_module_get'
> arch/x86/built-in.o:(__tracepoints+0x80): first defined here
> kernel/built-in.o:(__tracepoints+0x9c0): multiple definition of `__tracepoint_module_load'
> arch/x86/built-in.o:(__tracepoints+0x0): first defined here
> kernel/built-in.o:(__tracepoints+0xa80): multiple definition of `__tracepoint_module_put'
> arch/x86/built-in.o:(__tracepoints+0xc0): first defined here
> kernel/built-in.o:(__tracepoints+0xa00): multiple definition of `__tracepoint_module_free'
> arch/x86/built-in.o:(__tracepoints+0x40): first defined here

Hmm, OK, I'm able to reproduce it when I add:

#define CREATE_TRACE_POINTS
#include <trace/events/syscalls.h>
#undef CREATE_TRACE_POINTS

as the first thing in ptrace.c. I'll investigate this further.

Thanks!

-- Steve

Re: [tip:perf/urgent] tracing: Include module.h in define_trace.h

[Steven Rostedt]

On Mon, 2011-01-10 at 09:52 -0500, Steven Rostedt wrote:

> >   LD      vmlinux.o
> > kernel/built-in.o:(__tracepoints+0xac0): multiple definition of `__tracepoint_module_request'
> > arch/x86/built-in.o:(__tracepoints+0x100): first defined here
> > kernel/built-in.o:(__tracepoints+0xa40): multiple definition of `__tracepoint_module_get'
> > arch/x86/built-in.o:(__tracepoints+0x80): first defined here
> > kernel/built-in.o:(__tracepoints+0x9c0): multiple definition of `__tracepoint_module_load'
> > arch/x86/built-in.o:(__tracepoints+0x0): first defined here
> > kernel/built-in.o:(__tracepoints+0xa80): multiple definition of `__tracepoint_module_put'
> > arch/x86/built-in.o:(__tracepoints+0xc0): first defined here
> > kernel/built-in.o:(__tracepoints+0xa00): multiple definition of `__tracepoint_module_free'
> > arch/x86/built-in.o:(__tracepoints+0x40): first defined here
> 
> Hmm, OK, I'm able to reproduce it when I add:

Correction, I only reproduced this because I'm an idiot ;-)

> 
> #define CREATE_TRACE_POINTS
> #include <trace/events/syscalls.h>
> #undef CREATE_TRACE_POINTS
> 
> as the first thing in ptrace.c. I'll investigate this further.

I added this, but forgot to remove:

#define CREATE_TRACE_POINTS
#include <trace/events/syscalls.h>

that is at the bottom, causing the duplicate definitions.

Removing the second installation, it complies fine.

Thus, it works fine for me. :-/

-- Steve