[PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Zhaolei]

Impact: refactor code, no functionality changed

Files in include/trace/ should be definition of tracepoints, and header
file for boot trace should put to include/linux/.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 include/{trace/boot.h => linux/boottrace.h} |    0 
 init/main.c                                 |    2 +-
 kernel/trace/trace.h                        |    2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename include/{trace/boot.h => linux/boottrace.h} (100%)

diff --git a/include/trace/boot.h b/include/linux/boottrace.h
similarity index 100%
rename from include/trace/boot.h
rename to include/linux/boottrace.h
diff --git a/init/main.c b/init/main.c
index 694a563..d7d4741 100644
--- a/init/main.c
+++ b/init/main.c
@@ -66,7 +66,7 @@
 #include <linux/ftrace.h>
 #include <linux/async.h>
 #include <linux/kmemtrace.h>
-#include <trace/boot.h>
+#include <linux/boottrace.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index ce6bdc4..5b654ce 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -8,7 +8,7 @@
 #include <linux/ring_buffer.h>
 #include <linux/mmiotrace.h>
 #include <linux/ftrace.h>
-#include <trace/boot.h>
+#include <linux/boottrace.h>
 #include <linux/kmemtrace.h>
 #include <trace/power.h>
 
-- 
1.5.5.3

[PATCH 2/2] tracing, syscalltrace: Move include/trace/syscall.h to include/linux/syscalltrace.h

[Zhaolei]

Impact: refactor code, no functionality changed

Files in include/trace/ should be definition of tracepoints, and header
file for syscall trace should put to include/linux/.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 arch/x86/kernel/ftrace.c                          |    3 +--
 arch/x86/kernel/ptrace.c                          |    3 +--
 include/linux/syscalls.h                          |    2 +-
 include/{trace/syscall.h => linux/syscalltrace.h} |    0 
 kernel/trace/trace_syscalls.c                     |    2 +-
 5 files changed, 4 insertions(+), 6 deletions(-)
 rename include/{trace/syscall.h => linux/syscalltrace.h} (100%)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 18dfa30..63ab16e 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -17,8 +17,7 @@
 #include <linux/sched.h>
 #include <linux/init.h>
 #include <linux/list.h>
-
-#include <trace/syscall.h>
+#include <linux/syscalltrace.h>
 
 #include <asm/cacheflush.h>
 #include <asm/ftrace.h>
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index d5252ae..25428a9 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -22,6 +22,7 @@
 #include <linux/seccomp.h>
 #include <linux/signal.h>
 #include <linux/workqueue.h>
+#include <linux/syscalltrace.h>
 
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
@@ -35,8 +36,6 @@
 #include <asm/proto.h>
 #include <asm/ds.h>
 
-#include <trace/syscall.h>
-
 #include "tls.h"
 
 enum x86_regset {
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 35bbf67..3c207e9 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -66,7 +66,7 @@ struct perf_counter_hw_event;
 #include <asm/signal.h>
 #include <linux/quota.h>
 #include <linux/key.h>
-#include <trace/syscall.h>
+#include <linux/syscalltrace.h>
 
 #define __SC_DECL1(t1, a1)	t1 a1
 #define __SC_DECL2(t2, a2, ...) t2 a2, __SC_DECL1(__VA_ARGS__)
diff --git a/include/trace/syscall.h b/include/linux/syscalltrace.h
similarity index 100%
rename from include/trace/syscall.h
rename to include/linux/syscalltrace.h
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 5e57964..ab00bc5 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -1,4 +1,4 @@
-#include <trace/syscall.h>
+#include <linux/syscalltrace.h>
 #include <linux/kernel.h>
 #include <asm/syscall.h>
 
-- 
1.5.5.3

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Frederic Weisbecker]

On Mon, Apr 13, 2009 at 11:54:11AM +0800, Zhaolei wrote:
> Impact: refactor code, no functionality changed
> 
> Files in include/trace/ should be definition of tracepoints, and header
> file for boot trace should put to include/linux/.
> 
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---



Until now I had the opinion that it's good to let every tracing headers
to be placed in include/trace/* because they are not useful for anything
else than the tracer itself so that we don't encumber include/linux for
private things.

So that we have both tracepoints/trace_events plus the low-level tracers
headers in include/trace/*

I'm not opposite to this change, but seeing this patch and the recent divide
of kmemtrace headers, I would like to know the opinion of Ingo and Steven
about the strict role of include/trace/*
Is it only for tracepoints-like bits, or oslo intended for every private tracing
purposes?


Thanks.


>  include/{trace/boot.h => linux/boottrace.h} |    0 
>  init/main.c                                 |    2 +-
>  kernel/trace/trace.h                        |    2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename include/{trace/boot.h => linux/boottrace.h} (100%)
> 
> diff --git a/include/trace/boot.h b/include/linux/boottrace.h
> similarity index 100%
> rename from include/trace/boot.h
> rename to include/linux/boottrace.h
> diff --git a/init/main.c b/init/main.c
> index 694a563..d7d4741 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -66,7 +66,7 @@
>  #include <linux/ftrace.h>
>  #include <linux/async.h>
>  #include <linux/kmemtrace.h>
> -#include <trace/boot.h>
> +#include <linux/boottrace.h>
>  
>  #include <asm/io.h>
>  #include <asm/bugs.h>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index ce6bdc4..5b654ce 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -8,7 +8,7 @@
>  #include <linux/ring_buffer.h>
>  #include <linux/mmiotrace.h>
>  #include <linux/ftrace.h>
> -#include <trace/boot.h>
> +#include <linux/boottrace.h>
>  #include <linux/kmemtrace.h>
>  #include <trace/power.h>
>  
> -- 
> 1.5.5.3
> 
> 

Re: [PATCH 2/2] tracing, syscalltrace: Move include/trace/syscall.h to include/linux/syscalltrace.h

[Frederic Weisbecker]

On Mon, Apr 13, 2009 at 11:55:43AM +0800, Zhaolei wrote:
> Impact: refactor code, no functionality changed
> 
> Files in include/trace/ should be definition of tracepoints, and header
> file for syscall trace should put to include/linux/.
> 
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> ---


Same than for boot tracer change.
I'm fine with it if you prefer that the role of include/trace/* gains
more strict limitations, but I would like to know the opinion of tracing
maintainers.

Thanks,
Frederic.


>  arch/x86/kernel/ftrace.c                          |    3 +--
>  arch/x86/kernel/ptrace.c                          |    3 +--
>  include/linux/syscalls.h                          |    2 +-
>  include/{trace/syscall.h => linux/syscalltrace.h} |    0 
>  kernel/trace/trace_syscalls.c                     |    2 +-
>  5 files changed, 4 insertions(+), 6 deletions(-)
>  rename include/{trace/syscall.h => linux/syscalltrace.h} (100%)
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 18dfa30..63ab16e 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -17,8 +17,7 @@
>  #include <linux/sched.h>
>  #include <linux/init.h>
>  #include <linux/list.h>
> -
> -#include <trace/syscall.h>
> +#include <linux/syscalltrace.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/ftrace.h>
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index d5252ae..25428a9 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -22,6 +22,7 @@
>  #include <linux/seccomp.h>
>  #include <linux/signal.h>
>  #include <linux/workqueue.h>
> +#include <linux/syscalltrace.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/pgtable.h>
> @@ -35,8 +36,6 @@
>  #include <asm/proto.h>
>  #include <asm/ds.h>
>  
> -#include <trace/syscall.h>
> -
>  #include "tls.h"
>  
>  enum x86_regset {
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 35bbf67..3c207e9 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -66,7 +66,7 @@ struct perf_counter_hw_event;
>  #include <asm/signal.h>
>  #include <linux/quota.h>
>  #include <linux/key.h>
> -#include <trace/syscall.h>
> +#include <linux/syscalltrace.h>
>  
>  #define __SC_DECL1(t1, a1)	t1 a1
>  #define __SC_DECL2(t2, a2, ...) t2 a2, __SC_DECL1(__VA_ARGS__)
> diff --git a/include/trace/syscall.h b/include/linux/syscalltrace.h
> similarity index 100%
> rename from include/trace/syscall.h
> rename to include/linux/syscalltrace.h
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 5e57964..ab00bc5 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -1,4 +1,4 @@
> -#include <trace/syscall.h>
> +#include <linux/syscalltrace.h>
>  #include <linux/kernel.h>
>  #include <asm/syscall.h>
>  
> -- 
> 1.5.5.3
> 
> 

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Mon, Apr 13, 2009 at 11:54:11AM +0800, Zhaolei wrote:
> > Impact: refactor code, no functionality changed
> > 
> > Files in include/trace/ should be definition of tracepoints, and header
> > file for boot trace should put to include/linux/.
> > 
> > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > ---
> 
> Until now I had the opinion that it's good to let every tracing 
> headers to be placed in include/trace/* because they are not 
> useful for anything else than the tracer itself so that we don't 
> encumber include/linux for private things.
> 
> So that we have both tracepoints/trace_events plus the low-level 
> tracers headers in include/trace/*
> 
> I'm not opposite to this change, but seeing this patch and the 
> recent divide of kmemtrace headers, I would like to know the 
> opinion of Ingo and Steven about the strict role of 
> include/trace/* Is it only for tracepoints-like bits, or oslo 
> intended for every private tracing purposes?

The header split itself is probably good to do - to keep the 'pure' 
portions of tracepoint definitions cleanly separated from more 
functional details like kmem tracer initialization.

The move to include/linux/ is indeed more debatable. I think if a 
header says 'footrace.h' in its name, it could easily be in 
include/trace/foo.h instead? Makes for a tidier structure - 
include/linux/ is massively over-crowded already.

Steve, what do you think?

	Ingo

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Steven Rostedt]

On Tue, 14 Apr 2009, Ingo Molnar wrote:

> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > On Mon, Apr 13, 2009 at 11:54:11AM +0800, Zhaolei wrote:
> > > Impact: refactor code, no functionality changed
> > > 
> > > Files in include/trace/ should be definition of tracepoints, and header
> > > file for boot trace should put to include/linux/.
> > > 
> > > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > ---
> > 
> > Until now I had the opinion that it's good to let every tracing 
> > headers to be placed in include/trace/* because they are not 
> > useful for anything else than the tracer itself so that we don't 
> > encumber include/linux for private things.
> > 
> > So that we have both tracepoints/trace_events plus the low-level 
> > tracers headers in include/trace/*
> > 
> > I'm not opposite to this change, but seeing this patch and the 
> > recent divide of kmemtrace headers, I would like to know the 
> > opinion of Ingo and Steven about the strict role of 
> > include/trace/* Is it only for tracepoints-like bits, or oslo 
> > intended for every private tracing purposes?
> 
> The header split itself is probably good to do - to keep the 'pure' 
> portions of tracepoint definitions cleanly separated from more 
> functional details like kmem tracer initialization.
> 
> The move to include/linux/ is indeed more debatable. I think if a 
> header says 'footrace.h' in its name, it could easily be in 
> include/trace/foo.h instead? Makes for a tidier structure - 
> include/linux/ is massively over-crowded already.
> 
> Steve, what do you think?

We actually discussed this a little at the Linux Collaboration Summit.
The idea was to keep only the tracepoints aka TRACE_EVENT code in 
include/trace/ and perhaps special headers that work with the TRACE_EVENT 
macros. But the infrastructure of the tracers would stay in include/linux.

The rational is that we have a separate directory reserved only for trace 
points / trace events. Adding more headers into that directory would make 
it a bit harder to see right away what trace events where defined for a 
particular kernel source.

I'm getting ready to post my code to allow trace events for modules, and 
it cleans up this directory a little.

-- Steve

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Tue, 14 Apr 2009, Ingo Molnar wrote:
> 
> > 
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > 
> > > On Mon, Apr 13, 2009 at 11:54:11AM +0800, Zhaolei wrote:
> > > > Impact: refactor code, no functionality changed
> > > > 
> > > > Files in include/trace/ should be definition of tracepoints, and header
> > > > file for boot trace should put to include/linux/.
> > > > 
> > > > Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > ---
> > > 
> > > Until now I had the opinion that it's good to let every tracing 
> > > headers to be placed in include/trace/* because they are not 
> > > useful for anything else than the tracer itself so that we don't 
> > > encumber include/linux for private things.
> > > 
> > > So that we have both tracepoints/trace_events plus the low-level 
> > > tracers headers in include/trace/*
> > > 
> > > I'm not opposite to this change, but seeing this patch and the 
> > > recent divide of kmemtrace headers, I would like to know the 
> > > opinion of Ingo and Steven about the strict role of 
> > > include/trace/* Is it only for tracepoints-like bits, or oslo 
> > > intended for every private tracing purposes?
> > 
> > The header split itself is probably good to do - to keep the 'pure' 
> > portions of tracepoint definitions cleanly separated from more 
> > functional details like kmem tracer initialization.
> > 
> > The move to include/linux/ is indeed more debatable. I think if a 
> > header says 'footrace.h' in its name, it could easily be in 
> > include/trace/foo.h instead? Makes for a tidier structure - 
> > include/linux/ is massively over-crowded already.
> > 
> > Steve, what do you think?
> 
> We actually discussed this a little at the Linux Collaboration 
> Summit. The idea was to keep only the tracepoints aka TRACE_EVENT 
> code in include/trace/ and perhaps special headers that work with 
> the TRACE_EVENT macros. But the infrastructure of the tracers 
> would stay in include/linux.
> 
> The rational is that we have a separate directory reserved only 
> for trace points / trace events. Adding more headers into that 
> directory would make it a bit harder to see right away what trace 
> events where defined for a particular kernel source.

Hm, i have to say that is true committee design ;-)

The sane thing would be to put event headers into 
include/trace/events/ and put more generic/utility headers into 
include/trace/.

Reserving a full subdirectory for one singular purpose is a needless 
waste of a nice (and unique) name-space resource.

	Ingo

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Steven Rostedt]

On Tue, 14 Apr 2009, Ingo Molnar wrote:
> 
> The sane thing would be to put event headers into 
> include/trace/events/ and put more generic/utility headers into 
> include/trace/.
> 
> Reserving a full subdirectory for one singular purpose is a needless 
> waste of a nice (and unique) name-space resource.

That's fine with me too. I just want the trace points to be easily seen. 
But by making a sub directory, wont we need to have all users of 
tracepoints do something like:

#include <trace/events/sched.h>


That might be fine too.

-- Steve

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 14 Apr 2009, Ingo Molnar wrote:
> > 
> > The sane thing would be to put event headers into 
> > include/trace/events/ and put more generic/utility headers into 
> > include/trace/.
> > 
> > Reserving a full subdirectory for one singular purpose is a needless 
> > waste of a nice (and unique) name-space resource.
> 
> That's fine with me too. I just want the trace points to be easily 
> seen. But by making a sub directory, wont we need to have all 
> users of tracepoints do something like:
> 
> #include <trace/events/sched.h>
> 
> That might be fine too.

Yes - it would allow the dropping of the annoyingly repetitive 
_event string from those definition files as well?

	Ingo

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Steven Rostedt]

On Tue, 14 Apr 2009, Ingo Molnar wrote:

> 
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Tue, 14 Apr 2009, Ingo Molnar wrote:
> > > 
> > > The sane thing would be to put event headers into 
> > > include/trace/events/ and put more generic/utility headers into 
> > > include/trace/.
> > > 
> > > Reserving a full subdirectory for one singular purpose is a needless 
> > > waste of a nice (and unique) name-space resource.
> > 
> > That's fine with me too. I just want the trace points to be easily 
> > seen. But by making a sub directory, wont we need to have all 
> > users of tracepoints do something like:
> > 
> > #include <trace/events/sched.h>
> > 
> > That might be fine too.
> 
> Yes - it would allow the dropping of the annoyingly repetitive 
> _event string from those definition files as well?

The coming patches for modules already does that ;-)

-- Steve

Re: [PATCH 1/2] tracing, boottrace: Move include/trace/boot.h to include/linux/boottrace.h

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> > > #include <trace/events/sched.h>
> > > 
> > > That might be fine too.
> > 
> > Yes - it would allow the dropping of the annoyingly repetitive 
> > _event string from those definition files as well?
> 
> The coming patches for modules already does that ;-)

Cool. Cannot wait! :-)

	Ingo