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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
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
>
>
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 <[email protected]>
> ---
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
>
>
* Frederic Weisbecker <[email protected]> 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 <[email protected]>
> > ---
>
> 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
On Tue, 14 Apr 2009, Ingo Molnar wrote:
>
> * Frederic Weisbecker <[email protected]> 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 <[email protected]>
> > > ---
> >
> > 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
* Steven Rostedt <[email protected]> wrote:
>
> On Tue, 14 Apr 2009, Ingo Molnar wrote:
>
> >
> > * Frederic Weisbecker <[email protected]> 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 <[email protected]>
> > > > ---
> > >
> > > 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
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
* Steven Rostedt <[email protected]> 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
On Tue, 14 Apr 2009, Ingo Molnar wrote:
>
> * Steven Rostedt <[email protected]> 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
* Steven Rostedt <[email protected]> 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