* [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs
@ 2010-05-04 3:40 Steven Rostedt
2010-05-04 3:40 ` [PATCH 1/9 - v2][RFC] tracing: Create class struct for events Steven Rostedt
` (8 more replies)
0 siblings, 9 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[Version 2]
This is an RFC patch set that also affects kprobes and perf.
At the Linux Collaboration Summit, I talked with Mathieu and others about
lowering the footprint of trace events. I spent all of last week
trying to get the size as small as I could.
Currently, each TRACE_EVENT() macro adds 1 - 5K per tracepoint. I got various
results by adding a TRACE_EVENT() with the compiler, depending on
config options that did not seem related. The new tracepoint I added
would add between 1 and 5K, but I did not investigate enough to
see what the true size was.
What was consistent, was the DEFINE_EVENT(). Currently, it adds
a little over 700 bytes per DEFINE_EVENT().
This patch series does not seem to affect TRACE_EVENT() much (had
the same various sizes), but consistently brings DEFINE_EVENT()s
down from 700 bytes to 250 bytes per DEFINE_EVENT(). Since syscalls
use one "class" and are equivalent to DEFINE_EVENT() this can
be a significant savings.
With events and syscalls (82 events and 616 syscalls), before this
patch series, the size of vmlinux was: 16161794, and afterward: 16058182.
That is 103,612 bytes in savings! (over 100K)
Without tracing syscalls (82 events), it brought the size of vmlinux
down from 1591046 to 15999394.
22,071 bytes in savings.
This is just an RFC (for now), to get peoples opinions on the changes.
It does a bit of rewriting of the CPP macros, just to warning you ;-)
Changes in v2:
o Ported to latest tip/tracing/core
o Removed DECLARE_TRACE_DATA() and made DECLARE_TRACE() have
the ability to pass a data parameter. This makes DECLARE_TRACE()
not work with no args. A new DECLARE_TRACE_NOARGS() has been created
that also allows data to be passed, but does is for tracepoint(void).
o Made the callbacks be "proto, void *data" and typecast the data
within the function.
-- Steve
The code can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/tracing/rfc-1
Steven Rostedt (9):
tracing: Create class struct for events
tracing: Let tracepoints have data passed to tracepoint callbacks
tracing: Remove per event trace registering
tracing: Move fields from event to class structure
tracing: Move raw_init from events to class
tracing: Allow events to share their print functions
tracing: Move print functions into event class
tracing: Remove duplicate id information in event structure
tracing: Combine event filter_active and enable into single flags field
----
include/linux/ftrace_event.h | 73 ++++++++---
include/linux/syscalls.h | 57 +++------
include/linux/tracepoint.h | 94 +++++++++++---
include/trace/ftrace.h | 218 ++++++++++----------------------
include/trace/syscall.h | 10 +-
kernel/trace/blktrace.c | 13 ++-
kernel/trace/kmemtrace.c | 28 +++--
kernel/trace/trace.c | 9 +-
kernel/trace/trace.h | 5 +-
kernel/trace/trace_event_perf.c | 17 ++-
kernel/trace/trace_events.c | 131 ++++++++++++-------
kernel/trace/trace_events_filter.c | 28 +++--
kernel/trace/trace_export.c | 16 ++-
kernel/trace/trace_functions_graph.c | 13 ++-
kernel/trace/trace_kprobe.c | 104 +++++++++------
kernel/trace/trace_output.c | 137 ++++++++++++++-------
kernel/trace/trace_output.h | 2 +-
kernel/trace/trace_syscalls.c | 113 +++++++++++++++--
kernel/tracepoint.c | 91 ++++++++------
samples/tracepoints/tp-samples-trace.h | 4 +-
20 files changed, 697 insertions(+), 466 deletions(-)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/9 - v2][RFC] tracing: Create class struct for events
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-07 4:21 ` Frederic Weisbecker
2010-05-04 3:40 ` [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks Steven Rostedt
` (7 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[-- Attachment #1: 0001-tracing-Create-class-struct-for-events.patch --]
[-- Type: text/plain, Size: 12898 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
This patch creates a ftrace_event_class struct that event structs point to.
This class struct will be made to hold information to modify the
events. Currently the class struct only holds the events system name.
This patch slightly increases the size of the text as well as decreases
the data size. The overall change is still a slight increase, but
this change lays the ground work of other changes to make the footprint
of tracepoints smaller.
With 82 standard tracepoints, and 616 system call tracepoints:
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
This patch also cleans up some stale comments in ftrace.h.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 6 ++++-
include/linux/syscalls.h | 6 +++-
include/trace/ftrace.h | 40 +++++++++++++++--------------------
kernel/trace/trace_events.c | 20 +++++++++---------
kernel/trace/trace_events_filter.c | 6 ++--
kernel/trace/trace_export.c | 6 ++++-
kernel/trace/trace_kprobe.c | 12 +++++-----
kernel/trace/trace_syscalls.c | 4 +++
8 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 39e71b0..496eea8 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -113,10 +113,14 @@ void tracing_record_cmdline(struct task_struct *tsk);
struct event_filter;
+struct ftrace_event_class {
+ char *system;
+};
+
struct ftrace_event_call {
struct list_head list;
+ struct ftrace_event_class *class;
char *name;
- char *system;
struct dentry *dir;
struct trace_event *event;
int enabled;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 057929b..ac5791d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -134,6 +134,8 @@ struct perf_event_attr;
#define __SC_STR_TDECL5(t, a, ...) #t, __SC_STR_TDECL4(__VA_ARGS__)
#define __SC_STR_TDECL6(t, a, ...) #t, __SC_STR_TDECL5(__VA_ARGS__)
+extern struct ftrace_event_class event_class_syscalls;
+
#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
@@ -146,7 +148,7 @@ struct perf_event_attr;
__attribute__((section("_ftrace_events"))) \
event_enter_##sname = { \
.name = "sys_enter"#sname, \
- .system = "syscalls", \
+ .class = &event_class_syscalls, \
.event = &enter_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
.define_fields = syscall_enter_define_fields, \
@@ -168,7 +170,7 @@ struct perf_event_attr;
__attribute__((section("_ftrace_events"))) \
event_exit_##sname = { \
.name = "sys_exit"#sname, \
- .system = "syscalls", \
+ .class = &event_class_syscalls, \
.event = &exit_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
.define_fields = syscall_exit_define_fields, \
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 75dd778..d5ec9d1 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -62,7 +62,10 @@
struct trace_entry ent; \
tstruct \
char __data[0]; \
- };
+ }; \
+ \
+ static struct ftrace_event_class event_class_##name;
+
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
static struct ftrace_event_call \
@@ -430,22 +433,6 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
*
* Override the macros in <trace/trace_events.h> to include the following:
*
- * static void ftrace_event_<call>(proto)
- * {
- * event_trace_printk(_RET_IP_, "<call>: " <fmt>);
- * }
- *
- * static int ftrace_reg_event_<call>(struct ftrace_event_call *unused)
- * {
- * return register_trace_<call>(ftrace_event_<call>);
- * }
- *
- * static void ftrace_unreg_event_<call>(struct ftrace_event_call *unused)
- * {
- * unregister_trace_<call>(ftrace_event_<call>);
- * }
- *
- *
* For those macros defined with TRACE_EVENT:
*
* static struct ftrace_event_call event_<call>;
@@ -497,14 +484,18 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
*
* static const char print_fmt_<call>[] = <TP_printk>;
*
+ * static struct ftrace_event_class __used event_class_<template> = {
+ * .system = "<system>",
+ * }
+ *
* static struct ftrace_event_call __used
* __attribute__((__aligned__(4)))
* __attribute__((section("_ftrace_events"))) event_<call> = {
* .name = "<call>",
- * .system = "<system>",
+ * .class = event_class_<template>,
* .raw_init = trace_event_raw_init,
- * .regfunc = ftrace_reg_event_<call>,
- * .unregfunc = ftrace_unreg_event_<call>,
+ * .regfunc = ftrace_raw_reg_event_<call>,
+ * .unregfunc = ftrace_raw_unreg_event_<call>,
* .print_fmt = print_fmt_<call>,
* .define_fields = ftrace_define_fields_<call>,
* }
@@ -627,7 +618,10 @@ static struct trace_event ftrace_event_type_##call = { \
#undef DECLARE_EVENT_CLASS
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
-static const char print_fmt_##call[] = print;
+static const char print_fmt_##call[] = print; \
+static struct ftrace_event_class __used event_class_##call = { \
+ .system = __stringify(TRACE_SYSTEM) \
+}
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, call, proto, args) \
@@ -636,7 +630,7 @@ static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
- .system = __stringify(TRACE_SYSTEM), \
+ .class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
.regfunc = ftrace_raw_reg_event_##call, \
@@ -655,7 +649,7 @@ static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
- .system = __stringify(TRACE_SYSTEM), \
+ .class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
.regfunc = ftrace_raw_reg_event_##call, \
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c697c70..2f54b48 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -176,10 +176,10 @@ static int __ftrace_set_clr_event(const char *match, const char *sub,
if (match &&
strcmp(match, call->name) != 0 &&
- strcmp(match, call->system) != 0)
+ strcmp(match, call->class->system) != 0)
continue;
- if (sub && strcmp(sub, call->system) != 0)
+ if (sub && strcmp(sub, call->class->system) != 0)
continue;
if (event && strcmp(event, call->name) != 0)
@@ -355,8 +355,8 @@ static int t_show(struct seq_file *m, void *v)
{
struct ftrace_event_call *call = v;
- if (strcmp(call->system, TRACE_SYSTEM) != 0)
- seq_printf(m, "%s:", call->system);
+ if (strcmp(call->class->system, TRACE_SYSTEM) != 0)
+ seq_printf(m, "%s:", call->class->system);
seq_printf(m, "%s\n", call->name);
return 0;
@@ -453,7 +453,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
if (!call->name || !call->regfunc)
continue;
- if (system && strcmp(call->system, system) != 0)
+ if (system && strcmp(call->class->system, system) != 0)
continue;
/*
@@ -925,8 +925,8 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
* If the trace point header did not define TRACE_SYSTEM
* then the system would be called "TRACE_SYSTEM".
*/
- if (strcmp(call->system, TRACE_SYSTEM) != 0)
- d_events = event_subsystem_dir(call->system, d_events);
+ if (strcmp(call->class->system, TRACE_SYSTEM) != 0)
+ d_events = event_subsystem_dir(call->class->system, d_events);
call->dir = debugfs_create_dir(call->name, d_events);
if (!call->dir) {
@@ -1041,7 +1041,7 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
list_del(&call->list);
trace_destroy_fields(call);
destroy_preds(call);
- remove_subsystem_dir(call->system);
+ remove_subsystem_dir(call->class->system);
}
/* Remove an event_call */
@@ -1399,8 +1399,8 @@ static __init void event_trace_self_tests(void)
* syscalls as we test.
*/
#ifndef CONFIG_EVENT_TRACE_TEST_SYSCALLS
- if (call->system &&
- strcmp(call->system, "syscalls") == 0)
+ if (call->class->system &&
+ strcmp(call->class->system, "syscalls") == 0)
continue;
#endif
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 88c0b6d..ca32960 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -628,7 +628,7 @@ static int init_subsystem_preds(struct event_subsystem *system)
if (!call->define_fields)
continue;
- if (strcmp(call->system, system->name) != 0)
+ if (strcmp(call->class->system, system->name) != 0)
continue;
err = init_preds(call);
@@ -647,7 +647,7 @@ static void filter_free_subsystem_preds(struct event_subsystem *system)
if (!call->define_fields)
continue;
- if (strcmp(call->system, system->name) != 0)
+ if (strcmp(call->class->system, system->name) != 0)
continue;
filter_disable_preds(call);
@@ -1252,7 +1252,7 @@ static int replace_system_preds(struct event_subsystem *system,
if (!call->define_fields)
continue;
- if (strcmp(call->system, system->name) != 0)
+ if (strcmp(call->class->system, system->name) != 0)
continue;
/* try to see if the filter can be applied */
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index e091f64..7f16e21 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -18,6 +18,10 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM ftrace
+struct ftrace_event_class event_class_ftrace = {
+ .system = __stringify(TRACE_SYSTEM),
+};
+
/* not needed for this file */
#undef __field_struct
#define __field_struct(type, item)
@@ -160,7 +164,7 @@ __attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.id = type, \
- .system = __stringify(TRACE_SYSTEM), \
+ .class = &event_class_ftrace, \
.raw_init = ftrace_raw_init_event, \
.print_fmt = print, \
.define_fields = ftrace_define_fields_##call, \
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 1251e36..eda220b 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -332,8 +332,8 @@ static struct trace_probe *alloc_trace_probe(const char *group,
goto error;
}
- tp->call.system = kstrdup(group, GFP_KERNEL);
- if (!tp->call.system)
+ tp->call.class->system = kstrdup(group, GFP_KERNEL);
+ if (!tp->call.class->system)
goto error;
INIT_LIST_HEAD(&tp->list);
@@ -361,7 +361,7 @@ static void free_trace_probe(struct trace_probe *tp)
for (i = 0; i < tp->nr_args; i++)
free_probe_arg(&tp->args[i]);
- kfree(tp->call.system);
+ kfree(tp->call.class->system);
kfree(tp->call.name);
kfree(tp->symbol);
kfree(tp);
@@ -374,7 +374,7 @@ static struct trace_probe *find_probe_event(const char *event,
list_for_each_entry(tp, &probe_list, list)
if (strcmp(tp->call.name, event) == 0 &&
- strcmp(tp->call.system, group) == 0)
+ strcmp(tp->call.class->system, group) == 0)
return tp;
return NULL;
}
@@ -399,7 +399,7 @@ static int register_trace_probe(struct trace_probe *tp)
mutex_lock(&probe_lock);
/* register as an event */
- old_tp = find_probe_event(tp->call.name, tp->call.system);
+ old_tp = find_probe_event(tp->call.name, tp->call.class->system);
if (old_tp) {
/* delete old event */
unregister_trace_probe(old_tp);
@@ -798,7 +798,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
char buf[MAX_ARGSTR_LEN + 1];
seq_printf(m, "%c", probe_is_return(tp) ? 'r' : 'p');
- seq_printf(m, ":%s/%s", tp->call.system, tp->call.name);
+ seq_printf(m, ":%s/%s", tp->call.class->system, tp->call.name);
if (!tp->symbol)
seq_printf(m, " 0x%p", tp->rp.kp.addr);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 4d6d711..d036a74 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -15,6 +15,10 @@ static int sys_refcount_exit;
static DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls);
static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);
+struct ftrace_event_class event_class_syscalls = {
+ .system = "syscalls"
+};
+
extern unsigned long __start_syscalls_metadata[];
extern unsigned long __stop_syscalls_metadata[];
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
2010-05-04 3:40 ` [PATCH 1/9 - v2][RFC] tracing: Create class struct for events Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-07 3:52 ` Frederic Weisbecker
2010-05-04 3:40 ` [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering Steven Rostedt
` (6 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
[-- Attachment #1: 0002-tracing-Let-tracepoints-have-data-passed-to-tracepoi.patch --]
[-- Type: text/plain, Size: 16979 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
This patch allows data to be passed to the tracepoint callbacks
if the tracepoint was created to do so.
The DECLARE_TRACE() now adds two new functions:
register_trace_mytracepoint_data()
unregister_trace_mytracepoint_data()
These two are the same as the original
register_trace_mytracepoint()
unregister_trace_mytracepoint()
But now allow you to pass a private data pointer that will
be passed to the callback handle. For example:
DECLARE_TRACE(mytracepoint, int value, value);
will create a function called trace_mytracepoint()
void trace_mytracepoint(int value);
If the user wants to pass data to register a function to this tracepoint
and have data also passed to this callback, they can use:
int mycallback(int value, void *data);
register_trace_mytracepoint_data(mycallback, mydata);
Then the mycallback() will receive the "mydata" as the parameter after
the args.
A more detailed example:
DECLARE_TRACE(mytracepoint, TP_PROTO(int status), TP_ARGS(status));
/* In the C file */
DEFINE_TRACE(mytracepoint, TP_PROTO(int status), TP_ARGS(status));
[...]
trace_mytacepoint(status);
/* In a file registering this tracepoint */
int my_callback(int status, void *data)
{
struct my_struct my_data = data;
[...]
}
[...]
my_data = kmalloc(sizeof(*my_data), GFP_KERNEL);
init_my_data(my_data);
register_trace_mytracepoint_data(my_callback, my_data);
The same callback can also be registered to the same tracepoint as long
as the data registered is the different. Note, the data must also be used
to unregister the callback:
unregister_trace_mytracepoint_data(my_callback, my_data);
Because of the data parameter, tracepoints declared this way can not have
no args. That is:
DECLARE_TRACE(mytracepoint, TP_PROTO(void), TP_ARGS());
will cause an error.
If no arguments are needed, a new macro can be used instead:
DECLARE_TRACE_NOARGS(mytracepoint);
Since there are no arguments, the proto and args fields are left out.
This is part of a series to make the tracepoint footprint smaller:
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint
Again, this patch also increases the size of the kernel, but
lays the ground work for decreasing it.
v2: Made the DECLARE_TRACE() have the ability to pass arguments
and added a new DECLARE_TRACE_NOARGS() for tracepoints that
do not need any arguments.
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 94 +++++++++++++++++++++++++-------
kernel/tracepoint.c | 91 +++++++++++++++++--------------
samples/tracepoints/tp-samples-trace.h | 4 +-
3 files changed, 126 insertions(+), 63 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 78b4bd3..ee8059a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -20,12 +20,17 @@
struct module;
struct tracepoint;
+struct tracepoint_func {
+ void *func;
+ void *data;
+};
+
struct tracepoint {
const char *name; /* Tracepoint name */
int state; /* State. */
void (*regfunc)(void);
void (*unregfunc)(void);
- void **funcs;
+ struct tracepoint_func *funcs;
} __attribute__((aligned(32))); /*
* Aligned on 32 bytes because it is
* globally visible and gcc happily
@@ -46,14 +51,18 @@ struct tracepoint {
*/
#define __DO_TRACE(tp, proto, args) \
do { \
- void **it_func; \
+ struct tracepoint_func *it_func_ptr; \
+ void *it_func; \
+ void *__data; \
\
rcu_read_lock_sched_notrace(); \
- it_func = rcu_dereference_sched((tp)->funcs); \
- if (it_func) { \
+ it_func_ptr = rcu_dereference_sched((tp)->funcs); \
+ if (it_func_ptr) { \
do { \
- ((void(*)(proto))(*it_func))(args); \
- } while (*(++it_func)); \
+ it_func = (it_func_ptr)->func; \
+ __data = (it_func_ptr)->data; \
+ ((void(*)(proto))(it_func))(args); \
+ } while ((++it_func_ptr)->func); \
} \
rcu_read_unlock_sched_notrace(); \
} while (0)
@@ -63,23 +72,47 @@ struct tracepoint {
* not add unwanted padding between the beginning of the section and the
* structure. Force alignment to the same alignment as the section start.
*/
-#define DECLARE_TRACE(name, proto, args) \
+#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
extern struct tracepoint __tracepoint_##name; \
static inline void trace_##name(proto) \
{ \
if (unlikely(__tracepoint_##name.state)) \
__DO_TRACE(&__tracepoint_##name, \
- TP_PROTO(proto), TP_ARGS(args)); \
+ TP_PROTO(data_proto), \
+ TP_ARGS(data_args)); \
} \
static inline int register_trace_##name(void (*probe)(proto)) \
{ \
- return tracepoint_probe_register(#name, (void *)probe); \
+ return tracepoint_probe_register(#name, (void *)probe, \
+ NULL); \
+ } \
+ static inline int unregister_trace_##name(void (*probe)(proto)) \
+ { \
+ return tracepoint_probe_unregister(#name, (void *)probe,\
+ NULL); \
} \
- static inline int unregister_trace_##name(void (*probe)(proto)) \
+ static inline int \
+ register_trace_##name##_data(void (*probe)(data_proto), \
+ void *data) \
{ \
- return tracepoint_probe_unregister(#name, (void *)probe);\
+ return tracepoint_probe_register(#name, (void *)probe, \
+ data); \
+ } \
+ static inline int \
+ unregister_trace_##name##_data(void (*probe)(data_proto), \
+ void *data) \
+ { \
+ return tracepoint_probe_unregister(#name, (void *)probe,\
+ data); \
}
+#define DECLARE_TRACE_NOARGS(name) \
+ __DECLARE_TRACE(name, void, , void *__data, __data)
+
+#define DECLARE_TRACE(name, proto, args) \
+ __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
+ PARAMS(proto, void *__data), \
+ PARAMS(args, __data))
#define DEFINE_TRACE_FN(name, reg, unreg) \
static const char __tpstrtab_##name[] \
@@ -100,19 +133,37 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
struct tracepoint *end);
#else /* !CONFIG_TRACEPOINTS */
-#define DECLARE_TRACE(name, proto, args) \
- static inline void _do_trace_##name(struct tracepoint *tp, proto) \
- { } \
+#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
static inline void trace_##name(proto) \
- { } \
+ { \
+ } \
static inline int register_trace_##name(void (*probe)(proto)) \
{ \
return -ENOSYS; \
} \
- static inline int unregister_trace_##name(void (*probe)(proto)) \
+ static inline int unregister_trace_##name(void (*probe)(proto)) \
+ { \
+ return -ENOSYS; \
+ } \
+ static inline int \
+ register_trace_##name##_data(void (*probe)(data_proto), \
+ void *data) \
+ { \
+ return -ENOSYS; \
+ } \
+ static inline int \
+ unregister_trace_##name##_data(void (*probe)(data_proto), \
+ void *data) \
{ \
return -ENOSYS; \
}
+#define DECLARE_TRACE_NOARGS(name) \
+ __DECLARE_TRACE(name, void, , void *__data, __data)
+
+#define DECLARE_TRACE(name, proto, args) \
+ __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
+ PARAMS(proto, void *__data), \
+ PARAMS(args, __data))
#define DEFINE_TRACE_FN(name, reg, unreg)
#define DEFINE_TRACE(name)
@@ -129,16 +180,19 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
* Connect a probe to a tracepoint.
* Internal API, should not be used directly.
*/
-extern int tracepoint_probe_register(const char *name, void *probe);
+extern int tracepoint_probe_register(const char *name, void *probe, void *data);
/*
* Disconnect a probe from a tracepoint.
* Internal API, should not be used directly.
*/
-extern int tracepoint_probe_unregister(const char *name, void *probe);
+extern int
+tracepoint_probe_unregister(const char *name, void *probe, void *data);
-extern int tracepoint_probe_register_noupdate(const char *name, void *probe);
-extern int tracepoint_probe_unregister_noupdate(const char *name, void *probe);
+extern int tracepoint_probe_register_noupdate(const char *name, void *probe,
+ void *data);
+extern int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
+ void *data);
extern void tracepoint_probe_update_all(void);
struct tracepoint_iter {
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index cc89be5..c77f3ec 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -54,7 +54,7 @@ static struct hlist_head tracepoint_table[TRACEPOINT_TABLE_SIZE];
*/
struct tracepoint_entry {
struct hlist_node hlist;
- void **funcs;
+ struct tracepoint_func *funcs;
int refcount; /* Number of times armed. 0 if disarmed. */
char name[0];
};
@@ -64,12 +64,12 @@ struct tp_probes {
struct rcu_head rcu;
struct list_head list;
} u;
- void *probes[0];
+ struct tracepoint_func probes[0];
};
static inline void *allocate_probes(int count)
{
- struct tp_probes *p = kmalloc(count * sizeof(void *)
+ struct tp_probes *p = kmalloc(count * sizeof(struct tracepoint_func)
+ sizeof(struct tp_probes), GFP_KERNEL);
return p == NULL ? NULL : p->probes;
}
@@ -79,7 +79,7 @@ static void rcu_free_old_probes(struct rcu_head *head)
kfree(container_of(head, struct tp_probes, u.rcu));
}
-static inline void release_probes(void *old)
+static inline void release_probes(struct tracepoint_func *old)
{
if (old) {
struct tp_probes *tp_probes = container_of(old,
@@ -95,15 +95,16 @@ static void debug_print_probes(struct tracepoint_entry *entry)
if (!tracepoint_debug || !entry->funcs)
return;
- for (i = 0; entry->funcs[i]; i++)
- printk(KERN_DEBUG "Probe %d : %p\n", i, entry->funcs[i]);
+ for (i = 0; entry->funcs[i].func; i++)
+ printk(KERN_DEBUG "Probe %d : %p\n", i, entry->funcs[i].func);
}
-static void *
-tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
+static struct tracepoint_func *
+tracepoint_entry_add_probe(struct tracepoint_entry *entry,
+ void *probe, void *data)
{
int nr_probes = 0;
- void **old, **new;
+ struct tracepoint_func *old, *new;
WARN_ON(!probe);
@@ -111,8 +112,9 @@ tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
old = entry->funcs;
if (old) {
/* (N -> N+1), (N != 0, 1) probes */
- for (nr_probes = 0; old[nr_probes]; nr_probes++)
- if (old[nr_probes] == probe)
+ for (nr_probes = 0; old[nr_probes].func; nr_probes++)
+ if (old[nr_probes].func == probe &&
+ old[nr_probes].data == data)
return ERR_PTR(-EEXIST);
}
/* + 2 : one for new probe, one for NULL func */
@@ -120,9 +122,10 @@ tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old)
- memcpy(new, old, nr_probes * sizeof(void *));
- new[nr_probes] = probe;
- new[nr_probes + 1] = NULL;
+ memcpy(new, old, nr_probes * sizeof(struct tracepoint_func));
+ new[nr_probes].func = probe;
+ new[nr_probes].data = data;
+ new[nr_probes + 1].func = NULL;
entry->refcount = nr_probes + 1;
entry->funcs = new;
debug_print_probes(entry);
@@ -130,10 +133,11 @@ tracepoint_entry_add_probe(struct tracepoint_entry *entry, void *probe)
}
static void *
-tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe)
+tracepoint_entry_remove_probe(struct tracepoint_entry *entry,
+ void *probe, void *data)
{
int nr_probes = 0, nr_del = 0, i;
- void **old, **new;
+ struct tracepoint_func *old, *new;
old = entry->funcs;
@@ -142,8 +146,10 @@ tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe)
debug_print_probes(entry);
/* (N -> M), (N > 1, M >= 0) probes */
- for (nr_probes = 0; old[nr_probes]; nr_probes++) {
- if ((!probe || old[nr_probes] == probe))
+ for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+ if (!probe ||
+ (old[nr_probes].func == probe &&
+ old[nr_probes].data == data))
nr_del++;
}
@@ -160,10 +166,11 @@ tracepoint_entry_remove_probe(struct tracepoint_entry *entry, void *probe)
new = allocate_probes(nr_probes - nr_del + 1);
if (new == NULL)
return ERR_PTR(-ENOMEM);
- for (i = 0; old[i]; i++)
- if ((probe && old[i] != probe))
+ for (i = 0; old[i].func; i++)
+ if (probe &&
+ (old[i].func != probe || old[i].data != data))
new[j++] = old[i];
- new[nr_probes - nr_del] = NULL;
+ new[nr_probes - nr_del].func = NULL;
entry->refcount = nr_probes - nr_del;
entry->funcs = new;
}
@@ -315,18 +322,19 @@ static void tracepoint_update_probes(void)
module_update_tracepoints();
}
-static void *tracepoint_add_probe(const char *name, void *probe)
+static struct tracepoint_func *
+tracepoint_add_probe(const char *name, void *probe, void *data)
{
struct tracepoint_entry *entry;
- void *old;
+ struct tracepoint_func *old;
entry = get_tracepoint(name);
if (!entry) {
entry = add_tracepoint(name);
if (IS_ERR(entry))
- return entry;
+ return (struct tracepoint_func *)entry;
}
- old = tracepoint_entry_add_probe(entry, probe);
+ old = tracepoint_entry_add_probe(entry, probe, data);
if (IS_ERR(old) && !entry->refcount)
remove_tracepoint(entry);
return old;
@@ -340,12 +348,12 @@ static void *tracepoint_add_probe(const char *name, void *probe)
* Returns 0 if ok, error value on error.
* The probe address must at least be aligned on the architecture pointer size.
*/
-int tracepoint_probe_register(const char *name, void *probe)
+int tracepoint_probe_register(const char *name, void *probe, void *data)
{
- void *old;
+ struct tracepoint_func *old;
mutex_lock(&tracepoints_mutex);
- old = tracepoint_add_probe(name, probe);
+ old = tracepoint_add_probe(name, probe, data);
mutex_unlock(&tracepoints_mutex);
if (IS_ERR(old))
return PTR_ERR(old);
@@ -356,15 +364,16 @@ int tracepoint_probe_register(const char *name, void *probe)
}
EXPORT_SYMBOL_GPL(tracepoint_probe_register);
-static void *tracepoint_remove_probe(const char *name, void *probe)
+static struct tracepoint_func *
+tracepoint_remove_probe(const char *name, void *probe, void *data)
{
struct tracepoint_entry *entry;
- void *old;
+ struct tracepoint_func *old;
entry = get_tracepoint(name);
if (!entry)
return ERR_PTR(-ENOENT);
- old = tracepoint_entry_remove_probe(entry, probe);
+ old = tracepoint_entry_remove_probe(entry, probe, data);
if (IS_ERR(old))
return old;
if (!entry->refcount)
@@ -382,12 +391,12 @@ static void *tracepoint_remove_probe(const char *name, void *probe)
* itself uses stop_machine(), which insures that every preempt disabled section
* have finished.
*/
-int tracepoint_probe_unregister(const char *name, void *probe)
+int tracepoint_probe_unregister(const char *name, void *probe, void *data)
{
- void *old;
+ struct tracepoint_func *old;
mutex_lock(&tracepoints_mutex);
- old = tracepoint_remove_probe(name, probe);
+ old = tracepoint_remove_probe(name, probe, data);
mutex_unlock(&tracepoints_mutex);
if (IS_ERR(old))
return PTR_ERR(old);
@@ -418,12 +427,13 @@ static void tracepoint_add_old_probes(void *old)
*
* caller must call tracepoint_probe_update_all()
*/
-int tracepoint_probe_register_noupdate(const char *name, void *probe)
+int tracepoint_probe_register_noupdate(const char *name, void *probe,
+ void *data)
{
- void *old;
+ struct tracepoint_func *old;
mutex_lock(&tracepoints_mutex);
- old = tracepoint_add_probe(name, probe);
+ old = tracepoint_add_probe(name, probe, data);
if (IS_ERR(old)) {
mutex_unlock(&tracepoints_mutex);
return PTR_ERR(old);
@@ -441,12 +451,13 @@ EXPORT_SYMBOL_GPL(tracepoint_probe_register_noupdate);
*
* caller must call tracepoint_probe_update_all()
*/
-int tracepoint_probe_unregister_noupdate(const char *name, void *probe)
+int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
+ void *data)
{
- void *old;
+ struct tracepoint_func *old;
mutex_lock(&tracepoints_mutex);
- old = tracepoint_remove_probe(name, probe);
+ old = tracepoint_remove_probe(name, probe, data);
if (IS_ERR(old)) {
mutex_unlock(&tracepoints_mutex);
return PTR_ERR(old);
diff --git a/samples/tracepoints/tp-samples-trace.h b/samples/tracepoints/tp-samples-trace.h
index dffdc49..4d46be9 100644
--- a/samples/tracepoints/tp-samples-trace.h
+++ b/samples/tracepoints/tp-samples-trace.h
@@ -7,7 +7,5 @@
DECLARE_TRACE(subsys_event,
TP_PROTO(struct inode *inode, struct file *file),
TP_ARGS(inode, file));
-DECLARE_TRACE(subsys_eventb,
- TP_PROTO(void),
- TP_ARGS());
+DECLARE_TRACE_NOARGS(subsys_eventb);
#endif
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
2010-05-04 3:40 ` [PATCH 1/9 - v2][RFC] tracing: Create class struct for events Steven Rostedt
2010-05-04 3:40 ` [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-07 4:20 ` Frederic Weisbecker
2010-05-07 8:20 ` Li Zefan
2010-05-04 3:40 ` [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure Steven Rostedt
` (5 subsequent siblings)
8 siblings, 2 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[-- Attachment #1: 0003-tracing-Remove-per-event-trace-registering.patch --]
[-- Type: text/plain, Size: 21114 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
This patch removes the register functions of TRACE_EVENT() to enable
and disable tracepoints. The registering of a event is now down
directly in the trace_events.c file. The tracepoint_probe_register()
is now called directly.
The prototypes are no longer type checked, but this should not be
an issue since the tracepoints are created automatically by the
macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
other macros will catch it.
The trace_event_class structure now holds the probes to be called
by the callbacks. This removes needing to have each event have
a separate pointer for the probe.
To handle kprobes and syscalls, since they register probes in a
different manner, a "reg" field is added to the ftrace_event_class
structure. If the "reg" field is assigned, then it will be called for
enabling and disabling of the probe for either ftrace or perf. To let
the reg function know what is happening, a new enum (trace_reg) is
created that has the type of control that is needed.
With this new rework, the 82 kernel events and 616 syscall events
has their footprint dramatically lowered:
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint
5796926 1337748 9351592 16486266 fb8f7a vmlinux.data
5774316 1306580 9351592 16432488 fabd68 vmlinux.regs
The size went from 16477030 to 16432488, that's a total of 44K
in savings. With tracepoints being continuously added, this is
critical that the footprint becomes minimal.
v2: Changed the callback probes to pass void * and typecast the
value within the function.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 19 +++++--
include/linux/syscalls.h | 29 ++---------
include/trace/ftrace.h | 111 ++++++---------------------------------
kernel/trace/trace_event_perf.c | 15 ++++-
kernel/trace/trace_events.c | 26 +++++++---
kernel/trace/trace_kprobe.c | 34 +++++++++---
kernel/trace/trace_syscalls.c | 56 +++++++++++++++++++-
7 files changed, 145 insertions(+), 145 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 496eea8..e665ed3 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -113,8 +113,23 @@ void tracing_record_cmdline(struct task_struct *tsk);
struct event_filter;
+enum trace_reg {
+ TRACE_REG_REGISTER,
+ TRACE_REG_UNREGISTER,
+ TRACE_REG_PERF_REGISTER,
+ TRACE_REG_PERF_UNREGISTER,
+};
+
+struct ftrace_event_call;
+
struct ftrace_event_class {
char *system;
+ void *probe;
+#ifdef CONFIG_PERF_EVENTS
+ void *perf_probe;
+#endif
+ int (*reg)(struct ftrace_event_call *event,
+ enum trace_reg type);
};
struct ftrace_event_call {
@@ -124,8 +139,6 @@ struct ftrace_event_call {
struct dentry *dir;
struct trace_event *event;
int enabled;
- int (*regfunc)(struct ftrace_event_call *);
- void (*unregfunc)(struct ftrace_event_call *);
int id;
const char *print_fmt;
int (*raw_init)(struct ftrace_event_call *);
@@ -137,8 +150,6 @@ struct ftrace_event_call {
void *data;
int perf_refcount;
- int (*perf_event_enable)(struct ftrace_event_call *);
- void (*perf_event_disable)(struct ftrace_event_call *);
};
#define PERF_MAX_TRACE_SIZE 2048
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index ac5791d..e3348c4 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -103,22 +103,6 @@ struct perf_event_attr;
#define __SC_TEST5(t5, a5, ...) __SC_TEST(t5); __SC_TEST4(__VA_ARGS__)
#define __SC_TEST6(t6, a6, ...) __SC_TEST(t6); __SC_TEST5(__VA_ARGS__)
-#ifdef CONFIG_PERF_EVENTS
-
-#define TRACE_SYS_ENTER_PERF_INIT(sname) \
- .perf_event_enable = perf_sysenter_enable, \
- .perf_event_disable = perf_sysenter_disable,
-
-#define TRACE_SYS_EXIT_PERF_INIT(sname) \
- .perf_event_enable = perf_sysexit_enable, \
- .perf_event_disable = perf_sysexit_disable,
-#else
-#define TRACE_SYS_ENTER_PERF(sname)
-#define TRACE_SYS_ENTER_PERF_INIT(sname)
-#define TRACE_SYS_EXIT_PERF(sname)
-#define TRACE_SYS_EXIT_PERF_INIT(sname)
-#endif /* CONFIG_PERF_EVENTS */
-
#ifdef CONFIG_FTRACE_SYSCALLS
#define __SC_STR_ADECL1(t, a) #a
#define __SC_STR_ADECL2(t, a, ...) #a, __SC_STR_ADECL1(__VA_ARGS__)
@@ -134,7 +118,8 @@ struct perf_event_attr;
#define __SC_STR_TDECL5(t, a, ...) #t, __SC_STR_TDECL4(__VA_ARGS__)
#define __SC_STR_TDECL6(t, a, ...) #t, __SC_STR_TDECL5(__VA_ARGS__)
-extern struct ftrace_event_class event_class_syscalls;
+extern struct ftrace_event_class event_class_syscall_enter;
+extern struct ftrace_event_class event_class_syscall_exit;
#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
@@ -148,14 +133,11 @@ extern struct ftrace_event_class event_class_syscalls;
__attribute__((section("_ftrace_events"))) \
event_enter_##sname = { \
.name = "sys_enter"#sname, \
- .class = &event_class_syscalls, \
+ .class = &event_class_syscall_enter, \
.event = &enter_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
.define_fields = syscall_enter_define_fields, \
- .regfunc = reg_event_syscall_enter, \
- .unregfunc = unreg_event_syscall_enter, \
.data = (void *)&__syscall_meta_##sname,\
- TRACE_SYS_ENTER_PERF_INIT(sname) \
}
#define SYSCALL_TRACE_EXIT_EVENT(sname) \
@@ -170,14 +152,11 @@ extern struct ftrace_event_class event_class_syscalls;
__attribute__((section("_ftrace_events"))) \
event_exit_##sname = { \
.name = "sys_exit"#sname, \
- .class = &event_class_syscalls, \
+ .class = &event_class_syscall_exit, \
.event = &exit_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
.define_fields = syscall_exit_define_fields, \
- .regfunc = reg_event_syscall_exit, \
- .unregfunc = unreg_event_syscall_exit, \
.data = (void *)&__syscall_meta_##sname,\
- TRACE_SYS_EXIT_PERF_INIT(sname) \
}
#define SYSCALL_METADATA(sname, nb) \
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index d5ec9d1..bc01395 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -381,53 +381,6 @@ static inline notrace int ftrace_get_offsets_##call( \
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
-#ifdef CONFIG_PERF_EVENTS
-
-/*
- * Generate the functions needed for tracepoint perf_event support.
- *
- * NOTE: The insertion profile callback (ftrace_profile_<call>) is defined later
- *
- * static int ftrace_profile_enable_<call>(void)
- * {
- * return register_trace_<call>(ftrace_profile_<call>);
- * }
- *
- * static void ftrace_profile_disable_<call>(void)
- * {
- * unregister_trace_<call>(ftrace_profile_<call>);
- * }
- *
- */
-
-#undef DECLARE_EVENT_CLASS
-#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)
-
-#undef DEFINE_EVENT
-#define DEFINE_EVENT(template, name, proto, args) \
- \
-static void perf_trace_##name(proto); \
- \
-static notrace int \
-perf_trace_enable_##name(struct ftrace_event_call *unused) \
-{ \
- return register_trace_##name(perf_trace_##name); \
-} \
- \
-static notrace void \
-perf_trace_disable_##name(struct ftrace_event_call *unused) \
-{ \
- unregister_trace_##name(perf_trace_##name); \
-}
-
-#undef DEFINE_EVENT_PRINT
-#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
- DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))
-
-#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
-
-#endif /* CONFIG_PERF_EVENTS */
-
/*
* Stage 4 of the trace events.
*
@@ -437,8 +390,9 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
*
* static struct ftrace_event_call event_<call>;
*
- * static void ftrace_raw_event_<call>(proto)
+ * static void ftrace_raw_event_<call>(proto, void *__data)
* {
+ * struct ftrace_event_call *event_call = __data;
* struct ftrace_data_offsets_<call> __maybe_unused __data_offsets;
* struct ring_buffer_event *event;
* struct ftrace_raw_<call> *entry; <-- defined in stage 1
@@ -468,16 +422,6 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
* event, irq_flags, pc);
* }
*
- * static int ftrace_raw_reg_event_<call>(struct ftrace_event_call *unused)
- * {
- * return register_trace_<call>(ftrace_raw_event_<call>);
- * }
- *
- * static void ftrace_unreg_event_<call>(struct ftrace_event_call *unused)
- * {
- * unregister_trace_<call>(ftrace_raw_event_<call>);
- * }
- *
* static struct trace_event ftrace_event_type_<call> = {
* .trace = ftrace_raw_output_<call>, <-- stage 2
* };
@@ -504,11 +448,15 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
#ifdef CONFIG_PERF_EVENTS
+#define _TRACE_PERF_PROTO(call, proto) \
+ static notrace void \
+ perf_trace_##call(proto, struct ftrace_event_call *event);
+
#define _TRACE_PERF_INIT(call) \
- .perf_event_enable = perf_trace_enable_##call, \
- .perf_event_disable = perf_trace_disable_##call,
+ .perf_probe = perf_trace_##call,
#else
+#define _TRACE_PERF_PROTO(call, proto)
#define _TRACE_PERF_INIT(call)
#endif /* CONFIG_PERF_EVENTS */
@@ -542,9 +490,9 @@ perf_trace_disable_##name(struct ftrace_event_call *unused) \
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
\
static notrace void \
-ftrace_raw_event_id_##call(struct ftrace_event_call *event_call, \
- proto) \
+ftrace_raw_event_##call(proto, void *__data) \
{ \
+ struct ftrace_event_call *event_call = __data; \
struct ftrace_data_offsets_##call __maybe_unused __data_offsets;\
struct ring_buffer_event *event; \
struct ftrace_raw_##call *entry; \
@@ -578,23 +526,6 @@ ftrace_raw_event_id_##call(struct ftrace_event_call *event_call, \
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, call, proto, args) \
\
-static notrace void ftrace_raw_event_##call(proto) \
-{ \
- ftrace_raw_event_id_##template(&event_##call, args); \
-} \
- \
-static notrace int \
-ftrace_raw_reg_event_##call(struct ftrace_event_call *unused) \
-{ \
- return register_trace_##call(ftrace_raw_event_##call); \
-} \
- \
-static notrace void \
-ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused) \
-{ \
- unregister_trace_##call(ftrace_raw_event_##call); \
-} \
- \
static struct trace_event ftrace_event_type_##call = { \
.trace = ftrace_raw_output_##call, \
};
@@ -618,9 +549,12 @@ static struct trace_event ftrace_event_type_##call = { \
#undef DECLARE_EVENT_CLASS
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
+_TRACE_PERF_PROTO(call, PARAMS(proto)); \
static const char print_fmt_##call[] = print; \
static struct ftrace_event_class __used event_class_##call = { \
- .system = __stringify(TRACE_SYSTEM) \
+ .system = __stringify(TRACE_SYSTEM), \
+ .probe = ftrace_raw_event_##call, \
+ _TRACE_PERF_INIT(call) \
}
#undef DEFINE_EVENT
@@ -633,11 +567,8 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
- .regfunc = ftrace_raw_reg_event_##call, \
- .unregfunc = ftrace_raw_unreg_event_##call, \
.print_fmt = print_fmt_##template, \
.define_fields = ftrace_define_fields_##template, \
- _TRACE_PERF_INIT(call) \
}
#undef DEFINE_EVENT_PRINT
@@ -652,11 +583,8 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
- .regfunc = ftrace_raw_reg_event_##call, \
- .unregfunc = ftrace_raw_unreg_event_##call, \
.print_fmt = print_fmt_##call, \
.define_fields = ftrace_define_fields_##template, \
- _TRACE_PERF_INIT(call) \
}
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
@@ -756,8 +684,7 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
#undef DECLARE_EVENT_CLASS
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
static notrace void \
-perf_trace_templ_##call(struct ftrace_event_call *event_call, \
- proto) \
+perf_trace_##call(proto, struct ftrace_event_call *event_call) \
{ \
struct ftrace_data_offsets_##call __maybe_unused __data_offsets;\
struct ftrace_raw_##call *entry; \
@@ -792,13 +719,7 @@ perf_trace_templ_##call(struct ftrace_event_call *event_call, \
}
#undef DEFINE_EVENT
-#define DEFINE_EVENT(template, call, proto, args) \
-static notrace void perf_trace_##call(proto) \
-{ \
- struct ftrace_event_call *event_call = &event_##call; \
- \
- perf_trace_templ_##template(event_call, args); \
-}
+#define DEFINE_EVENT(template, call, proto, args)
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 0565bb4..196fe9d 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -49,7 +49,12 @@ static int perf_trace_event_enable(struct ftrace_event_call *event)
rcu_assign_pointer(perf_trace_buf_nmi, buf);
}
- ret = event->perf_event_enable(event);
+ if (event->class->reg)
+ ret = event->class->reg(event, TRACE_REG_PERF_REGISTER);
+ else
+ ret = tracepoint_probe_register(event->name,
+ event->class->perf_probe,
+ event);
if (!ret) {
total_ref_count++;
return 0;
@@ -75,7 +80,8 @@ int perf_trace_enable(int event_id)
mutex_lock(&event_mutex);
list_for_each_entry(event, &ftrace_events, list) {
- if (event->id == event_id && event->perf_event_enable &&
+ if (event->id == event_id &&
+ event->class && event->class->perf_probe &&
try_module_get(event->mod)) {
ret = perf_trace_event_enable(event);
break;
@@ -93,7 +99,10 @@ static void perf_trace_event_disable(struct ftrace_event_call *event)
if (--event->perf_refcount > 0)
return;
- event->perf_event_disable(event);
+ if (event->class->reg)
+ event->class->reg(event, TRACE_REG_PERF_UNREGISTER);
+ else
+ tracepoint_probe_unregister(event->name, event->class->perf_probe, event);
if (!--total_ref_count) {
buf = perf_trace_buf;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2f54b48..0eb9b28 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -127,13 +127,23 @@ static int ftrace_event_enable_disable(struct ftrace_event_call *call,
if (call->enabled) {
call->enabled = 0;
tracing_stop_cmdline_record();
- call->unregfunc(call);
+ if (call->class->reg)
+ call->class->reg(call, TRACE_REG_UNREGISTER);
+ else
+ tracepoint_probe_unregister(call->name,
+ call->class->probe,
+ call);
}
break;
case 1:
if (!call->enabled) {
tracing_start_cmdline_record();
- ret = call->regfunc(call);
+ if (call->class->reg)
+ ret = call->class->reg(call, TRACE_REG_REGISTER);
+ else
+ ret = tracepoint_probe_register(call->name,
+ call->class->probe,
+ call);
if (ret) {
tracing_stop_cmdline_record();
pr_info("event trace: Could not enable event "
@@ -171,7 +181,8 @@ static int __ftrace_set_clr_event(const char *match, const char *sub,
mutex_lock(&event_mutex);
list_for_each_entry(call, &ftrace_events, list) {
- if (!call->name || !call->regfunc)
+ if (!call->name || !call->class ||
+ (!call->class->probe && !call->class->reg))
continue;
if (match &&
@@ -297,7 +308,7 @@ t_next(struct seq_file *m, void *v, loff_t *pos)
* The ftrace subsystem is for showing formats only.
* They can not be enabled or disabled via the event files.
*/
- if (call->regfunc)
+ if (call->class && (call->class->probe || call->class->reg))
return call;
}
@@ -450,7 +461,8 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
mutex_lock(&event_mutex);
list_for_each_entry(call, &ftrace_events, list) {
- if (!call->name || !call->regfunc)
+ if (!call->name || !call->class ||
+ (!call->class->probe && !call->class->reg))
continue;
if (system && strcmp(call->class->system, system) != 0)
@@ -935,11 +947,11 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
return -1;
}
- if (call->regfunc)
+ if (call->class->probe || call->class->reg)
trace_create_file("enable", 0644, call->dir, call,
enable);
- if (call->id && call->perf_event_enable)
+ if (call->id && (call->class->perf_probe || call->class->reg))
trace_create_file("id", 0444, call->dir, call,
id);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index eda220b..f8af21a 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -202,6 +202,7 @@ struct trace_probe {
unsigned long nhit;
unsigned int flags; /* For TP_FLAG_* */
const char *symbol; /* symbol name */
+ struct ftrace_event_class class;
struct ftrace_event_call call;
struct trace_event event;
unsigned int nr_args;
@@ -323,6 +324,7 @@ static struct trace_probe *alloc_trace_probe(const char *group,
goto error;
}
+ tp->call.class = &tp->class;
tp->call.name = kstrdup(event, GFP_KERNEL);
if (!tp->call.name)
goto error;
@@ -332,8 +334,8 @@ static struct trace_probe *alloc_trace_probe(const char *group,
goto error;
}
- tp->call.class->system = kstrdup(group, GFP_KERNEL);
- if (!tp->call.class->system)
+ tp->class.system = kstrdup(group, GFP_KERNEL);
+ if (!tp->class.system)
goto error;
INIT_LIST_HEAD(&tp->list);
@@ -1302,6 +1304,26 @@ static void probe_perf_disable(struct ftrace_event_call *call)
}
#endif /* CONFIG_PERF_EVENTS */
+static __kprobes
+int kprobe_register(struct ftrace_event_call *event, enum trace_reg type)
+{
+ switch (type) {
+ case TRACE_REG_REGISTER:
+ return probe_event_enable(event);
+ case TRACE_REG_UNREGISTER:
+ probe_event_disable(event);
+ return 0;
+
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_REG_PERF_REGISTER:
+ return probe_perf_enable(event);
+ case TRACE_REG_PERF_UNREGISTER:
+ probe_perf_disable(event);
+ return 0;
+#endif
+ }
+ return 0;
+}
static __kprobes
int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
@@ -1355,13 +1377,7 @@ static int register_probe_event(struct trace_probe *tp)
return -ENODEV;
}
call->enabled = 0;
- call->regfunc = probe_event_enable;
- call->unregfunc = probe_event_disable;
-
-#ifdef CONFIG_PERF_EVENTS
- call->perf_event_enable = probe_perf_enable;
- call->perf_event_disable = probe_perf_disable;
-#endif
+ call->class->reg = kprobe_register;
call->data = tp;
ret = trace_add_event_call(call);
if (ret) {
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index d036a74..bf061d3 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -15,8 +15,19 @@ static int sys_refcount_exit;
static DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls);
static DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);
-struct ftrace_event_class event_class_syscalls = {
- .system = "syscalls"
+static int syscall_enter_register(struct ftrace_event_call *event,
+ enum trace_reg type);
+static int syscall_exit_register(struct ftrace_event_call *event,
+ enum trace_reg type);
+
+struct ftrace_event_class event_class_syscall_enter = {
+ .system = "syscalls",
+ .reg = syscall_enter_register
+};
+
+struct ftrace_event_class event_class_syscall_exit = {
+ .system = "syscalls",
+ .reg = syscall_exit_register
};
extern unsigned long __start_syscalls_metadata[];
@@ -587,3 +598,44 @@ void perf_sysexit_disable(struct ftrace_event_call *call)
#endif /* CONFIG_PERF_EVENTS */
+static int syscall_enter_register(struct ftrace_event_call *event,
+ enum trace_reg type)
+{
+ switch (type) {
+ case TRACE_REG_REGISTER:
+ return reg_event_syscall_enter(event);
+ case TRACE_REG_UNREGISTER:
+ unreg_event_syscall_enter(event);
+ return 0;
+
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_REG_PERF_REGISTER:
+ return perf_sysenter_enable(event);
+ case TRACE_REG_PERF_UNREGISTER:
+ perf_sysenter_disable(event);
+ return 0;
+#endif
+ }
+ return 0;
+}
+
+static int syscall_exit_register(struct ftrace_event_call *event,
+ enum trace_reg type)
+{
+ switch (type) {
+ case TRACE_REG_REGISTER:
+ return reg_event_syscall_exit(event);
+ case TRACE_REG_UNREGISTER:
+ unreg_event_syscall_exit(event);
+ return 0;
+
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_REG_PERF_REGISTER:
+ return perf_sysexit_enable(event);
+ case TRACE_REG_PERF_UNREGISTER:
+ perf_sysexit_disable(event);
+ return 0;
+#endif
+ }
+ return 0;
+}
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (2 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-07 4:49 ` Frederic Weisbecker
2010-05-04 3:40 ` [PATCH 5/9 - v2][RFC] tracing: Move raw_init from events to class Steven Rostedt
` (4 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers, Tom Zanussi
[-- Attachment #1: 0004-tracing-Move-fields-from-event-to-class-structure.patch --]
[-- Type: text/plain, Size: 19530 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Move the defined fields from the event to the class structure.
Since the fields of the event are defined by the class they belong
to, it makes sense to have the class hold the information instead
of the individual events. The events of the same class would just
hold duplicate information.
After this change the size of the kernel dropped another 8K:
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5774316 1306580 9351592 16432488 fabd68 vmlinux.reg
5774503 1297492 9351592 16423587 fa9aa3 vmlinux.fields
Although the text increased, this was mainly due to the C files
having to adapt to the change. This is a constant increase, where
new tracepoints will not increase the Text. But the big drop is
in the data size (as well as needed allocations to hold the fields).
This will give even more savings as more tracepoints are created.
Note, if just TRACE_EVENT()s are used and not DECLARE_EVENT_CLASS()
with several DEFINE_EVENT()s, then the savings will be lost. But
we are pushing developers to consolidate events with DEFINE_EVENT()
so this should not be an issue.
The kprobes define a unique class to every new event, but are dynamic
so it should not be a issue.
The syscalls however have a single class but the fields for the individual
events are different. The syscalls use a metadata to define the
fields. I moved the fields list from the event to the metadata and
added a "get_fields()" function to the class. This function is used
to find the fields. For normal events and kprobes, get_fields() just
returns a pointer to the fields list_head in the class. For syscall
events, it returns the fields list_head in the metadata for the event.
v2: Fixed the syscall fields. The syscall metadata needs a list
of fields for both enter and exit.
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 5 ++-
include/linux/syscalls.h | 14 ++++++----
include/trace/ftrace.h | 11 ++++---
include/trace/syscall.h | 4 +-
kernel/trace/trace.h | 3 ++
kernel/trace/trace_events.c | 48 +++++++++++++++++++++++++++---------
kernel/trace/trace_events_filter.c | 10 ++++---
kernel/trace/trace_export.c | 14 +++++-----
kernel/trace/trace_kprobe.c | 8 +++---
kernel/trace/trace_syscalls.c | 31 ++++++++++++++++++++---
10 files changed, 102 insertions(+), 46 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index e665ed3..479c3c1 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -130,6 +130,9 @@ struct ftrace_event_class {
#endif
int (*reg)(struct ftrace_event_call *event,
enum trace_reg type);
+ int (*define_fields)(struct ftrace_event_call *);
+ struct list_head *(*get_fields)(struct ftrace_event_call *);
+ struct list_head fields;
};
struct ftrace_event_call {
@@ -142,8 +145,6 @@ struct ftrace_event_call {
int id;
const char *print_fmt;
int (*raw_init)(struct ftrace_event_call *);
- int (*define_fields)(struct ftrace_event_call *);
- struct list_head fields;
int filter_active;
struct event_filter *filter;
void *mod;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e3348c4..fd0f1f2 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -122,7 +122,7 @@ extern struct ftrace_event_class event_class_syscall_enter;
extern struct ftrace_event_class event_class_syscall_exit;
#define SYSCALL_TRACE_ENTER_EVENT(sname) \
- static const struct syscall_metadata __syscall_meta_##sname; \
+ static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_enter_##sname; \
static struct trace_event enter_syscall_print_##sname = { \
@@ -136,12 +136,11 @@ extern struct ftrace_event_class event_class_syscall_exit;
.class = &event_class_syscall_enter, \
.event = &enter_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
- .define_fields = syscall_enter_define_fields, \
.data = (void *)&__syscall_meta_##sname,\
}
#define SYSCALL_TRACE_EXIT_EVENT(sname) \
- static const struct syscall_metadata __syscall_meta_##sname; \
+ static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_exit_##sname; \
static struct trace_event exit_syscall_print_##sname = { \
@@ -155,14 +154,13 @@ extern struct ftrace_event_class event_class_syscall_exit;
.class = &event_class_syscall_exit, \
.event = &exit_syscall_print_##sname, \
.raw_init = init_syscall_trace, \
- .define_fields = syscall_exit_define_fields, \
.data = (void *)&__syscall_meta_##sname,\
}
#define SYSCALL_METADATA(sname, nb) \
SYSCALL_TRACE_ENTER_EVENT(sname); \
SYSCALL_TRACE_EXIT_EVENT(sname); \
- static const struct syscall_metadata __used \
+ static struct syscall_metadata __used \
__attribute__((__aligned__(4))) \
__attribute__((section("__syscalls_metadata"))) \
__syscall_meta_##sname = { \
@@ -172,12 +170,14 @@ extern struct ftrace_event_class event_class_syscall_exit;
.args = args_##sname, \
.enter_event = &event_enter_##sname, \
.exit_event = &event_exit_##sname, \
+ .enter_fields = LIST_HEAD_INIT(__syscall_meta_##sname.enter_fields), \
+ .exit_fields = LIST_HEAD_INIT(__syscall_meta_##sname.exit_fields), \
};
#define SYSCALL_DEFINE0(sname) \
SYSCALL_TRACE_ENTER_EVENT(_##sname); \
SYSCALL_TRACE_EXIT_EVENT(_##sname); \
- static const struct syscall_metadata __used \
+ static struct syscall_metadata __used \
__attribute__((__aligned__(4))) \
__attribute__((section("__syscalls_metadata"))) \
__syscall_meta__##sname = { \
@@ -185,6 +185,8 @@ extern struct ftrace_event_class event_class_syscall_exit;
.nb_args = 0, \
.enter_event = &event_enter__##sname, \
.exit_event = &event_exit__##sname, \
+ .enter_fields = LIST_HEAD_INIT(__syscall_meta__##sname.enter_fields), \
+ .exit_fields = LIST_HEAD_INIT(__syscall_meta__##sname.exit_fields), \
}; \
asmlinkage long sys_##sname(void)
#else
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index bc01395..4db8448 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -430,6 +430,9 @@ static inline notrace int ftrace_get_offsets_##call( \
*
* static struct ftrace_event_class __used event_class_<template> = {
* .system = "<system>",
+ * .define_fields = ftrace_define_fields_<call>,
+ .fields = LIST_HEAD_INIT(event_class_##call.fields),\
+ .probe = ftrace_raw_event_##call, \
* }
*
* static struct ftrace_event_call __used
@@ -438,10 +441,8 @@ static inline notrace int ftrace_get_offsets_##call( \
* .name = "<call>",
* .class = event_class_<template>,
* .raw_init = trace_event_raw_init,
- * .regfunc = ftrace_raw_reg_event_<call>,
- * .unregfunc = ftrace_raw_unreg_event_<call>,
+ * .event = &ftrace_event_type_<call>,
* .print_fmt = print_fmt_<call>,
- * .define_fields = ftrace_define_fields_<call>,
* }
*
*/
@@ -553,6 +554,8 @@ _TRACE_PERF_PROTO(call, PARAMS(proto)); \
static const char print_fmt_##call[] = print; \
static struct ftrace_event_class __used event_class_##call = { \
.system = __stringify(TRACE_SYSTEM), \
+ .define_fields = ftrace_define_fields_##call, \
+ .fields = LIST_HEAD_INIT(event_class_##call.fields),\
.probe = ftrace_raw_event_##call, \
_TRACE_PERF_INIT(call) \
}
@@ -568,7 +571,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
.print_fmt = print_fmt_##template, \
- .define_fields = ftrace_define_fields_##template, \
}
#undef DEFINE_EVENT_PRINT
@@ -584,7 +586,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.event = &ftrace_event_type_##call, \
.raw_init = trace_event_raw_init, \
.print_fmt = print_fmt_##call, \
- .define_fields = ftrace_define_fields_##template, \
}
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index e5e5f48..3964774 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -25,6 +25,8 @@ struct syscall_metadata {
int nb_args;
const char **types;
const char **args;
+ struct list_head enter_fields;
+ struct list_head exit_fields;
struct ftrace_event_call *enter_event;
struct ftrace_event_call *exit_event;
@@ -34,8 +36,6 @@ struct syscall_metadata {
extern unsigned long arch_syscall_addr(int nr);
extern int init_syscall_trace(struct ftrace_event_call *call);
-extern int syscall_enter_define_fields(struct ftrace_event_call *call);
-extern int syscall_exit_define_fields(struct ftrace_event_call *call);
extern int reg_event_syscall_enter(struct ftrace_event_call *call);
extern void unreg_event_syscall_enter(struct ftrace_event_call *call);
extern int reg_event_syscall_exit(struct ftrace_event_call *call);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 911e986..c88c563 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -794,6 +794,9 @@ extern void print_subsystem_event_filter(struct event_subsystem *system,
struct trace_seq *s);
extern int filter_assign_type(const char *type);
+struct list_head *
+trace_get_fields(struct ftrace_event_call *event_call);
+
static inline int
filter_check_discard(struct ftrace_event_call *call, void *rec,
struct ring_buffer *buffer,
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0eb9b28..a446908 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -29,11 +29,23 @@ DEFINE_MUTEX(event_mutex);
LIST_HEAD(ftrace_events);
+struct list_head *
+trace_get_fields(struct ftrace_event_call *event_call)
+{
+ if (!event_call->class->get_fields)
+ return &event_call->class->fields;
+ return event_call->class->get_fields(event_call);
+}
+
int trace_define_field(struct ftrace_event_call *call, const char *type,
const char *name, int offset, int size, int is_signed,
int filter_type)
{
struct ftrace_event_field *field;
+ struct list_head *head;
+
+ if (WARN_ON(!call->class))
+ return 0;
field = kzalloc(sizeof(*field), GFP_KERNEL);
if (!field)
@@ -56,7 +68,8 @@ int trace_define_field(struct ftrace_event_call *call, const char *type,
field->size = size;
field->is_signed = is_signed;
- list_add(&field->link, &call->fields);
+ head = trace_get_fields(call);
+ list_add(&field->link, head);
return 0;
@@ -94,8 +107,10 @@ static int trace_define_common_fields(struct ftrace_event_call *call)
void trace_destroy_fields(struct ftrace_event_call *call)
{
struct ftrace_event_field *field, *next;
+ struct list_head *head;
- list_for_each_entry_safe(field, next, &call->fields, link) {
+ head = trace_get_fields(call);
+ list_for_each_entry_safe(field, next, head, link) {
list_del(&field->link);
kfree(field->type);
kfree(field->name);
@@ -111,7 +126,6 @@ int trace_event_raw_init(struct ftrace_event_call *call)
if (!id)
return -ENODEV;
call->id = id;
- INIT_LIST_HEAD(&call->fields);
return 0;
}
@@ -537,6 +551,7 @@ event_format_read(struct file *filp, char __user *ubuf, size_t cnt,
{
struct ftrace_event_call *call = filp->private_data;
struct ftrace_event_field *field;
+ struct list_head *head;
struct trace_seq *s;
int common_field_count = 5;
char *buf;
@@ -555,7 +570,8 @@ event_format_read(struct file *filp, char __user *ubuf, size_t cnt,
trace_seq_printf(s, "ID: %d\n", call->id);
trace_seq_printf(s, "format:\n");
- list_for_each_entry_reverse(field, &call->fields, link) {
+ head = trace_get_fields(call);
+ list_for_each_entry_reverse(field, head, link) {
/*
* Smartly shows the array type(except dynamic array).
* Normal:
@@ -931,6 +947,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
const struct file_operations *filter,
const struct file_operations *format)
{
+ struct list_head *head;
int ret;
/*
@@ -955,14 +972,21 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
trace_create_file("id", 0444, call->dir, call,
id);
- if (call->define_fields) {
- ret = trace_define_common_fields(call);
- if (!ret)
- ret = call->define_fields(call);
- if (ret < 0) {
- pr_warning("Could not initialize trace point"
- " events/%s\n", call->name);
- return ret;
+ if (call->class->define_fields) {
+ /*
+ * Other events may have the same class. Only update
+ * the fields if they are not already defined.
+ */
+ head = trace_get_fields(call);
+ if (list_empty(head)) {
+ ret = trace_define_common_fields(call);
+ if (!ret)
+ ret = call->class->define_fields(call);
+ if (ret < 0) {
+ pr_warning("Could not initialize trace point"
+ " events/%s\n", call->name);
+ return ret;
+ }
}
trace_create_file("filter", 0644, call->dir, call,
filter);
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ca32960..961f99b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -500,8 +500,10 @@ static struct ftrace_event_field *
find_event_field(struct ftrace_event_call *call, char *name)
{
struct ftrace_event_field *field;
+ struct list_head *head;
- list_for_each_entry(field, &call->fields, link) {
+ head = trace_get_fields(call);
+ list_for_each_entry(field, head, link) {
if (!strcmp(field->name, name))
return field;
}
@@ -625,7 +627,7 @@ static int init_subsystem_preds(struct event_subsystem *system)
int err;
list_for_each_entry(call, &ftrace_events, list) {
- if (!call->define_fields)
+ if (!call->class || !call->class->define_fields)
continue;
if (strcmp(call->class->system, system->name) != 0)
@@ -644,7 +646,7 @@ static void filter_free_subsystem_preds(struct event_subsystem *system)
struct ftrace_event_call *call;
list_for_each_entry(call, &ftrace_events, list) {
- if (!call->define_fields)
+ if (!call->class || !call->class->define_fields)
continue;
if (strcmp(call->class->system, system->name) != 0)
@@ -1249,7 +1251,7 @@ static int replace_system_preds(struct event_subsystem *system,
list_for_each_entry(call, &ftrace_events, list) {
struct event_filter *filter = call->filter;
- if (!call->define_fields)
+ if (!call->class || !call->class->define_fields)
continue;
if (strcmp(call->class->system, system->name) != 0)
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 7f16e21..e700a0c 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -18,10 +18,6 @@
#undef TRACE_SYSTEM
#define TRACE_SYSTEM ftrace
-struct ftrace_event_class event_class_ftrace = {
- .system = __stringify(TRACE_SYSTEM),
-};
-
/* not needed for this file */
#undef __field_struct
#define __field_struct(type, item)
@@ -131,7 +127,7 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call) \
static int ftrace_raw_init_event(struct ftrace_event_call *call)
{
- INIT_LIST_HEAD(&call->fields);
+ INIT_LIST_HEAD(&call->class->fields);
return 0;
}
@@ -159,15 +155,19 @@ static int ftrace_raw_init_event(struct ftrace_event_call *call)
#undef FTRACE_ENTRY
#define FTRACE_ENTRY(call, struct_name, type, tstruct, print) \
\
+struct ftrace_event_class event_class_ftrace_##call = { \
+ .system = __stringify(TRACE_SYSTEM), \
+ .define_fields = ftrace_define_fields_##call, \
+}; \
+ \
struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.id = type, \
- .class = &event_class_ftrace, \
+ .class = &event_class_ftrace_##call, \
.raw_init = ftrace_raw_init_event, \
.print_fmt = print, \
- .define_fields = ftrace_define_fields_##call, \
}; \
#include "trace_entries.h"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index f8af21a..b14bf74 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1112,8 +1112,6 @@ static void probe_event_disable(struct ftrace_event_call *call)
static int probe_event_raw_init(struct ftrace_event_call *event_call)
{
- INIT_LIST_HEAD(&event_call->fields);
-
return 0;
}
@@ -1362,11 +1360,13 @@ static int register_probe_event(struct trace_probe *tp)
if (probe_is_return(tp)) {
tp->event.trace = print_kretprobe_event;
call->raw_init = probe_event_raw_init;
- call->define_fields = kretprobe_event_define_fields;
+ INIT_LIST_HEAD(&call->class->fields);
+ call->class->define_fields = kretprobe_event_define_fields;
} else {
tp->event.trace = print_kprobe_event;
call->raw_init = probe_event_raw_init;
- call->define_fields = kprobe_event_define_fields;
+ INIT_LIST_HEAD(&call->class->fields);
+ call->class->define_fields = kprobe_event_define_fields;
}
if (set_print_fmt(tp) < 0)
return -ENOMEM;
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index bf061d3..0919841 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -20,14 +20,37 @@ static int syscall_enter_register(struct ftrace_event_call *event,
static int syscall_exit_register(struct ftrace_event_call *event,
enum trace_reg type);
+static int syscall_enter_define_fields(struct ftrace_event_call *call);
+static int syscall_exit_define_fields(struct ftrace_event_call *call);
+
+static struct list_head *
+syscall_get_enter_fields(struct ftrace_event_call *call)
+{
+ struct syscall_metadata *entry = call->data;
+
+ return &entry->enter_fields;
+}
+
+static struct list_head *
+syscall_get_exit_fields(struct ftrace_event_call *call)
+{
+ struct syscall_metadata *entry = call->data;
+
+ return &entry->exit_fields;
+}
+
struct ftrace_event_class event_class_syscall_enter = {
.system = "syscalls",
- .reg = syscall_enter_register
+ .reg = syscall_enter_register,
+ .define_fields = syscall_enter_define_fields,
+ .get_fields = syscall_get_enter_fields,
};
struct ftrace_event_class event_class_syscall_exit = {
.system = "syscalls",
- .reg = syscall_exit_register
+ .reg = syscall_exit_register,
+ .define_fields = syscall_exit_define_fields,
+ .get_fields = syscall_get_exit_fields,
};
extern unsigned long __start_syscalls_metadata[];
@@ -220,7 +243,7 @@ static void free_syscall_print_fmt(struct ftrace_event_call *call)
kfree(call->print_fmt);
}
-int syscall_enter_define_fields(struct ftrace_event_call *call)
+static int syscall_enter_define_fields(struct ftrace_event_call *call)
{
struct syscall_trace_enter trace;
struct syscall_metadata *meta = call->data;
@@ -243,7 +266,7 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
return ret;
}
-int syscall_exit_define_fields(struct ftrace_event_call *call)
+static int syscall_exit_define_fields(struct ftrace_event_call *call)
{
struct syscall_trace_exit trace;
int ret;
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 5/9 - v2][RFC] tracing: Move raw_init from events to class
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (3 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 6/9 - v2][RFC] tracing: Allow events to share their print functions Steven Rostedt
` (3 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
[-- Attachment #1: 0005-tracing-Move-raw_init-from-events-to-class.patch --]
[-- Type: text/plain, Size: 8504 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
The raw_init function pointer in the event is used to initialize
various kinds of events. The type of initialization needed is usually
classed to the kind of event it is.
Two events with the same class will always have the same initialization
function, so it makes sense to move this to the class structure.
Perhaps even making a special system structure would work since
the initialization is the same for all events within a system.
But since there's no system structure (yet), this will just move it
to the class.
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5774567 1297492 9351592 16423651 fa9ae3 vmlinux.fields
5774510 1293204 9351592 16419306 fa89ea vmlinux.init
The text grew very slightly, but this is a constant growth that happened
with the changing of the C files that call the init code.
The bigger savings is the data which will be saved the more events share
a class.
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 2 +-
include/linux/syscalls.h | 2 --
include/trace/ftrace.h | 9 ++++-----
kernel/trace/trace_events.c | 12 ++++++------
kernel/trace/trace_export.c | 2 +-
kernel/trace/trace_kprobe.c | 6 +++---
kernel/trace/trace_syscalls.c | 2 ++
7 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 479c3c1..393a839 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -133,6 +133,7 @@ struct ftrace_event_class {
int (*define_fields)(struct ftrace_event_call *);
struct list_head *(*get_fields)(struct ftrace_event_call *);
struct list_head fields;
+ int (*raw_init)(struct ftrace_event_call *);
};
struct ftrace_event_call {
@@ -144,7 +145,6 @@ struct ftrace_event_call {
int enabled;
int id;
const char *print_fmt;
- int (*raw_init)(struct ftrace_event_call *);
int filter_active;
struct event_filter *filter;
void *mod;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index fd0f1f2..211c704 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -135,7 +135,6 @@ extern struct ftrace_event_class event_class_syscall_exit;
.name = "sys_enter"#sname, \
.class = &event_class_syscall_enter, \
.event = &enter_syscall_print_##sname, \
- .raw_init = init_syscall_trace, \
.data = (void *)&__syscall_meta_##sname,\
}
@@ -153,7 +152,6 @@ extern struct ftrace_event_class event_class_syscall_exit;
.name = "sys_exit"#sname, \
.class = &event_class_syscall_exit, \
.event = &exit_syscall_print_##sname, \
- .raw_init = init_syscall_trace, \
.data = (void *)&__syscall_meta_##sname,\
}
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 4db8448..b4cd203 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -431,8 +431,9 @@ static inline notrace int ftrace_get_offsets_##call( \
* static struct ftrace_event_class __used event_class_<template> = {
* .system = "<system>",
* .define_fields = ftrace_define_fields_<call>,
- .fields = LIST_HEAD_INIT(event_class_##call.fields),\
- .probe = ftrace_raw_event_##call, \
+ * .fields = LIST_HEAD_INIT(event_class_##call.fields),
+ * .raw_init = trace_event_raw_init,
+ * .probe = ftrace_raw_event_##call,
* }
*
* static struct ftrace_event_call __used
@@ -440,7 +441,6 @@ static inline notrace int ftrace_get_offsets_##call( \
* __attribute__((section("_ftrace_events"))) event_<call> = {
* .name = "<call>",
* .class = event_class_<template>,
- * .raw_init = trace_event_raw_init,
* .event = &ftrace_event_type_<call>,
* .print_fmt = print_fmt_<call>,
* }
@@ -556,6 +556,7 @@ static struct ftrace_event_class __used event_class_##call = { \
.system = __stringify(TRACE_SYSTEM), \
.define_fields = ftrace_define_fields_##call, \
.fields = LIST_HEAD_INIT(event_class_##call.fields),\
+ .raw_init = trace_event_raw_init, \
.probe = ftrace_raw_event_##call, \
_TRACE_PERF_INIT(call) \
}
@@ -569,7 +570,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
- .raw_init = trace_event_raw_init, \
.print_fmt = print_fmt_##template, \
}
@@ -584,7 +584,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
.event = &ftrace_event_type_##call, \
- .raw_init = trace_event_raw_init, \
.print_fmt = print_fmt_##call, \
}
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index a446908..7cfd91b 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1006,8 +1006,8 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
if (!call->name)
return -EINVAL;
- if (call->raw_init) {
- ret = call->raw_init(call);
+ if (call->class->raw_init) {
+ ret = call->class->raw_init(call);
if (ret < 0) {
if (ret != -ENOSYS)
pr_warning("Could not initialize trace "
@@ -1168,8 +1168,8 @@ static void trace_module_add_events(struct module *mod)
/* The linker may leave blanks */
if (!call->name)
continue;
- if (call->raw_init) {
- ret = call->raw_init(call);
+ if (call->class->raw_init) {
+ ret = call->class->raw_init(call);
if (ret < 0) {
if (ret != -ENOSYS)
pr_warning("Could not initialize trace "
@@ -1322,8 +1322,8 @@ static __init int event_trace_init(void)
/* The linker may leave blanks */
if (!call->name)
continue;
- if (call->raw_init) {
- ret = call->raw_init(call);
+ if (call->class->raw_init) {
+ ret = call->class->raw_init(call);
if (ret < 0) {
if (ret != -ENOSYS)
pr_warning("Could not initialize trace "
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index e700a0c..e878d06 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -158,6 +158,7 @@ static int ftrace_raw_init_event(struct ftrace_event_call *call)
struct ftrace_event_class event_class_ftrace_##call = { \
.system = __stringify(TRACE_SYSTEM), \
.define_fields = ftrace_define_fields_##call, \
+ .raw_init = ftrace_raw_init_event, \
}; \
\
struct ftrace_event_call __used \
@@ -166,7 +167,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.id = type, \
.class = &event_class_ftrace_##call, \
- .raw_init = ftrace_raw_init_event, \
.print_fmt = print, \
}; \
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b14bf74..428f4a5 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1359,13 +1359,13 @@ static int register_probe_event(struct trace_probe *tp)
/* Initialize ftrace_event_call */
if (probe_is_return(tp)) {
tp->event.trace = print_kretprobe_event;
- call->raw_init = probe_event_raw_init;
INIT_LIST_HEAD(&call->class->fields);
+ call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kretprobe_event_define_fields;
} else {
- tp->event.trace = print_kprobe_event;
- call->raw_init = probe_event_raw_init;
INIT_LIST_HEAD(&call->class->fields);
+ tp->event.trace = print_kprobe_event;
+ call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kprobe_event_define_fields;
}
if (set_print_fmt(tp) < 0)
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 0919841..5887a57 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -44,6 +44,7 @@ struct ftrace_event_class event_class_syscall_enter = {
.reg = syscall_enter_register,
.define_fields = syscall_enter_define_fields,
.get_fields = syscall_get_enter_fields,
+ .raw_init = init_syscall_trace,
};
struct ftrace_event_class event_class_syscall_exit = {
@@ -51,6 +52,7 @@ struct ftrace_event_class event_class_syscall_exit = {
.reg = syscall_exit_register,
.define_fields = syscall_exit_define_fields,
.get_fields = syscall_get_exit_fields,
+ .raw_init = init_syscall_trace,
};
extern unsigned long __start_syscalls_metadata[];
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 6/9 - v2][RFC] tracing: Allow events to share their print functions
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (4 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 5/9 - v2][RFC] tracing: Move raw_init from events to class Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 7/9 - v2][RFC] tracing: Move print functions into event class Steven Rostedt
` (2 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
[-- Attachment #1: 0006-tracing-Allow-events-to-share-their-print-functions.patch --]
[-- Type: text/plain, Size: 27843 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Multiple events may use the same method to print their data.
Instead of having all events have a pointer to their print funtions,
the trace_event structure now points to a trace_event_functions structure
that will hold the way to print ouf the event.
The event itself is now passed to the print function to let the print
function know what kind of event it should print.
This opens the door to consolidating the way several events print
their output.
v2: Fix the new function graph tracer event calls to handle this change.
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 17 +++-
include/linux/syscalls.h | 10 ++-
include/trace/ftrace.h | 12 ++-
include/trace/syscall.h | 6 +-
kernel/trace/blktrace.c | 13 ++-
kernel/trace/kmemtrace.c | 28 +++++--
kernel/trace/trace.c | 9 +-
kernel/trace/trace_functions_graph.c | 13 ++-
kernel/trace/trace_kprobe.c | 22 ++++--
kernel/trace/trace_output.c | 137 +++++++++++++++++++++++-----------
kernel/trace/trace_output.h | 2 +-
kernel/trace/trace_syscalls.c | 6 +-
12 files changed, 186 insertions(+), 89 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 393a839..4f77932 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -70,18 +70,25 @@ struct trace_iterator {
};
+struct trace_event;
+
typedef enum print_line_t (*trace_print_func)(struct trace_iterator *iter,
- int flags);
-struct trace_event {
- struct hlist_node node;
- struct list_head list;
- int type;
+ int flags, struct trace_event *event);
+
+struct trace_event_functions {
trace_print_func trace;
trace_print_func raw;
trace_print_func hex;
trace_print_func binary;
};
+struct trace_event {
+ struct hlist_node node;
+ struct list_head list;
+ int type;
+ struct trace_event_functions *funcs;
+};
+
extern int register_ftrace_event(struct trace_event *event);
extern int unregister_ftrace_event(struct trace_event *event);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 211c704..f725677 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -125,9 +125,12 @@ extern struct ftrace_event_class event_class_syscall_exit;
static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_enter_##sname; \
- static struct trace_event enter_syscall_print_##sname = { \
+ static struct trace_event_functions enter_syscall_print_funcs_##sname = { \
.trace = print_syscall_enter, \
}; \
+ static struct trace_event enter_syscall_print_##sname = { \
+ .funcs = &enter_syscall_print_funcs_##sname, \
+ }; \
static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
@@ -142,9 +145,12 @@ extern struct ftrace_event_class event_class_syscall_exit;
static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_exit_##sname; \
- static struct trace_event exit_syscall_print_##sname = { \
+ static struct trace_event_functions exit_syscall_print_funcs_##sname = { \
.trace = print_syscall_exit, \
}; \
+ static struct trace_event exit_syscall_print_##sname = { \
+ .funcs = &exit_syscall_print_funcs_##sname, \
+ }; \
static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index b4cd203..78650f7 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -239,7 +239,8 @@ ftrace_raw_output_id_##call(int event_id, const char *name, \
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
static notrace enum print_line_t \
-ftrace_raw_output_##name(struct trace_iterator *iter, int flags) \
+ftrace_raw_output_##name(struct trace_iterator *iter, int flags, \
+ struct trace_event *event) \
{ \
return ftrace_raw_output_id_##template(event_##name.id, \
#name, iter, flags); \
@@ -248,7 +249,8 @@ ftrace_raw_output_##name(struct trace_iterator *iter, int flags) \
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, call, proto, args, print) \
static notrace enum print_line_t \
-ftrace_raw_output_##call(struct trace_iterator *iter, int flags) \
+ftrace_raw_output_##call(struct trace_iterator *iter, int flags, \
+ struct trace_event *event) \
{ \
struct trace_seq *s = &iter->seq; \
struct ftrace_raw_##template *field; \
@@ -526,9 +528,11 @@ ftrace_raw_event_##call(proto, void *__data) \
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, call, proto, args) \
- \
-static struct trace_event ftrace_event_type_##call = { \
+static struct trace_event_functions ftrace_event_type_funcs_##call = { \
.trace = ftrace_raw_output_##call, \
+}; \
+static struct trace_event ftrace_event_type_##call = { \
+ .funcs = &ftrace_event_type_funcs_##call, \
};
#undef DEFINE_EVENT_PRINT
diff --git a/include/trace/syscall.h b/include/trace/syscall.h
index 3964774..257e089 100644
--- a/include/trace/syscall.h
+++ b/include/trace/syscall.h
@@ -42,8 +42,10 @@ extern int reg_event_syscall_exit(struct ftrace_event_call *call);
extern void unreg_event_syscall_exit(struct ftrace_event_call *call);
extern int
ftrace_format_syscall(struct ftrace_event_call *call, struct trace_seq *s);
-enum print_line_t print_syscall_enter(struct trace_iterator *iter, int flags);
-enum print_line_t print_syscall_exit(struct trace_iterator *iter, int flags);
+enum print_line_t print_syscall_enter(struct trace_iterator *iter, int flags,
+ struct trace_event *event);
+enum print_line_t print_syscall_exit(struct trace_iterator *iter, int flags,
+ struct trace_event *event);
#endif
#ifdef CONFIG_PERF_EVENTS
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index b3bc91a..b671c30 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1321,7 +1321,7 @@ out:
}
static enum print_line_t blk_trace_event_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
return print_one_line(iter, false);
}
@@ -1343,7 +1343,8 @@ static int blk_trace_synthesize_old_trace(struct trace_iterator *iter)
}
static enum print_line_t
-blk_trace_event_print_binary(struct trace_iterator *iter, int flags)
+blk_trace_event_print_binary(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return blk_trace_synthesize_old_trace(iter) ?
TRACE_TYPE_HANDLED : TRACE_TYPE_PARTIAL_LINE;
@@ -1381,12 +1382,16 @@ static struct tracer blk_tracer __read_mostly = {
.set_flag = blk_tracer_set_flag,
};
-static struct trace_event trace_blk_event = {
- .type = TRACE_BLK,
+static struct trace_event_functions trace_blk_event_funcs = {
.trace = blk_trace_event_print,
.binary = blk_trace_event_print_binary,
};
+static struct trace_event trace_blk_event = {
+ .type = TRACE_BLK,
+ .funcs = &trace_blk_event_funcs,
+};
+
static int __init init_blk_tracer(void)
{
if (!register_ftrace_event(&trace_blk_event)) {
diff --git a/kernel/trace/kmemtrace.c b/kernel/trace/kmemtrace.c
index a91da69..6a24fe0 100644
--- a/kernel/trace/kmemtrace.c
+++ b/kernel/trace/kmemtrace.c
@@ -237,7 +237,8 @@ struct kmemtrace_user_event_alloc {
};
static enum print_line_t
-kmemtrace_print_alloc(struct trace_iterator *iter, int flags)
+kmemtrace_print_alloc(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct kmemtrace_alloc_entry *entry;
@@ -257,7 +258,8 @@ kmemtrace_print_alloc(struct trace_iterator *iter, int flags)
}
static enum print_line_t
-kmemtrace_print_free(struct trace_iterator *iter, int flags)
+kmemtrace_print_free(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct kmemtrace_free_entry *entry;
@@ -275,7 +277,8 @@ kmemtrace_print_free(struct trace_iterator *iter, int flags)
}
static enum print_line_t
-kmemtrace_print_alloc_user(struct trace_iterator *iter, int flags)
+kmemtrace_print_alloc_user(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct kmemtrace_alloc_entry *entry;
@@ -309,7 +312,8 @@ kmemtrace_print_alloc_user(struct trace_iterator *iter, int flags)
}
static enum print_line_t
-kmemtrace_print_free_user(struct trace_iterator *iter, int flags)
+kmemtrace_print_free_user(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct kmemtrace_free_entry *entry;
@@ -463,18 +467,26 @@ static enum print_line_t kmemtrace_print_line(struct trace_iterator *iter)
}
}
-static struct trace_event kmem_trace_alloc = {
- .type = TRACE_KMEM_ALLOC,
+static struct trace_event_functions kmem_trace_alloc_funcs = {
.trace = kmemtrace_print_alloc,
.binary = kmemtrace_print_alloc_user,
};
-static struct trace_event kmem_trace_free = {
- .type = TRACE_KMEM_FREE,
+static struct trace_event kmem_trace_alloc = {
+ .type = TRACE_KMEM_ALLOC,
+ .funcs = &kmem_trace_alloc_funcs,
+};
+
+static struct trace_event_functions kmem_trace_free_funcs = {
.trace = kmemtrace_print_free,
.binary = kmemtrace_print_free_user,
};
+static struct trace_event kmem_trace_free = {
+ .type = TRACE_KMEM_FREE,
+ .funcs = &kmem_trace_free_funcs,
+};
+
static struct tracer kmem_tracer __read_mostly = {
.name = "kmemtrace",
.init = kmem_trace_init,
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 756d728..ba0ec81 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1936,7 +1936,7 @@ static enum print_line_t print_trace_fmt(struct trace_iterator *iter)
}
if (event)
- return event->trace(iter, sym_flags);
+ return event->funcs->trace(iter, sym_flags, event);
if (!trace_seq_printf(s, "Unknown type %d\n", entry->type))
goto partial;
@@ -1962,7 +1962,7 @@ static enum print_line_t print_raw_fmt(struct trace_iterator *iter)
event = ftrace_find_event(entry->type);
if (event)
- return event->raw(iter, 0);
+ return event->funcs->raw(iter, 0, event);
if (!trace_seq_printf(s, "%d ?\n", entry->type))
goto partial;
@@ -1989,7 +1989,7 @@ static enum print_line_t print_hex_fmt(struct trace_iterator *iter)
event = ftrace_find_event(entry->type);
if (event) {
- enum print_line_t ret = event->hex(iter, 0);
+ enum print_line_t ret = event->funcs->hex(iter, 0, event);
if (ret != TRACE_TYPE_HANDLED)
return ret;
}
@@ -2014,7 +2014,8 @@ static enum print_line_t print_bin_fmt(struct trace_iterator *iter)
}
event = ftrace_find_event(entry->type);
- return event ? event->binary(iter, 0) : TRACE_TYPE_HANDLED;
+ return event ? event->funcs->binary(iter, 0, event) :
+ TRACE_TYPE_HANDLED;
}
int trace_empty(struct trace_iterator *iter)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index dd11c83..79f4bac 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -1025,7 +1025,7 @@ print_graph_comment(struct trace_seq *s, struct trace_entry *ent,
if (!event)
return TRACE_TYPE_UNHANDLED;
- ret = event->trace(iter, sym_flags);
+ ret = event->funcs->trace(iter, sym_flags, event);
if (ret != TRACE_TYPE_HANDLED)
return ret;
}
@@ -1112,7 +1112,8 @@ print_graph_function(struct trace_iterator *iter)
}
static enum print_line_t
-print_graph_function_event(struct trace_iterator *iter, int flags)
+print_graph_function_event(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return print_graph_function(iter);
}
@@ -1225,14 +1226,18 @@ void graph_trace_close(struct trace_iterator *iter)
}
}
+static struct trace_event_functions graph_functions = {
+ .trace = print_graph_function_event,
+};
+
static struct trace_event graph_trace_entry_event = {
.type = TRACE_GRAPH_ENT,
- .trace = print_graph_function_event,
+ .funcs = &graph_functions,
};
static struct trace_event graph_trace_ret_event = {
.type = TRACE_GRAPH_RET,
- .trace = print_graph_function_event,
+ .funcs = &graph_functions
};
static struct tracer graph_trace __read_mostly = {
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 428f4a5..b989ae2 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1011,16 +1011,15 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
/* Event entry printers */
enum print_line_t
-print_kprobe_event(struct trace_iterator *iter, int flags)
+print_kprobe_event(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct kprobe_trace_entry *field;
struct trace_seq *s = &iter->seq;
- struct trace_event *event;
struct trace_probe *tp;
int i;
field = (struct kprobe_trace_entry *)iter->ent;
- event = ftrace_find_event(field->ent.type);
tp = container_of(event, struct trace_probe, event);
if (!trace_seq_printf(s, "%s: (", tp->call.name))
@@ -1046,16 +1045,15 @@ partial:
}
enum print_line_t
-print_kretprobe_event(struct trace_iterator *iter, int flags)
+print_kretprobe_event(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct kretprobe_trace_entry *field;
struct trace_seq *s = &iter->seq;
- struct trace_event *event;
struct trace_probe *tp;
int i;
field = (struct kretprobe_trace_entry *)iter->ent;
- event = ftrace_find_event(field->ent.type);
tp = container_of(event, struct trace_probe, event);
if (!trace_seq_printf(s, "%s: (", tp->call.name))
@@ -1351,6 +1349,14 @@ int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
return 0; /* We don't tweek kernel, so just return 0 */
}
+static struct trace_event_functions kretprobe_funcs = {
+ .trace = print_kretprobe_event
+};
+
+static struct trace_event_functions kprobe_funcs = {
+ .trace = print_kprobe_event
+};
+
static int register_probe_event(struct trace_probe *tp)
{
struct ftrace_event_call *call = &tp->call;
@@ -1358,13 +1364,13 @@ static int register_probe_event(struct trace_probe *tp)
/* Initialize ftrace_event_call */
if (probe_is_return(tp)) {
- tp->event.trace = print_kretprobe_event;
+ tp->event.funcs = &kretprobe_funcs;
INIT_LIST_HEAD(&call->class->fields);
call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kretprobe_event_define_fields;
} else {
INIT_LIST_HEAD(&call->class->fields);
- tp->event.trace = print_kprobe_event;
+ tp->event.funcs = &kprobe_funcs;
call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kprobe_event_define_fields;
}
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 8e46b33..9c00283 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -726,6 +726,9 @@ int register_ftrace_event(struct trace_event *event)
if (WARN_ON(!event))
goto out;
+ if (WARN_ON(!event->funcs))
+ goto out;
+
INIT_LIST_HEAD(&event->list);
if (!event->type) {
@@ -758,14 +761,14 @@ int register_ftrace_event(struct trace_event *event)
goto out;
}
- if (event->trace == NULL)
- event->trace = trace_nop_print;
- if (event->raw == NULL)
- event->raw = trace_nop_print;
- if (event->hex == NULL)
- event->hex = trace_nop_print;
- if (event->binary == NULL)
- event->binary = trace_nop_print;
+ if (event->funcs->trace == NULL)
+ event->funcs->trace = trace_nop_print;
+ if (event->funcs->raw == NULL)
+ event->funcs->raw = trace_nop_print;
+ if (event->funcs->hex == NULL)
+ event->funcs->hex = trace_nop_print;
+ if (event->funcs->binary == NULL)
+ event->funcs->binary = trace_nop_print;
key = event->type & (EVENT_HASHSIZE - 1);
@@ -807,13 +810,15 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_event);
* Standard events
*/
-enum print_line_t trace_nop_print(struct trace_iterator *iter, int flags)
+enum print_line_t trace_nop_print(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return TRACE_TYPE_HANDLED;
}
/* TRACE_FN */
-static enum print_line_t trace_fn_trace(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_fn_trace(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct ftrace_entry *field;
struct trace_seq *s = &iter->seq;
@@ -840,7 +845,8 @@ static enum print_line_t trace_fn_trace(struct trace_iterator *iter, int flags)
return TRACE_TYPE_PARTIAL_LINE;
}
-static enum print_line_t trace_fn_raw(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_fn_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct ftrace_entry *field;
@@ -854,7 +860,8 @@ static enum print_line_t trace_fn_raw(struct trace_iterator *iter, int flags)
return TRACE_TYPE_HANDLED;
}
-static enum print_line_t trace_fn_hex(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_fn_hex(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct ftrace_entry *field;
struct trace_seq *s = &iter->seq;
@@ -867,7 +874,8 @@ static enum print_line_t trace_fn_hex(struct trace_iterator *iter, int flags)
return TRACE_TYPE_HANDLED;
}
-static enum print_line_t trace_fn_bin(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_fn_bin(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct ftrace_entry *field;
struct trace_seq *s = &iter->seq;
@@ -880,14 +888,18 @@ static enum print_line_t trace_fn_bin(struct trace_iterator *iter, int flags)
return TRACE_TYPE_HANDLED;
}
-static struct trace_event trace_fn_event = {
- .type = TRACE_FN,
+static struct trace_event_functions trace_fn_funcs = {
.trace = trace_fn_trace,
.raw = trace_fn_raw,
.hex = trace_fn_hex,
.binary = trace_fn_bin,
};
+static struct trace_event trace_fn_event = {
+ .type = TRACE_FN,
+ .funcs = &trace_fn_funcs,
+};
+
/* TRACE_CTX an TRACE_WAKE */
static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
char *delim)
@@ -916,13 +928,14 @@ static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
return TRACE_TYPE_HANDLED;
}
-static enum print_line_t trace_ctx_print(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_ctx_print(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return trace_ctxwake_print(iter, "==>");
}
static enum print_line_t trace_wake_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
return trace_ctxwake_print(iter, " +");
}
@@ -950,12 +963,14 @@ static int trace_ctxwake_raw(struct trace_iterator *iter, char S)
return TRACE_TYPE_HANDLED;
}
-static enum print_line_t trace_ctx_raw(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_ctx_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return trace_ctxwake_raw(iter, 0);
}
-static enum print_line_t trace_wake_raw(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_wake_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return trace_ctxwake_raw(iter, '+');
}
@@ -984,18 +999,20 @@ static int trace_ctxwake_hex(struct trace_iterator *iter, char S)
return TRACE_TYPE_HANDLED;
}
-static enum print_line_t trace_ctx_hex(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_ctx_hex(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return trace_ctxwake_hex(iter, 0);
}
-static enum print_line_t trace_wake_hex(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_wake_hex(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
return trace_ctxwake_hex(iter, '+');
}
static enum print_line_t trace_ctxwake_bin(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct ctx_switch_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1012,25 +1029,33 @@ static enum print_line_t trace_ctxwake_bin(struct trace_iterator *iter,
return TRACE_TYPE_HANDLED;
}
-static struct trace_event trace_ctx_event = {
- .type = TRACE_CTX,
+static struct trace_event_functions trace_ctx_funcs = {
.trace = trace_ctx_print,
.raw = trace_ctx_raw,
.hex = trace_ctx_hex,
.binary = trace_ctxwake_bin,
};
-static struct trace_event trace_wake_event = {
- .type = TRACE_WAKE,
+static struct trace_event trace_ctx_event = {
+ .type = TRACE_CTX,
+ .funcs = &trace_ctx_funcs,
+};
+
+static struct trace_event_functions trace_wake_funcs = {
.trace = trace_wake_print,
.raw = trace_wake_raw,
.hex = trace_wake_hex,
.binary = trace_ctxwake_bin,
};
+static struct trace_event trace_wake_event = {
+ .type = TRACE_WAKE,
+ .funcs = &trace_wake_funcs,
+};
+
/* TRACE_SPECIAL */
static enum print_line_t trace_special_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct special_entry *field;
@@ -1046,7 +1071,7 @@ static enum print_line_t trace_special_print(struct trace_iterator *iter,
}
static enum print_line_t trace_special_hex(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct special_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1061,7 +1086,7 @@ static enum print_line_t trace_special_hex(struct trace_iterator *iter,
}
static enum print_line_t trace_special_bin(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct special_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1075,18 +1100,22 @@ static enum print_line_t trace_special_bin(struct trace_iterator *iter,
return TRACE_TYPE_HANDLED;
}
-static struct trace_event trace_special_event = {
- .type = TRACE_SPECIAL,
+static struct trace_event_functions trace_special_funcs = {
.trace = trace_special_print,
.raw = trace_special_print,
.hex = trace_special_hex,
.binary = trace_special_bin,
};
+static struct trace_event trace_special_event = {
+ .type = TRACE_SPECIAL,
+ .funcs = &trace_special_funcs,
+};
+
/* TRACE_STACK */
static enum print_line_t trace_stack_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct stack_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1114,17 +1143,21 @@ static enum print_line_t trace_stack_print(struct trace_iterator *iter,
return TRACE_TYPE_PARTIAL_LINE;
}
-static struct trace_event trace_stack_event = {
- .type = TRACE_STACK,
+static struct trace_event_functions trace_stack_funcs = {
.trace = trace_stack_print,
.raw = trace_special_print,
.hex = trace_special_hex,
.binary = trace_special_bin,
};
+static struct trace_event trace_stack_event = {
+ .type = TRACE_STACK,
+ .funcs = &trace_stack_funcs,
+};
+
/* TRACE_USER_STACK */
static enum print_line_t trace_user_stack_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct userstack_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1143,17 +1176,22 @@ static enum print_line_t trace_user_stack_print(struct trace_iterator *iter,
return TRACE_TYPE_PARTIAL_LINE;
}
-static struct trace_event trace_user_stack_event = {
- .type = TRACE_USER_STACK,
+static struct trace_event_functions trace_user_stack_funcs = {
.trace = trace_user_stack_print,
.raw = trace_special_print,
.hex = trace_special_hex,
.binary = trace_special_bin,
};
+static struct trace_event trace_user_stack_event = {
+ .type = TRACE_USER_STACK,
+ .funcs = &trace_user_stack_funcs,
+};
+
/* TRACE_BPRINT */
static enum print_line_t
-trace_bprint_print(struct trace_iterator *iter, int flags)
+trace_bprint_print(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_entry *entry = iter->ent;
struct trace_seq *s = &iter->seq;
@@ -1178,7 +1216,8 @@ trace_bprint_print(struct trace_iterator *iter, int flags)
static enum print_line_t
-trace_bprint_raw(struct trace_iterator *iter, int flags)
+trace_bprint_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct bprint_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1197,16 +1236,19 @@ trace_bprint_raw(struct trace_iterator *iter, int flags)
return TRACE_TYPE_PARTIAL_LINE;
}
+static struct trace_event_functions trace_bprint_funcs = {
+ .trace = trace_bprint_print,
+ .raw = trace_bprint_raw,
+};
static struct trace_event trace_bprint_event = {
.type = TRACE_BPRINT,
- .trace = trace_bprint_print,
- .raw = trace_bprint_raw,
+ .funcs = &trace_bprint_funcs,
};
/* TRACE_PRINT */
static enum print_line_t trace_print_print(struct trace_iterator *iter,
- int flags)
+ int flags, struct trace_event *event)
{
struct print_entry *field;
struct trace_seq *s = &iter->seq;
@@ -1225,7 +1267,8 @@ static enum print_line_t trace_print_print(struct trace_iterator *iter,
return TRACE_TYPE_PARTIAL_LINE;
}
-static enum print_line_t trace_print_raw(struct trace_iterator *iter, int flags)
+static enum print_line_t trace_print_raw(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct print_entry *field;
@@ -1240,12 +1283,16 @@ static enum print_line_t trace_print_raw(struct trace_iterator *iter, int flags)
return TRACE_TYPE_PARTIAL_LINE;
}
-static struct trace_event trace_print_event = {
- .type = TRACE_PRINT,
+static struct trace_event_functions trace_print_funcs = {
.trace = trace_print_print,
.raw = trace_print_raw,
};
+static struct trace_event trace_print_event = {
+ .type = TRACE_PRINT,
+ .funcs = &trace_print_funcs,
+};
+
static struct trace_event *events[] __initdata = {
&trace_fn_event,
diff --git a/kernel/trace/trace_output.h b/kernel/trace/trace_output.h
index 9d91c72..c038eba 100644
--- a/kernel/trace/trace_output.h
+++ b/kernel/trace/trace_output.h
@@ -25,7 +25,7 @@ extern void trace_event_read_unlock(void);
extern struct trace_event *ftrace_find_event(int type);
extern enum print_line_t trace_nop_print(struct trace_iterator *iter,
- int flags);
+ int flags, struct trace_event *event);
extern int
trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 5887a57..6c98162 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -93,7 +93,8 @@ static struct syscall_metadata *syscall_nr_to_meta(int nr)
}
enum print_line_t
-print_syscall_enter(struct trace_iterator *iter, int flags)
+print_syscall_enter(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct trace_entry *ent = iter->ent;
@@ -145,7 +146,8 @@ end:
}
enum print_line_t
-print_syscall_exit(struct trace_iterator *iter, int flags)
+print_syscall_exit(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
{
struct trace_seq *s = &iter->seq;
struct trace_entry *ent = iter->ent;
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 7/9 - v2][RFC] tracing: Move print functions into event class
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (5 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 6/9 - v2][RFC] tracing: Allow events to share their print functions Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 8/9 - v2][RFC] tracing: Remove duplicate id information in event structure Steven Rostedt
2010-05-04 3:40 ` [PATCH 9/9 - v2][RFC] tracing: Combine event filter_active and enable into single flags field Steven Rostedt
8 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
[-- Attachment #1: 0007-tracing-Move-print-functions-into-event-class.patch --]
[-- Type: text/plain, Size: 11790 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Currently, every event has its own trace_event structure. This is
fine since the structure is needed anyway. But the print function
structure (trace_event_functions) is now separate. Since the output
of the trace event is done by the class (with the exception of events
defined by DEFINE_EVENT_PRINT), it makes sense to have the class
define the print functions that all events in the class can use.
This makes a bigger deal with the syscall events since all syscall events
use the same class. The savings here is another 37K.
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5774574 1293204 9351592 16419370 fa8a2a vmlinux.init
5761154 1268356 9351592 16381102 f9f4ae vmlinux.print
To accomplish this, and to let the class know what event is being
printed, the event structure is embedded in the ftrace_event_call
structure. This should not be an issues since the event structure
was created for each event anyway.
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 2 +-
include/linux/syscalls.h | 18 +++------------
include/trace/ftrace.h | 47 +++++++++++++++++-----------------------
kernel/trace/trace_events.c | 6 ++--
kernel/trace/trace_kprobe.c | 14 +++++-------
kernel/trace/trace_syscalls.c | 8 +++++++
6 files changed, 42 insertions(+), 53 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 4f77932..b1a007d 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -148,7 +148,7 @@ struct ftrace_event_call {
struct ftrace_event_class *class;
char *name;
struct dentry *dir;
- struct trace_event *event;
+ struct trace_event event;
int enabled;
int id;
const char *print_fmt;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f725677..a1a86a5 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -120,24 +120,20 @@ struct perf_event_attr;
extern struct ftrace_event_class event_class_syscall_enter;
extern struct ftrace_event_class event_class_syscall_exit;
+extern struct trace_event_functions enter_syscall_print_funcs;
+extern struct trace_event_functions exit_syscall_print_funcs;
#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_enter_##sname; \
- static struct trace_event_functions enter_syscall_print_funcs_##sname = { \
- .trace = print_syscall_enter, \
- }; \
- static struct trace_event enter_syscall_print_##sname = { \
- .funcs = &enter_syscall_print_funcs_##sname, \
- }; \
static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
event_enter_##sname = { \
.name = "sys_enter"#sname, \
.class = &event_class_syscall_enter, \
- .event = &enter_syscall_print_##sname, \
+ .event.funcs = &enter_syscall_print_funcs, \
.data = (void *)&__syscall_meta_##sname,\
}
@@ -145,19 +141,13 @@ extern struct ftrace_event_class event_class_syscall_exit;
static struct syscall_metadata __syscall_meta_##sname; \
static struct ftrace_event_call \
__attribute__((__aligned__(4))) event_exit_##sname; \
- static struct trace_event_functions exit_syscall_print_funcs_##sname = { \
- .trace = print_syscall_exit, \
- }; \
- static struct trace_event exit_syscall_print_##sname = { \
- .funcs = &exit_syscall_print_funcs_##sname, \
- }; \
static struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
event_exit_##sname = { \
.name = "sys_exit"#sname, \
.class = &event_class_syscall_exit, \
- .event = &exit_syscall_print_##sname, \
+ .event.funcs = &exit_syscall_print_funcs, \
.data = (void *)&__syscall_meta_##sname,\
}
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 78650f7..9765454 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -206,18 +206,22 @@
#undef DECLARE_EVENT_CLASS
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print) \
static notrace enum print_line_t \
-ftrace_raw_output_id_##call(int event_id, const char *name, \
- struct trace_iterator *iter, int flags) \
+ftrace_raw_output_##call(struct trace_iterator *iter, int flags, \
+ struct trace_event *trace_event) \
{ \
+ struct ftrace_event_call *event; \
struct trace_seq *s = &iter->seq; \
struct ftrace_raw_##call *field; \
struct trace_entry *entry; \
struct trace_seq *p; \
int ret; \
\
+ event = container_of(trace_event, struct ftrace_event_call, \
+ event); \
+ \
entry = iter->ent; \
\
- if (entry->type != event_id) { \
+ if (entry->type != event->id) { \
WARN_ON_ONCE(1); \
return TRACE_TYPE_UNHANDLED; \
} \
@@ -226,7 +230,7 @@ ftrace_raw_output_id_##call(int event_id, const char *name, \
\
p = &get_cpu_var(ftrace_event_seq); \
trace_seq_init(p); \
- ret = trace_seq_printf(s, "%s: ", name); \
+ ret = trace_seq_printf(s, "%s: ", event->name); \
if (ret) \
ret = trace_seq_printf(s, print); \
put_cpu(); \
@@ -234,17 +238,10 @@ ftrace_raw_output_id_##call(int event_id, const char *name, \
return TRACE_TYPE_PARTIAL_LINE; \
\
return TRACE_TYPE_HANDLED; \
-}
-
-#undef DEFINE_EVENT
-#define DEFINE_EVENT(template, name, proto, args) \
-static notrace enum print_line_t \
-ftrace_raw_output_##name(struct trace_iterator *iter, int flags, \
- struct trace_event *event) \
-{ \
- return ftrace_raw_output_id_##template(event_##name.id, \
- #name, iter, flags); \
-}
+} \
+static struct trace_event_functions ftrace_event_type_funcs_##call = { \
+ .trace = ftrace_raw_output_##call, \
+};
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, call, proto, args, print) \
@@ -277,7 +274,10 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags, \
return TRACE_TYPE_PARTIAL_LINE; \
\
return TRACE_TYPE_HANDLED; \
-}
+} \
+static struct trace_event_functions ftrace_event_type_funcs_##call = { \
+ .trace = ftrace_raw_output_##call, \
+};
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
@@ -527,17 +527,10 @@ ftrace_raw_event_##call(proto, void *__data) \
}
#undef DEFINE_EVENT
-#define DEFINE_EVENT(template, call, proto, args) \
-static struct trace_event_functions ftrace_event_type_funcs_##call = { \
- .trace = ftrace_raw_output_##call, \
-}; \
-static struct trace_event ftrace_event_type_##call = { \
- .funcs = &ftrace_event_type_funcs_##call, \
-};
+#define DEFINE_EVENT(template, call, proto, args)
#undef DEFINE_EVENT_PRINT
-#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
- DEFINE_EVENT(template, name, PARAMS(proto), PARAMS(args))
+#define DEFINE_EVENT_PRINT(template, name, proto, args, print)
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
@@ -573,7 +566,7 @@ __attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
- .event = &ftrace_event_type_##call, \
+ .event.funcs = &ftrace_event_type_funcs_##template, \
.print_fmt = print_fmt_##template, \
}
@@ -587,7 +580,7 @@ __attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
- .event = &ftrace_event_type_##call, \
+ .event.funcs = &ftrace_event_type_funcs_##call, \
.print_fmt = print_fmt_##call, \
}
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 7cfd91b..82ee590 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -122,7 +122,7 @@ int trace_event_raw_init(struct ftrace_event_call *call)
{
int id;
- id = register_ftrace_event(call->event);
+ id = register_ftrace_event(&call->event);
if (!id)
return -ENODEV;
call->id = id;
@@ -1071,8 +1071,8 @@ static void remove_subsystem_dir(const char *name)
static void __trace_remove_event_call(struct ftrace_event_call *call)
{
ftrace_event_enable_disable(call, 0);
- if (call->event)
- __unregister_ftrace_event(call->event);
+ if (call->event.funcs)
+ __unregister_ftrace_event(&call->event);
debugfs_remove_recursive(call->dir);
list_del(&call->list);
trace_destroy_fields(call);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b989ae2..d8061c3 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -204,7 +204,6 @@ struct trace_probe {
const char *symbol; /* symbol name */
struct ftrace_event_class class;
struct ftrace_event_call call;
- struct trace_event event;
unsigned int nr_args;
struct probe_arg args[];
};
@@ -1020,7 +1019,7 @@ print_kprobe_event(struct trace_iterator *iter, int flags,
int i;
field = (struct kprobe_trace_entry *)iter->ent;
- tp = container_of(event, struct trace_probe, event);
+ tp = container_of(event, struct trace_probe, call.event);
if (!trace_seq_printf(s, "%s: (", tp->call.name))
goto partial;
@@ -1054,7 +1053,7 @@ print_kretprobe_event(struct trace_iterator *iter, int flags,
int i;
field = (struct kretprobe_trace_entry *)iter->ent;
- tp = container_of(event, struct trace_probe, event);
+ tp = container_of(event, struct trace_probe, call.event);
if (!trace_seq_printf(s, "%s: (", tp->call.name))
goto partial;
@@ -1364,20 +1363,19 @@ static int register_probe_event(struct trace_probe *tp)
/* Initialize ftrace_event_call */
if (probe_is_return(tp)) {
- tp->event.funcs = &kretprobe_funcs;
INIT_LIST_HEAD(&call->class->fields);
+ call->event.funcs = &kretprobe_funcs;
call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kretprobe_event_define_fields;
} else {
INIT_LIST_HEAD(&call->class->fields);
- tp->event.funcs = &kprobe_funcs;
+ call->event.funcs = &kprobe_funcs;
call->class->raw_init = probe_event_raw_init;
call->class->define_fields = kprobe_event_define_fields;
}
if (set_print_fmt(tp) < 0)
return -ENOMEM;
- call->event = &tp->event;
- call->id = register_ftrace_event(&tp->event);
+ call->id = register_ftrace_event(&call->event);
if (!call->id) {
kfree(call->print_fmt);
return -ENODEV;
@@ -1389,7 +1387,7 @@ static int register_probe_event(struct trace_probe *tp)
if (ret) {
pr_info("Failed to register kprobe event: %s\n", call->name);
kfree(call->print_fmt);
- unregister_ftrace_event(&tp->event);
+ unregister_ftrace_event(&call->event);
}
return ret;
}
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 6c98162..d8c076b 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -39,6 +39,14 @@ syscall_get_exit_fields(struct ftrace_event_call *call)
return &entry->exit_fields;
}
+struct trace_event_functions enter_syscall_print_funcs = {
+ .trace = print_syscall_enter,
+};
+
+struct trace_event_functions exit_syscall_print_funcs = {
+ .trace = print_syscall_exit,
+};
+
struct ftrace_event_class event_class_syscall_enter = {
.system = "syscalls",
.reg = syscall_enter_register,
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 8/9 - v2][RFC] tracing: Remove duplicate id information in event structure
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (6 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 7/9 - v2][RFC] tracing: Move print functions into event class Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 9/9 - v2][RFC] tracing: Combine event filter_active and enable into single flags field Steven Rostedt
8 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[-- Attachment #1: 0008-tracing-Remove-duplicate-id-information-in-event-str.patch --]
[-- Type: text/plain, Size: 10958 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Now that the trace_event structure is embedded in the ftrace_event_call
structure, there is no need for the ftrace_event_call id field.
The id field is the same as the trace_event type field.
Removing the id and re-arranging the structure brings down the tracepoint
footprint by another 5K.
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5761154 1268356 9351592 16381102 f9f4ae vmlinux.print
5761074 1262596 9351592 16375262 f9ddde vmlinux.id
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 5 ++---
include/trace/ftrace.h | 12 ++++++------
kernel/trace/trace_event_perf.c | 4 ++--
kernel/trace/trace_events.c | 7 +++----
kernel/trace/trace_events_filter.c | 2 +-
kernel/trace/trace_export.c | 4 ++--
kernel/trace/trace_kprobe.c | 18 ++++++++++--------
kernel/trace/trace_syscalls.c | 14 ++++++++------
8 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index b1a007d..0be0285 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -149,14 +149,13 @@ struct ftrace_event_call {
char *name;
struct dentry *dir;
struct trace_event event;
- int enabled;
- int id;
const char *print_fmt;
- int filter_active;
struct event_filter *filter;
void *mod;
void *data;
+ int enabled;
+ int filter_active;
int perf_refcount;
};
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 9765454..db839d9 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -150,7 +150,7 @@
*
* entry = iter->ent;
*
- * if (entry->type != event_<call>.id) {
+ * if (entry->type != event_<call>->event.type) {
* WARN_ON_ONCE(1);
* return TRACE_TYPE_UNHANDLED;
* }
@@ -221,7 +221,7 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags, \
\
entry = iter->ent; \
\
- if (entry->type != event->id) { \
+ if (entry->type != event->event.type) { \
WARN_ON_ONCE(1); \
return TRACE_TYPE_UNHANDLED; \
} \
@@ -257,7 +257,7 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags, \
\
entry = iter->ent; \
\
- if (entry->type != event_##call.id) { \
+ if (entry->type != event_##call.event.type) { \
WARN_ON_ONCE(1); \
return TRACE_TYPE_UNHANDLED; \
} \
@@ -409,7 +409,7 @@ static inline notrace int ftrace_get_offsets_##call( \
* __data_size = ftrace_get_offsets_<call>(&__data_offsets, args);
*
* event = trace_current_buffer_lock_reserve(&buffer,
- * event_<call>.id,
+ * event_<call>->event.type,
* sizeof(*entry) + __data_size,
* irq_flags, pc);
* if (!event)
@@ -510,7 +510,7 @@ ftrace_raw_event_##call(proto, void *__data) \
__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
\
event = trace_current_buffer_lock_reserve(&buffer, \
- event_call->id, \
+ event_call->event.type, \
sizeof(*entry) + __data_size, \
irq_flags, pc); \
if (!event) \
@@ -701,7 +701,7 @@ perf_trace_##call(proto, struct ftrace_event_call *event_call) \
"profile buffer not large enough")) \
return; \
entry = (struct ftrace_raw_##call *)perf_trace_buf_prepare( \
- __entry_size, event_call->id, &rctx, &irq_flags); \
+ __entry_size, event_call->event.type, &rctx, &irq_flags); \
if (!entry) \
return; \
tstruct \
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 196fe9d..0a47e8d 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -80,7 +80,7 @@ int perf_trace_enable(int event_id)
mutex_lock(&event_mutex);
list_for_each_entry(event, &ftrace_events, list) {
- if (event->id == event_id &&
+ if (event->event.type == event_id &&
event->class && event->class->perf_probe &&
try_module_get(event->mod)) {
ret = perf_trace_event_enable(event);
@@ -128,7 +128,7 @@ void perf_trace_disable(int event_id)
mutex_lock(&event_mutex);
list_for_each_entry(event, &ftrace_events, list) {
- if (event->id == event_id) {
+ if (event->event.type == event_id) {
perf_trace_event_disable(event);
module_put(event->mod);
break;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 82ee590..8a2b370 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -125,7 +125,6 @@ int trace_event_raw_init(struct ftrace_event_call *call)
id = register_ftrace_event(&call->event);
if (!id)
return -ENODEV;
- call->id = id;
return 0;
}
@@ -567,7 +566,7 @@ event_format_read(struct file *filp, char __user *ubuf, size_t cnt,
trace_seq_init(s);
trace_seq_printf(s, "name: %s\n", call->name);
- trace_seq_printf(s, "ID: %d\n", call->id);
+ trace_seq_printf(s, "ID: %d\n", call->event.type);
trace_seq_printf(s, "format:\n");
head = trace_get_fields(call);
@@ -641,7 +640,7 @@ event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
return -ENOMEM;
trace_seq_init(s);
- trace_seq_printf(s, "%d\n", call->id);
+ trace_seq_printf(s, "%d\n", call->event.type);
r = simple_read_from_buffer(ubuf, cnt, ppos,
s->buffer, s->len);
@@ -968,7 +967,7 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
trace_create_file("enable", 0644, call->dir, call,
enable);
- if (call->id && (call->class->perf_probe || call->class->reg))
+ if (call->event.type && (call->class->perf_probe || call->class->reg))
trace_create_file("id", 0444, call->dir, call,
id);
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 961f99b..2702d6b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1395,7 +1395,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
mutex_lock(&event_mutex);
list_for_each_entry(call, &ftrace_events, list) {
- if (call->id == event_id)
+ if (call->event.type == event_id)
break;
}
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index e878d06..8536e2a 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -153,7 +153,7 @@ static int ftrace_raw_init_event(struct ftrace_event_call *call)
#define F_printk(fmt, args...) #fmt ", " __stringify(args)
#undef FTRACE_ENTRY
-#define FTRACE_ENTRY(call, struct_name, type, tstruct, print) \
+#define FTRACE_ENTRY(call, struct_name, etype, tstruct, print) \
\
struct ftrace_event_class event_class_ftrace_##call = { \
.system = __stringify(TRACE_SYSTEM), \
@@ -165,7 +165,7 @@ struct ftrace_event_call __used \
__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
- .id = type, \
+ .event.type = etype, \
.class = &event_class_ftrace_##call, \
.print_fmt = print, \
}; \
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d8061c3..934078b 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -960,8 +960,8 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
- event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
- irq_flags, pc);
+ event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
+ size, irq_flags, pc);
if (!event)
return;
@@ -992,8 +992,8 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
- event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
- irq_flags, pc);
+ event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
+ size, irq_flags, pc);
if (!event)
return;
@@ -1228,7 +1228,8 @@ static __kprobes void kprobe_perf_func(struct kprobe *kp,
"profile buffer not large enough"))
return;
- entry = perf_trace_buf_prepare(size, call->id, &rctx, &irq_flags);
+ entry = perf_trace_buf_prepare(size, call->event.type,
+ &rctx, &irq_flags);
if (!entry)
return;
@@ -1258,7 +1259,8 @@ static __kprobes void kretprobe_perf_func(struct kretprobe_instance *ri,
"profile buffer not large enough"))
return;
- entry = perf_trace_buf_prepare(size, call->id, &rctx, &irq_flags);
+ entry = perf_trace_buf_prepare(size, call->event.type,
+ &rctx, &irq_flags);
if (!entry)
return;
@@ -1375,8 +1377,8 @@ static int register_probe_event(struct trace_probe *tp)
}
if (set_print_fmt(tp) < 0)
return -ENOMEM;
- call->id = register_ftrace_event(&call->event);
- if (!call->id) {
+ ret = register_ftrace_event(&call->event);
+ if (!ret) {
kfree(call->print_fmt);
return -ENODEV;
}
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index d8c076b..59b68c1 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -117,7 +117,7 @@ print_syscall_enter(struct trace_iterator *iter, int flags,
if (!entry)
goto end;
- if (entry->enter_event->id != ent->type) {
+ if (entry->enter_event->event.type != ent->type) {
WARN_ON_ONCE(1);
goto end;
}
@@ -173,7 +173,7 @@ print_syscall_exit(struct trace_iterator *iter, int flags,
return TRACE_TYPE_HANDLED;
}
- if (entry->exit_event->id != ent->type) {
+ if (entry->exit_event->event.type != ent->type) {
WARN_ON_ONCE(1);
return TRACE_TYPE_UNHANDLED;
}
@@ -315,7 +315,7 @@ void ftrace_syscall_enter(struct pt_regs *regs, long id)
size = sizeof(*entry) + sizeof(unsigned long) * sys_data->nb_args;
event = trace_current_buffer_lock_reserve(&buffer,
- sys_data->enter_event->id, size, 0, 0);
+ sys_data->enter_event->event.type, size, 0, 0);
if (!event)
return;
@@ -347,7 +347,7 @@ void ftrace_syscall_exit(struct pt_regs *regs, long ret)
return;
event = trace_current_buffer_lock_reserve(&buffer,
- sys_data->exit_event->id, sizeof(*entry), 0, 0);
+ sys_data->exit_event->event.type, sizeof(*entry), 0, 0);
if (!event)
return;
@@ -511,7 +511,8 @@ static void perf_syscall_enter(struct pt_regs *regs, long id)
return;
rec = (struct syscall_trace_enter *)perf_trace_buf_prepare(size,
- sys_data->enter_event->id, &rctx, &flags);
+ sys_data->enter_event->event.type,
+ &rctx, &flags);
if (!rec)
return;
@@ -586,7 +587,8 @@ static void perf_syscall_exit(struct pt_regs *regs, long ret)
return;
rec = (struct syscall_trace_exit *)perf_trace_buf_prepare(size,
- sys_data->exit_event->id, &rctx, &flags);
+ sys_data->exit_event->event.type,
+ &rctx, &flags);
if (!rec)
return;
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 9/9 - v2][RFC] tracing: Combine event filter_active and enable into single flags field
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
` (7 preceding siblings ...)
2010-05-04 3:40 ` [PATCH 8/9 - v2][RFC] tracing: Remove duplicate id information in event structure Steven Rostedt
@ 2010-05-04 3:40 ` Steven Rostedt
8 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-04 3:40 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
Frederic Weisbecker, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[-- Attachment #1: 0009-tracing-Combine-event-filter_active-and-enable-into-.patch --]
[-- Type: text/plain, Size: 6236 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
The filter_active and enable both use an int (4 bytes each) to
set a single flag. We can save 4 bytes per event by combining the
two into a single integer.
text data bss dec hex filename
5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
5761074 1262596 9351592 16375262 f9ddde vmlinux.id
5761007 1256916 9351592 16369515 f9c76b vmlinux.flags
This gives us another 5K in savings.
The modification of both the enable and filter fields are done
under the event_mutex, so it is still safe to combine the two.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace_event.h | 21 +++++++++++++++++++--
kernel/trace/trace.h | 2 +-
kernel/trace/trace_events.c | 14 +++++++-------
kernel/trace/trace_events_filter.c | 10 +++++-----
kernel/trace/trace_kprobe.c | 2 +-
5 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 0be0285..5ac97a4 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -143,6 +143,16 @@ struct ftrace_event_class {
int (*raw_init)(struct ftrace_event_call *);
};
+enum {
+ TRACE_EVENT_FL_ENABLED_BIT,
+ TRACE_EVENT_FL_FILTERED_BIT,
+};
+
+enum {
+ TRACE_EVENT_FL_ENABLED = (1 << TRACE_EVENT_FL_ENABLED_BIT),
+ TRACE_EVENT_FL_FILTERED = (1 << TRACE_EVENT_FL_FILTERED_BIT),
+};
+
struct ftrace_event_call {
struct list_head list;
struct ftrace_event_class *class;
@@ -154,8 +164,15 @@ struct ftrace_event_call {
void *mod;
void *data;
- int enabled;
- int filter_active;
+ /*
+ * 32 bit flags:
+ * bit 1: enabled
+ * bit 2: filter_active
+ *
+ * Must hold event_mutex to change.
+ */
+ unsigned int flags;
+
int perf_refcount;
};
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index c88c563..6356259 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -802,7 +802,7 @@ filter_check_discard(struct ftrace_event_call *call, void *rec,
struct ring_buffer *buffer,
struct ring_buffer_event *event)
{
- if (unlikely(call->filter_active) &&
+ if (unlikely(call->flags & TRACE_EVENT_FL_FILTERED) &&
!filter_match_preds(call->filter, rec)) {
ring_buffer_discard_commit(buffer, event);
return 1;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 8a2b370..2593365 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -137,8 +137,8 @@ static int ftrace_event_enable_disable(struct ftrace_event_call *call,
switch (enable) {
case 0:
- if (call->enabled) {
- call->enabled = 0;
+ if (call->flags & TRACE_EVENT_FL_ENABLED) {
+ call->flags &= ~TRACE_EVENT_FL_ENABLED;
tracing_stop_cmdline_record();
if (call->class->reg)
call->class->reg(call, TRACE_REG_UNREGISTER);
@@ -149,7 +149,7 @@ static int ftrace_event_enable_disable(struct ftrace_event_call *call,
}
break;
case 1:
- if (!call->enabled) {
+ if (!(call->flags & TRACE_EVENT_FL_ENABLED)) {
tracing_start_cmdline_record();
if (call->class->reg)
ret = call->class->reg(call, TRACE_REG_REGISTER);
@@ -163,7 +163,7 @@ static int ftrace_event_enable_disable(struct ftrace_event_call *call,
"%s\n", call->name);
break;
}
- call->enabled = 1;
+ call->flags |= TRACE_EVENT_FL_ENABLED;
}
break;
}
@@ -352,7 +352,7 @@ s_next(struct seq_file *m, void *v, loff_t *pos)
(*pos)++;
list_for_each_entry_continue(call, &ftrace_events, list) {
- if (call->enabled)
+ if (call->flags & TRACE_EVENT_FL_ENABLED)
return call;
}
@@ -411,7 +411,7 @@ event_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
struct ftrace_event_call *call = filp->private_data;
char *buf;
- if (call->enabled)
+ if (call->flags & TRACE_EVENT_FL_ENABLED)
buf = "1\n";
else
buf = "0\n";
@@ -486,7 +486,7 @@ system_enable_read(struct file *filp, char __user *ubuf, size_t cnt,
* or if all events or cleared, or if we have
* a mixture.
*/
- set |= (1 << !!call->enabled);
+ set |= (1 << !!(call->flags & TRACE_EVENT_FL_ENABLED));
/*
* If we have a mixture, no need to look further.
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 2702d6b..239ea5d 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -547,7 +547,7 @@ static void filter_disable_preds(struct ftrace_event_call *call)
struct event_filter *filter = call->filter;
int i;
- call->filter_active = 0;
+ call->flags &= ~TRACE_EVENT_FL_FILTERED;
filter->n_preds = 0;
for (i = 0; i < MAX_FILTER_PRED; i++)
@@ -574,7 +574,7 @@ void destroy_preds(struct ftrace_event_call *call)
{
__free_preds(call->filter);
call->filter = NULL;
- call->filter_active = 0;
+ call->flags &= ~TRACE_EVENT_FL_FILTERED;
}
static struct event_filter *__alloc_preds(void)
@@ -613,7 +613,7 @@ static int init_preds(struct ftrace_event_call *call)
if (call->filter)
return 0;
- call->filter_active = 0;
+ call->flags &= ~TRACE_EVENT_FL_FILTERED;
call->filter = __alloc_preds();
if (IS_ERR(call->filter))
return PTR_ERR(call->filter);
@@ -1268,7 +1268,7 @@ static int replace_system_preds(struct event_subsystem *system,
if (err)
filter_disable_preds(call);
else {
- call->filter_active = 1;
+ call->flags |= TRACE_EVENT_FL_FILTERED;
replace_filter_string(filter, filter_string);
}
fail = false;
@@ -1317,7 +1317,7 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string)
if (err)
append_filter_err(ps, call->filter);
else
- call->filter_active = 1;
+ call->flags |= TRACE_EVENT_FL_FILTERED;
out:
filter_opstack_clear(ps);
postfix_clear(ps);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 934078b..0e3ded6 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1382,7 +1382,7 @@ static int register_probe_event(struct trace_probe *tp)
kfree(call->print_fmt);
return -ENODEV;
}
- call->enabled = 0;
+ call->flags = 0;
call->class->reg = kprobe_register;
call->data = tp;
ret = trace_add_event_call(call);
--
1.7.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks
2010-05-04 3:40 ` [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks Steven Rostedt
@ 2010-05-07 3:52 ` Frederic Weisbecker
2010-05-07 14:09 ` Steven Rostedt
0 siblings, 1 reply; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 3:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
On Mon, May 03, 2010 at 11:40:47PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> This patch allows data to be passed to the tracepoint callbacks
> if the tracepoint was created to do so.
>
> The DECLARE_TRACE() now adds two new functions:
>
> register_trace_mytracepoint_data()
> unregister_trace_mytracepoint_data()
>
> These two are the same as the original
>
> register_trace_mytracepoint()
> unregister_trace_mytracepoint()
>
> But now allow you to pass a private data pointer that will
> be passed to the callback handle. For example:
>
> DECLARE_TRACE(mytracepoint, int value, value);
>
> will create a function called trace_mytracepoint()
>
> void trace_mytracepoint(int value);
>
> If the user wants to pass data to register a function to this tracepoint
> and have data also passed to this callback, they can use:
>
> int mycallback(int value, void *data);
>
> register_trace_mytracepoint_data(mycallback, mydata);
>
> Then the mycallback() will receive the "mydata" as the parameter after
> the args.
>
> A more detailed example:
>
> DECLARE_TRACE(mytracepoint, TP_PROTO(int status), TP_ARGS(status));
>
> /* In the C file */
>
> DEFINE_TRACE(mytracepoint, TP_PROTO(int status), TP_ARGS(status));
>
> [...]
>
> trace_mytacepoint(status);
>
> /* In a file registering this tracepoint */
>
> int my_callback(int status, void *data)
> {
> struct my_struct my_data = data;
> [...]
> }
>
> [...]
> my_data = kmalloc(sizeof(*my_data), GFP_KERNEL);
> init_my_data(my_data);
> register_trace_mytracepoint_data(my_callback, my_data);
>
> The same callback can also be registered to the same tracepoint as long
> as the data registered is the different. Note, the data must also be used
> to unregister the callback:
>
> unregister_trace_mytracepoint_data(my_callback, my_data);
>
> Because of the data parameter, tracepoints declared this way can not have
> no args. That is:
>
> DECLARE_TRACE(mytracepoint, TP_PROTO(void), TP_ARGS());
>
> will cause an error.
>
> If no arguments are needed, a new macro can be used instead:
>
> DECLARE_TRACE_NOARGS(mytracepoint);
>
> Since there are no arguments, the proto and args fields are left out.
>
> This is part of a series to make the tracepoint footprint smaller:
>
> text data bss dec hex filename
> 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
> 5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
> 5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint
>
> Again, this patch also increases the size of the kernel, but
> lays the ground work for decreasing it.
>
> v2: Made the DECLARE_TRACE() have the ability to pass arguments
> and added a new DECLARE_TRACE_NOARGS() for tracepoints that
> do not need any arguments.
>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> include/linux/tracepoint.h | 94 +++++++++++++++++++++++++-------
> kernel/tracepoint.c | 91 +++++++++++++++++--------------
> samples/tracepoints/tp-samples-trace.h | 4 +-
> 3 files changed, 126 insertions(+), 63 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 78b4bd3..ee8059a 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -20,12 +20,17 @@
> struct module;
> struct tracepoint;
>
> +struct tracepoint_func {
> + void *func;
> + void *data;
> +};
> +
> struct tracepoint {
> const char *name; /* Tracepoint name */
> int state; /* State. */
> void (*regfunc)(void);
> void (*unregfunc)(void);
> - void **funcs;
> + struct tracepoint_func *funcs;
> } __attribute__((aligned(32))); /*
> * Aligned on 32 bytes because it is
> * globally visible and gcc happily
> @@ -46,14 +51,18 @@ struct tracepoint {
> */
> #define __DO_TRACE(tp, proto, args) \
> do { \
> - void **it_func; \
> + struct tracepoint_func *it_func_ptr; \
> + void *it_func; \
> + void *__data; \
> \
> rcu_read_lock_sched_notrace(); \
> - it_func = rcu_dereference_sched((tp)->funcs); \
> - if (it_func) { \
> + it_func_ptr = rcu_dereference_sched((tp)->funcs); \
> + if (it_func_ptr) { \
> do { \
> - ((void(*)(proto))(*it_func))(args); \
> - } while (*(++it_func)); \
> + it_func = (it_func_ptr)->func; \
> + __data = (it_func_ptr)->data; \
> + ((void(*)(proto))(it_func))(args); \
So, we had a talk about this and we concluded that it is probably fine
on every archs to push one more argument than needed in a function.
But I think it would be nice to add a comment about this. Firstly
because this line breaks all the self-explanation of the code, I mean
I tried hard to find how the non-data callback case was handled :)
Secondly to also to avoid people asking what happens here.
> + } while ((++it_func_ptr)->func); \
> } \
> rcu_read_unlock_sched_notrace(); \
> } while (0)
> @@ -63,23 +72,47 @@ struct tracepoint {
> * not add unwanted padding between the beginning of the section and the
> * structure. Force alignment to the same alignment as the section start.
> */
> -#define DECLARE_TRACE(name, proto, args) \
> +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> extern struct tracepoint __tracepoint_##name; \
> static inline void trace_##name(proto) \
> { \
> if (unlikely(__tracepoint_##name.state)) \
> __DO_TRACE(&__tracepoint_##name, \
> - TP_PROTO(proto), TP_ARGS(args)); \
> + TP_PROTO(data_proto), \
> + TP_ARGS(data_args)); \
> } \
> static inline int register_trace_##name(void (*probe)(proto)) \
> { \
> - return tracepoint_probe_register(#name, (void *)probe); \
> + return tracepoint_probe_register(#name, (void *)probe, \
> + NULL); \
> + } \
> + static inline int unregister_trace_##name(void (*probe)(proto)) \
> + { \
> + return tracepoint_probe_unregister(#name, (void *)probe,\
> + NULL); \
> } \
> - static inline int unregister_trace_##name(void (*probe)(proto)) \
> + static inline int \
> + register_trace_##name##_data(void (*probe)(data_proto), \
> + void *data) \
> { \
> - return tracepoint_probe_unregister(#name, (void *)probe);\
> + return tracepoint_probe_register(#name, (void *)probe, \
> + data); \
> + } \
> + static inline int \
> + unregister_trace_##name##_data(void (*probe)(data_proto), \
> + void *data) \
> + { \
> + return tracepoint_probe_unregister(#name, (void *)probe,\
> + data); \
> }
>
> +#define DECLARE_TRACE_NOARGS(name) \
> + __DECLARE_TRACE(name, void, , void *__data, __data)
That too, may be, deserves a small comment :)
> +
> +#define DECLARE_TRACE(name, proto, args) \
> + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> + PARAMS(proto, void *__data), \
> + PARAMS(args, __data))
>
> #define DEFINE_TRACE_FN(name, reg, unreg) \
> static const char __tpstrtab_##name[] \
> @@ -100,19 +133,37 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
> struct tracepoint *end);
>
> #else /* !CONFIG_TRACEPOINTS */
> -#define DECLARE_TRACE(name, proto, args) \
> - static inline void _do_trace_##name(struct tracepoint *tp, proto) \
> - { } \
> +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> static inline void trace_##name(proto) \
> - { } \
> + { \
> + } \
> static inline int register_trace_##name(void (*probe)(proto)) \
> { \
> return -ENOSYS; \
> } \
> - static inline int unregister_trace_##name(void (*probe)(proto)) \
> + static inline int unregister_trace_##name(void (*probe)(proto)) \
> + { \
> + return -ENOSYS; \
> + } \
> + static inline int \
> + register_trace_##name##_data(void (*probe)(data_proto), \
> + void *data) \
> + { \
> + return -ENOSYS; \
> + } \
> + static inline int \
> + unregister_trace_##name##_data(void (*probe)(data_proto), \
> + void *data) \
> { \
> return -ENOSYS; \
> }
> +#define DECLARE_TRACE_NOARGS(name) \
> + __DECLARE_TRACE(name, void, , void *__data, __data)
> +
> +#define DECLARE_TRACE(name, proto, args) \
> + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> + PARAMS(proto, void *__data), \
> + PARAMS(args, __data))
It seems that the on and off cases are exactly the same for DECLARE_TRACE*(),
you could provide a single version and let the __DECLARE_TRACE() do
the on/off trick.
Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-04 3:40 ` [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering Steven Rostedt
@ 2010-05-07 4:20 ` Frederic Weisbecker
2010-05-07 12:42 ` Steven Rostedt
2010-05-07 14:54 ` Mathieu Desnoyers
2010-05-07 8:20 ` Li Zefan
1 sibling, 2 replies; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 4:20 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> This patch removes the register functions of TRACE_EVENT() to enable
> and disable tracepoints. The registering of a event is now down
> directly in the trace_events.c file. The tracepoint_probe_register()
> is now called directly.
>
> The prototypes are no longer type checked, but this should not be
> an issue since the tracepoints are created automatically by the
> macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
> other macros will catch it.
Agreed. Typechecking matters for human code but not in this context.
Considering that the tracepoint and the probe are created by the same
CPP code, bugs will be tracked down quickly and located to a single
place.
>
> The trace_event_class structure now holds the probes to be called
> by the callbacks. This removes needing to have each event have
> a separate pointer for the probe.
>
> To handle kprobes and syscalls, since they register probes in a
> different manner, a "reg" field is added to the ftrace_event_class
> structure. If the "reg" field is assigned, then it will be called for
> enabling and disabling of the probe for either ftrace or perf. To let
> the reg function know what is happening, a new enum (trace_reg) is
> created that has the type of control that is needed.
>
> With this new rework, the 82 kernel events and 616 syscall events
> has their footprint dramatically lowered:
>
> text data bss dec hex filename
> 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
> 5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
> 5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint
> 5796926 1337748 9351592 16486266 fb8f7a vmlinux.data
> 5774316 1306580 9351592 16432488 fabd68 vmlinux.regs
>
> The size went from 16477030 to 16432488, that's a total of 44K
> in savings. With tracepoints being continuously added, this is
> critical that the footprint becomes minimal.
>
> v2: Changed the callback probes to pass void * and typecast the
> value within the function.
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Very nice!!
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/9 - v2][RFC] tracing: Create class struct for events
2010-05-04 3:40 ` [PATCH 1/9 - v2][RFC] tracing: Create class struct for events Steven Rostedt
@ 2010-05-07 4:21 ` Frederic Weisbecker
0 siblings, 0 replies; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 4:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
On Mon, May 03, 2010 at 11:40:46PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> This patch creates a ftrace_event_class struct that event structs point to.
> This class struct will be made to hold information to modify the
> events. Currently the class struct only holds the events system name.
>
> This patch slightly increases the size of the text as well as decreases
> the data size. The overall change is still a slight increase, but
> this change lays the ground work of other changes to make the footprint
> of tracepoints smaller.
>
> With 82 standard tracepoints, and 616 system call tracepoints:
>
> text data bss dec hex filename
> 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
> 5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
>
> This patch also cleans up some stale comments in ftrace.h.
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure
2010-05-04 3:40 ` [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure Steven Rostedt
@ 2010-05-07 4:49 ` Frederic Weisbecker
2010-05-07 12:57 ` Steven Rostedt
0 siblings, 1 reply; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 4:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers, Tom Zanussi
On Mon, May 03, 2010 at 11:40:49PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Move the defined fields from the event to the class structure.
> Since the fields of the event are defined by the class they belong
> to, it makes sense to have the class hold the information instead
> of the individual events. The events of the same class would just
> hold duplicate information.
>
> After this change the size of the kernel dropped another 8K:
>
> text data bss dec hex filename
> 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
> 5774316 1306580 9351592 16432488 fabd68 vmlinux.reg
> 5774503 1297492 9351592 16423587 fa9aa3 vmlinux.fields
>
> Although the text increased, this was mainly due to the C files
> having to adapt to the change. This is a constant increase, where
> new tracepoints will not increase the Text. But the big drop is
> in the data size (as well as needed allocations to hold the fields).
> This will give even more savings as more tracepoints are created.
>
> Note, if just TRACE_EVENT()s are used and not DECLARE_EVENT_CLASS()
> with several DEFINE_EVENT()s, then the savings will be lost. But
> we are pushing developers to consolidate events with DEFINE_EVENT()
> so this should not be an issue.
>
> The kprobes define a unique class to every new event, but are dynamic
> so it should not be a issue.
>
> The syscalls however have a single class but the fields for the individual
> events are different. The syscalls use a metadata to define the
> fields. I moved the fields list from the event to the metadata and
> added a "get_fields()" function to the class. This function is used
> to find the fields. For normal events and kprobes, get_fields() just
> returns a pointer to the fields list_head in the class. For syscall
> events, it returns the fields list_head in the metadata for the event.
>
> v2: Fixed the syscall fields. The syscall metadata needs a list
> of fields for both enter and exit.
>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Tom Zanussi <tzanussi@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Now I need to recover from my CPP headache before reviewing
this set further ;)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-04 3:40 ` [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering Steven Rostedt
2010-05-07 4:20 ` Frederic Weisbecker
@ 2010-05-07 8:20 ` Li Zefan
2010-05-07 12:59 ` Steven Rostedt
1 sibling, 1 reply; 29+ messages in thread
From: Li Zefan @ 2010-05-07 8:20 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Frederic Weisbecker, Arnaldo Carvalho de Melo,
Mathieu Desnoyers, Lai Jiangshan, Masami Hiramatsu,
Christoph Hellwig
> @@ -935,11 +947,11 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
> return -1;
> }
>
> - if (call->regfunc)
> + if (call->class->probe || call->class->reg)
> trace_create_file("enable", 0644, call->dir, call,
> enable);
>
> - if (call->id && call->perf_event_enable)
> + if (call->id && (call->class->perf_probe || call->class->reg))
> trace_create_file("id", 0444, call->dir, call,
> id);
>
Accessing of ->perf_probe needs to be guarded with CONFIG_PERF_EVENTS,
otherwise it won't pass compile.
The original code is fine, because ->perf_event_enable is always there
regardless of CONFIG_PERF_EVENTS.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 4:20 ` Frederic Weisbecker
@ 2010-05-07 12:42 ` Steven Rostedt
2010-05-07 14:54 ` Mathieu Desnoyers
1 sibling, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 12:42 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig
[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]
"Frederic Weisbecker" <fweisbec@gmail.com> wrote:
>On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
>> From: Steven Rostedt <srostedt@redhat.com>
>>
>> This patch removes the register functions of TRACE_EVENT() to enable
>> and disable tracepoints. The registering of a event is now down
>> directly in the trace_events.c file. The tracepoint_probe_register()
>> is now called directly.
>>
>> The prototypes are no longer type checked, but this should not be
>> an issue since the tracepoints are created automatically by the
>> macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
>> other macros will catch it.
>
>
>
>Agreed. Typechecking matters for human code but not in this context.
>Considering that the tracepoint and the probe are created by the same
>CPP code, bugs will be tracked down quickly and located to a single
>place.
>
>
>
>
>>
>> The trace_event_class structure now holds the probes to be called
>> by the callbacks. This removes needing to have each event have
>> a separate pointer for the probe.
>>
>> To handle kprobes and syscalls, since they register probes in a
>> different manner, a "reg" field is added to the ftrace_event_class
>> structure. If the "reg" field is assigned, then it will be called for
>> enabling and disabling of the probe for either ftrace or perf. To let
>> the reg function know what is happening, a new enum (trace_reg) is
>> created that has the type of control that is needed.
>>
>> With this new rework, the 82 kernel events and 616 syscall events
>> has their footprint dramatically lowered:
>>
>> text data bss dec hex filename
>> 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig
>> 5792282 1333796 9351592 16477670 fb6de6 vmlinux.class
>> 5793448 1333780 9351592 16478820 fb7264 vmlinux.tracepoint
>> 5796926 1337748 9351592 16486266 fb8f7a vmlinux.data
>> 5774316 1306580 9351592 16432488 fabd68 vmlinux.regs
>>
>> The size went from 16477030 to 16432488, that's a total of 44K
>> in savings. With tracepoints being continuously added, this is
>> critical that the footprint becomes minimal.
>>
>> v2: Changed the callback probes to pass void * and typecast the
>> value within the function.
>>
>> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
>
>
>
>Very nice!!
>
>Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
>
Thanks!
-- Steve
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure
2010-05-07 4:49 ` Frederic Weisbecker
@ 2010-05-07 12:57 ` Steven Rostedt
0 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 12:57 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers, Tom Zanussi
On Fri, 2010-05-07 at 06:49 +0200, Frederic Weisbecker wrote:
> On Mon, May 03, 2010 at 11:40:49PM -0400, Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@redhat.com>
>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
>
> Now I need to recover from my CPP headache before reviewing
> this set further ;)
>
I know how you feel. Actually writing CPP takes your brain away from the
ability to write documentation, hence my lack of it ;-)
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 8:20 ` Li Zefan
@ 2010-05-07 12:59 ` Steven Rostedt
0 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 12:59 UTC (permalink / raw)
To: Li Zefan
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Frederic Weisbecker, Arnaldo Carvalho de Melo,
Mathieu Desnoyers, Lai Jiangshan, Masami Hiramatsu,
Christoph Hellwig
On Fri, 2010-05-07 at 16:20 +0800, Li Zefan wrote:
> > @@ -935,11 +947,11 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
> > return -1;
> > }
> >
> > - if (call->regfunc)
> > + if (call->class->probe || call->class->reg)
> > trace_create_file("enable", 0644, call->dir, call,
> > enable);
> >
> > - if (call->id && call->perf_event_enable)
> > + if (call->id && (call->class->perf_probe || call->class->reg))
> > trace_create_file("id", 0444, call->dir, call,
> > id);
> >
>
> Accessing of ->perf_probe needs to be guarded with CONFIG_PERF_EVENTS,
> otherwise it won't pass compile.
>
> The original code is fine, because ->perf_event_enable is always there
> regardless of CONFIG_PERF_EVENTS.
Good catch! I wanted to test the !CONFIG_PERF_EVENTS but that needs to
be done on a non x86 box. I'll need to do that before posting my non-RFC
patch set.
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks
2010-05-07 3:52 ` Frederic Weisbecker
@ 2010-05-07 14:09 ` Steven Rostedt
2010-05-07 18:06 ` Frederic Weisbecker
0 siblings, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 14:09 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
Hmm, I replied originally from my Google phone, but I don't see it in
LKML. So apologies if this is duplicate.
On Fri, 2010-05-07 at 05:52 +0200, Frederic Weisbecker wrote:
> On Mon, May 03, 2010 at 11:40:47PM -0400, Steven Rostedt wrote:
> > #define __DO_TRACE(tp, proto, args) \
> > do { \
> > - void **it_func; \
> > + struct tracepoint_func *it_func_ptr; \
> > + void *it_func; \
> > + void *__data; \
> > \
> > rcu_read_lock_sched_notrace(); \
> > - it_func = rcu_dereference_sched((tp)->funcs); \
> > - if (it_func) { \
> > + it_func_ptr = rcu_dereference_sched((tp)->funcs); \
> > + if (it_func_ptr) { \
> > do { \
> > - ((void(*)(proto))(*it_func))(args); \
> > - } while (*(++it_func)); \
> > + it_func = (it_func_ptr)->func; \
> > + __data = (it_func_ptr)->data; \
> > + ((void(*)(proto))(it_func))(args); \
>
>
> So, we had a talk about this and we concluded that it is probably fine
> on every archs to push one more argument than needed in a function.
Yep, I'm hoping this is the case.
>
> But I think it would be nice to add a comment about this. Firstly
> because this line breaks all the self-explanation of the code, I mean
> I tried hard to find how the non-data callback case was handled :)
> Secondly to also to avoid people asking what happens here.
OK, I'll add documentation here. So much for my job security ;-)
>
>
>
>
> > + } while ((++it_func_ptr)->func); \
> > } \
> > rcu_read_unlock_sched_notrace(); \
> > } while (0)
> > @@ -63,23 +72,47 @@ struct tracepoint {
> > * not add unwanted padding between the beginning of the section and the
> > * structure. Force alignment to the same alignment as the section start.
> > */
> > -#define DECLARE_TRACE(name, proto, args) \
> > +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> > extern struct tracepoint __tracepoint_##name; \
> > static inline void trace_##name(proto) \
> > { \
> > if (unlikely(__tracepoint_##name.state)) \
> > __DO_TRACE(&__tracepoint_##name, \
> > - TP_PROTO(proto), TP_ARGS(args)); \
> > + TP_PROTO(data_proto), \
> > + TP_ARGS(data_args)); \
> > } \
> > static inline int register_trace_##name(void (*probe)(proto)) \
> > { \
> > - return tracepoint_probe_register(#name, (void *)probe); \
> > + return tracepoint_probe_register(#name, (void *)probe, \
> > + NULL); \
> > + } \
> > + static inline int unregister_trace_##name(void (*probe)(proto)) \
> > + { \
> > + return tracepoint_probe_unregister(#name, (void *)probe,\
> > + NULL); \
> > } \
> > - static inline int unregister_trace_##name(void (*probe)(proto)) \
> > + static inline int \
> > + register_trace_##name##_data(void (*probe)(data_proto), \
> > + void *data) \
> > { \
> > - return tracepoint_probe_unregister(#name, (void *)probe);\
> > + return tracepoint_probe_register(#name, (void *)probe, \
> > + data); \
> > + } \
> > + static inline int \
> > + unregister_trace_##name##_data(void (*probe)(data_proto), \
> > + void *data) \
> > + { \
> > + return tracepoint_probe_unregister(#name, (void *)probe,\
> > + data); \
> > }
> >
> > +#define DECLARE_TRACE_NOARGS(name) \
> > + __DECLARE_TRACE(name, void, , void *__data, __data)
>
>
>
> That too, may be, deserves a small comment :)
OK
>
>
>
> > +
> > +#define DECLARE_TRACE(name, proto, args) \
> > + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> > + PARAMS(proto, void *__data), \
> > + PARAMS(args, __data))
> >
> > #define DEFINE_TRACE_FN(name, reg, unreg) \
> > static const char __tpstrtab_##name[] \
> > @@ -100,19 +133,37 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
> > struct tracepoint *end);
> >
> > #else /* !CONFIG_TRACEPOINTS */
> > -#define DECLARE_TRACE(name, proto, args) \
> > - static inline void _do_trace_##name(struct tracepoint *tp, proto) \
> > - { } \
> > +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> > static inline void trace_##name(proto) \
> > - { } \
> > + { \
> > + } \
> > static inline int register_trace_##name(void (*probe)(proto)) \
> > { \
> > return -ENOSYS; \
> > } \
> > - static inline int unregister_trace_##name(void (*probe)(proto)) \
> > + static inline int unregister_trace_##name(void (*probe)(proto)) \
> > + { \
> > + return -ENOSYS; \
> > + } \
> > + static inline int \
> > + register_trace_##name##_data(void (*probe)(data_proto), \
> > + void *data) \
> > + { \
> > + return -ENOSYS; \
> > + } \
> > + static inline int \
> > + unregister_trace_##name##_data(void (*probe)(data_proto), \
> > + void *data) \
> > { \
> > return -ENOSYS; \
> > }
> > +#define DECLARE_TRACE_NOARGS(name) \
> > + __DECLARE_TRACE(name, void, , void *__data, __data)
> > +
> > +#define DECLARE_TRACE(name, proto, args) \
> > + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> > + PARAMS(proto, void *__data), \
> > + PARAMS(args, __data))
>
>
>
>
> It seems that the on and off cases are exactly the same for DECLARE_TRACE*(),
> you could provide a single version and let the __DECLARE_TRACE() do
> the on/off trick.
I don't know what you mean here. How would __DECLARE_TRACE() do what
both DECLARE_TRACE() and DECLARE_TRACE_NOARGS() do? It will fail the
compile if proto is "void".
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 4:20 ` Frederic Weisbecker
2010-05-07 12:42 ` Steven Rostedt
@ 2010-05-07 14:54 ` Mathieu Desnoyers
2010-05-07 15:12 ` Steven Rostedt
2010-05-07 18:01 ` Frederic Weisbecker
1 sibling, 2 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-05-07 14:54 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
* Frederic Weisbecker (fweisbec@gmail.com) wrote:
> On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@redhat.com>
> >
> > This patch removes the register functions of TRACE_EVENT() to enable
> > and disable tracepoints. The registering of a event is now down
> > directly in the trace_events.c file. The tracepoint_probe_register()
> > is now called directly.
> >
> > The prototypes are no longer type checked, but this should not be
> > an issue since the tracepoints are created automatically by the
> > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
> > other macros will catch it.
>
>
>
> Agreed. Typechecking matters for human code but not in this context.
> Considering that the tracepoint and the probe are created by the same
> CPP code, bugs will be tracked down quickly and located to a single
> place.
So it seems that I am the only one asking for extra type-checking and
caring about problems that can appear subtily on architectures where the
number of caller/callee arguments must match. And also the only one
considering that passing more arguments to a callback that does not
expect all of them might be a problem on some architectures.
Am I the only one thinking there is something fishy there ? I might be
entirely over-paranoid, but this approach has rarely failed me in the
past.
Thanks,
Mathieu
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 14:54 ` Mathieu Desnoyers
@ 2010-05-07 15:12 ` Steven Rostedt
2010-05-07 15:31 ` Mathieu Desnoyers
2010-05-07 18:01 ` Frederic Weisbecker
1 sibling, 1 reply; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 15:12 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Frederic Weisbecker, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
On Fri, 2010-05-07 at 10:54 -0400, Mathieu Desnoyers wrote:
> * Frederic Weisbecker (fweisbec@gmail.com) wrote:
> > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
> > > From: Steven Rostedt <srostedt@redhat.com>
> > >
> > > This patch removes the register functions of TRACE_EVENT() to enable
> > > and disable tracepoints. The registering of a event is now down
> > > directly in the trace_events.c file. The tracepoint_probe_register()
> > > is now called directly.
> > >
> > > The prototypes are no longer type checked, but this should not be
> > > an issue since the tracepoints are created automatically by the
> > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
> > > other macros will catch it.
> >
> >
> >
> > Agreed. Typechecking matters for human code but not in this context.
> > Considering that the tracepoint and the probe are created by the same
> > CPP code, bugs will be tracked down quickly and located to a single
> > place.
>
> So it seems that I am the only one asking for extra type-checking and
> caring about problems that can appear subtily on architectures where the
> number of caller/callee arguments must match. And also the only one
> considering that passing more arguments to a callback that does not
> expect all of them might be a problem on some architectures.
>
> Am I the only one thinking there is something fishy there ? I might be
> entirely over-paranoid, but this approach has rarely failed me in the
> past.
I think you are the only one not realizing that the caller and callee
are created automatically with the same data. There is no human
intervention here.
You are asking to add a check that I can not see helping. The only way
to add a check, is to use the automated process to check the automation.
If the automated process fails, it is very likely the check will also be
broken and will not catch the bug either.
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 15:12 ` Steven Rostedt
@ 2010-05-07 15:31 ` Mathieu Desnoyers
2010-05-07 15:43 ` Steven Rostedt
0 siblings, 1 reply; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-05-07 15:31 UTC (permalink / raw)
To: Steven Rostedt
Cc: Frederic Weisbecker, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Fri, 2010-05-07 at 10:54 -0400, Mathieu Desnoyers wrote:
> > * Frederic Weisbecker (fweisbec@gmail.com) wrote:
> > > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
> > > > From: Steven Rostedt <srostedt@redhat.com>
> > > >
> > > > This patch removes the register functions of TRACE_EVENT() to enable
> > > > and disable tracepoints. The registering of a event is now down
> > > > directly in the trace_events.c file. The tracepoint_probe_register()
> > > > is now called directly.
> > > >
> > > > The prototypes are no longer type checked, but this should not be
> > > > an issue since the tracepoints are created automatically by the
> > > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
> > > > other macros will catch it.
> > >
> > >
> > >
> > > Agreed. Typechecking matters for human code but not in this context.
> > > Considering that the tracepoint and the probe are created by the same
> > > CPP code, bugs will be tracked down quickly and located to a single
> > > place.
> >
> > So it seems that I am the only one asking for extra type-checking and
> > caring about problems that can appear subtily on architectures where the
> > number of caller/callee arguments must match. And also the only one
> > considering that passing more arguments to a callback that does not
> > expect all of them might be a problem on some architectures.
> >
> > Am I the only one thinking there is something fishy there ? I might be
> > entirely over-paranoid, but this approach has rarely failed me in the
> > past.
>
> I think you are the only one not realizing that the caller and callee
> are created automatically with the same data. There is no human
> intervention here.
>
> You are asking to add a check that I can not see helping. The only way
> to add a check, is to use the automated process to check the automation.
> If the automated process fails, it is very likely the check will also be
> broken and will not catch the bug either.
Tell me where to fetch the git head, I'll add it myself. ;)
Thanks,
Mathieu
>
> -- Steve
>
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 15:31 ` Mathieu Desnoyers
@ 2010-05-07 15:43 ` Steven Rostedt
0 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 15:43 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Frederic Weisbecker, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
On Fri, 2010-05-07 at 11:31 -0400, Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@goodmis.org) wrote:
> > You are asking to add a check that I can not see helping. The only way
> > to add a check, is to use the automated process to check the automation.
> > If the automated process fails, it is very likely the check will also be
> > broken and will not catch the bug either.
>
> Tell me where to fetch the git head, I'll add it myself. ;)
It was posted in the 0/9 patch. Although I am modifying it now to
include the latest changes.
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 14:54 ` Mathieu Desnoyers
2010-05-07 15:12 ` Steven Rostedt
@ 2010-05-07 18:01 ` Frederic Weisbecker
2010-05-07 19:08 ` Steven Rostedt
1 sibling, 1 reply; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 18:01 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote:
> * Frederic Weisbecker (fweisbec@gmail.com) wrote:
> > On Mon, May 03, 2010 at 11:40:48PM -0400, Steven Rostedt wrote:
> > > From: Steven Rostedt <srostedt@redhat.com>
> > >
> > > This patch removes the register functions of TRACE_EVENT() to enable
> > > and disable tracepoints. The registering of a event is now down
> > > directly in the trace_events.c file. The tracepoint_probe_register()
> > > is now called directly.
> > >
> > > The prototypes are no longer type checked, but this should not be
> > > an issue since the tracepoints are created automatically by the
> > > macros. If a prototype is incorrect in the TRACE_EVENT() macro, then
> > > other macros will catch it.
> >
> >
> >
> > Agreed. Typechecking matters for human code but not in this context.
> > Considering that the tracepoint and the probe are created by the same
> > CPP code, bugs will be tracked down quickly and located to a single
> > place.
>
> So it seems that I am the only one asking for extra type-checking and
> caring about problems that can appear subtily on architectures where the
> number of caller/callee arguments must match. And also the only one
> considering that passing more arguments to a callback that does not
> expect all of them might be a problem on some architectures.
>
> Am I the only one thinking there is something fishy there ? I might be
> entirely over-paranoid, but this approach has rarely failed me in the
> past.
You are confusing two different and unrelated issues here.
One is the lost typechecking when we _register_ a probe because we now use
directly tracepoint_probe_register() for that.
I don't care personally because both tracepoint and probe are created by
the same automation. There is very few risk of callback type mess and
if there is, it will be located in a small and isolated code.
The second is this extra parameter passed whether or not it is needed.
And although we suppose it is safe, I don't feel comfortable with it.
So if we can find a more proper way to avoid it, I'm all for it.
Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks
2010-05-07 14:09 ` Steven Rostedt
@ 2010-05-07 18:06 ` Frederic Weisbecker
2010-05-07 19:10 ` Steven Rostedt
0 siblings, 1 reply; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 18:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
On Fri, May 07, 2010 at 10:09:31AM -0400, Steven Rostedt wrote:
> > > +#define DECLARE_TRACE(name, proto, args) \
> > > + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> > > + PARAMS(proto, void *__data), \
> > > + PARAMS(args, __data))
> > >
> > > #define DEFINE_TRACE_FN(name, reg, unreg) \
> > > static const char __tpstrtab_##name[] \
> > > @@ -100,19 +133,37 @@ extern void tracepoint_update_probe_range(struct tracepoint *begin,
> > > struct tracepoint *end);
> > >
> > > #else /* !CONFIG_TRACEPOINTS */
> > > -#define DECLARE_TRACE(name, proto, args) \
> > > - static inline void _do_trace_##name(struct tracepoint *tp, proto) \
> > > - { } \
> > > +#define __DECLARE_TRACE(name, proto, args, data_proto, data_args) \
> > > static inline void trace_##name(proto) \
> > > - { } \
> > > + { \
> > > + } \
> > > static inline int register_trace_##name(void (*probe)(proto)) \
> > > { \
> > > return -ENOSYS; \
> > > } \
> > > - static inline int unregister_trace_##name(void (*probe)(proto)) \
> > > + static inline int unregister_trace_##name(void (*probe)(proto)) \
> > > + { \
> > > + return -ENOSYS; \
> > > + } \
> > > + static inline int \
> > > + register_trace_##name##_data(void (*probe)(data_proto), \
> > > + void *data) \
> > > + { \
> > > + return -ENOSYS; \
> > > + } \
> > > + static inline int \
> > > + unregister_trace_##name##_data(void (*probe)(data_proto), \
> > > + void *data) \
> > > { \
> > > return -ENOSYS; \
> > > }
> > > +#define DECLARE_TRACE_NOARGS(name) \
> > > + __DECLARE_TRACE(name, void, , void *__data, __data)
> > > +
> > > +#define DECLARE_TRACE(name, proto, args) \
> > > + __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> > > + PARAMS(proto, void *__data), \
> > > + PARAMS(args, __data))
> >
> >
> >
> >
> > It seems that the on and off cases are exactly the same for DECLARE_TRACE*(),
> > you could provide a single version and let the __DECLARE_TRACE() do
> > the on/off trick.
>
>
> I don't know what you mean here. How would __DECLARE_TRACE() do what
> both DECLARE_TRACE() and DECLARE_TRACE_NOARGS() do? It will fail the
> compile if proto is "void".
No, what I meant is that you have:
#ifdef CONFIG_TRACEPOINTS
[...]
+#define DECLARE_TRACE_NOARGS(name) \
__DECLARE_TRACE(name, void, , void *__data, __data)
#define DECLARE_TRACE(name, proto, args) \
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
PARAMS(proto, void *__data), \
PARAMS(args, __data))
[...]
#else
[...]
+#define DECLARE_TRACE_NOARGS(name) \
__DECLARE_TRACE(name, void, , void *__data, __data)
#define DECLARE_TRACE(name, proto, args) \
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
PARAMS(proto, void *__data), \
PARAMS(args, __data)
[...]
#endif
See? They seem to be the exact same version, so this could be only
one version outside the ifdef.
And the CONFIG_TRACEPOINTS on/off case is dealt from __DECLARE_TRACE().
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 18:01 ` Frederic Weisbecker
@ 2010-05-07 19:08 ` Steven Rostedt
2010-05-07 20:03 ` Frederic Weisbecker
2010-05-07 20:58 ` Mathieu Desnoyers
0 siblings, 2 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 19:08 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Mathieu Desnoyers, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
On Fri, 2010-05-07 at 20:01 +0200, Frederic Weisbecker wrote:
> On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote:
> The second is this extra parameter passed whether or not it is needed.
> And although we suppose it is safe, I don't feel comfortable with it.
> So if we can find a more proper way to avoid it, I'm all for it.
Now I'm making the extra parameter mandatory for all tracepoint
probes. ;-)
But this time, it will be at the start not the end.
void probe(void *data, proto);
Unfortunately we can't avoid it. In order to remove the extra code
(registering and unregistering) and even share the probe among several
events, we need a way to pass the data to the probe to let the probe
know what event it is dealing with (to put in the event id into the
buffer, to let the tracer output code know what event this data is for).
The current method is that only the proto that the tracepoint uses is
passed to the probe. This gives us no way to add any more information.
This new method allows data to be assigned at probe register, and the
probe gets this data as the first parameter.
The register_* functions will still do typechecking of the probes, they
just add the "void *" at the beginning.
Actually, here is a place that I can see where Mathieu's check does come
in handy. If we add the check test to each probe, and the tracepoint
proto changes, it will flag it.
Mathieu, you've been explaining this wrong ;-)
I'm not worried about changes to ftrace.h breaking things. I'm worried
about changes to tracepoint.h breaking ftrace.h. This is where your
check comes in. As I change the void *data from the end to the start,
I'm nervous about catching all the probes that are registered this way.
(ftrace events, syscalls, kprobes, and perf)
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks
2010-05-07 18:06 ` Frederic Weisbecker
@ 2010-05-07 19:10 ` Steven Rostedt
0 siblings, 0 replies; 29+ messages in thread
From: Steven Rostedt @ 2010-05-07 19:10 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Peter Zijlstra, Arnaldo Carvalho de Melo, Mathieu Desnoyers,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
Mathieu Desnoyers
On Fri, 2010-05-07 at 20:06 +0200, Frederic Weisbecker wrote:
> No, what I meant is that you have:
>
> #ifdef CONFIG_TRACEPOINTS
> [...]
> +#define DECLARE_TRACE_NOARGS(name) \
> __DECLARE_TRACE(name, void, , void *__data, __data)
>
> #define DECLARE_TRACE(name, proto, args) \
> __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> PARAMS(proto, void *__data), \
> PARAMS(args, __data))
> [...]
> #else
> [...]
> +#define DECLARE_TRACE_NOARGS(name) \
> __DECLARE_TRACE(name, void, , void *__data, __data)
>
> #define DECLARE_TRACE(name, proto, args) \
> __DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
> PARAMS(proto, void *__data), \
> PARAMS(args, __data)
> [...]
> #endif
>
>
> See? They seem to be the exact same version, so this could be only
> one version outside the ifdef.
> And the CONFIG_TRACEPOINTS on/off case is dealt from __DECLARE_TRACE().
Ah, I see (said the blind man as he slipped and fell on the ice).
-- Steve
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 19:08 ` Steven Rostedt
@ 2010-05-07 20:03 ` Frederic Weisbecker
2010-05-07 20:58 ` Mathieu Desnoyers
1 sibling, 0 replies; 29+ messages in thread
From: Frederic Weisbecker @ 2010-05-07 20:03 UTC (permalink / raw)
To: Steven Rostedt
Cc: Mathieu Desnoyers, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
On Fri, May 07, 2010 at 03:08:25PM -0400, Steven Rostedt wrote:
> On Fri, 2010-05-07 at 20:01 +0200, Frederic Weisbecker wrote:
> > On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote:
>
> > The second is this extra parameter passed whether or not it is needed.
> > And although we suppose it is safe, I don't feel comfortable with it.
> > So if we can find a more proper way to avoid it, I'm all for it.
>
> Now I'm making the extra parameter mandatory for all tracepoint
> probes. ;-)
>
> But this time, it will be at the start not the end.
>
> void probe(void *data, proto);
>
>
> Unfortunately we can't avoid it. In order to remove the extra code
> (registering and unregistering) and even share the probe among several
> events, we need a way to pass the data to the probe to let the probe
> know what event it is dealing with (to put in the event id into the
> buffer, to let the tracer output code know what event this data is for).
>
> The current method is that only the proto that the tracepoint uses is
> passed to the probe. This gives us no way to add any more information.
>
> This new method allows data to be assigned at probe register, and the
> probe gets this data as the first parameter.
>
> The register_* functions will still do typechecking of the probes, they
> just add the "void *" at the beginning.
>
> Actually, here is a place that I can see where Mathieu's check does come
> in handy. If we add the check test to each probe, and the tracepoint
> proto changes, it will flag it.
>
> Mathieu, you've been explaining this wrong ;-)
>
> I'm not worried about changes to ftrace.h breaking things. I'm worried
> about changes to tracepoint.h breaking ftrace.h. This is where your
> check comes in. As I change the void *data from the end to the start,
> I'm nervous about catching all the probes that are registered this way.
> (ftrace events, syscalls, kprobes, and perf)
Yeah right, I see the point.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering
2010-05-07 19:08 ` Steven Rostedt
2010-05-07 20:03 ` Frederic Weisbecker
@ 2010-05-07 20:58 ` Mathieu Desnoyers
1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2010-05-07 20:58 UTC (permalink / raw)
To: Steven Rostedt
Cc: Frederic Weisbecker, linux-kernel, Ingo Molnar, Andrew Morton,
Thomas Gleixner, Peter Zijlstra, Arnaldo Carvalho de Melo,
Lai Jiangshan, Li Zefan, Masami Hiramatsu, Christoph Hellwig,
H. Peter Anvin, Andi Kleen, Paul E. McKenney
* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Fri, 2010-05-07 at 20:01 +0200, Frederic Weisbecker wrote:
> > On Fri, May 07, 2010 at 10:54:38AM -0400, Mathieu Desnoyers wrote:
>
> > The second is this extra parameter passed whether or not it is needed.
> > And although we suppose it is safe, I don't feel comfortable with it.
> > So if we can find a more proper way to avoid it, I'm all for it.
>
> Now I'm making the extra parameter mandatory for all tracepoint
> probes. ;-)
>
> But this time, it will be at the start not the end.
>
> void probe(void *data, proto);
>
>
> Unfortunately we can't avoid it. In order to remove the extra code
> (registering and unregistering) and even share the probe among several
> events, we need a way to pass the data to the probe to let the probe
> know what event it is dealing with (to put in the event id into the
> buffer, to let the tracer output code know what event this data is for).
>
> The current method is that only the proto that the tracepoint uses is
> passed to the probe. This gives us no way to add any more information.
>
> This new method allows data to be assigned at probe register, and the
> probe gets this data as the first parameter.
>
> The register_* functions will still do typechecking of the probes, they
> just add the "void *" at the beginning.
>
> Actually, here is a place that I can see where Mathieu's check does come
> in handy. If we add the check test to each probe, and the tracepoint
> proto changes, it will flag it.
Ah ! finally we are getting on the same page. :-)
>
> Mathieu, you've been explaining this wrong ;-)
Yep, it seems like I've done a terrible job at trying to explain my motivation
to you. ;)
>
> I'm not worried about changes to ftrace.h breaking things. I'm worried
> about changes to tracepoint.h breaking ftrace.h. This is where your
> check comes in. As I change the void *data from the end to the start,
> I'm nervous about catching all the probes that are registered this way.
> (ftrace events, syscalls, kprobes, and perf)
I'm nervous about catching all tracepoint-related problems that may arise from
both tracepoint and tracepoint probes prototype changes, yes.
I'm glad we agree then. :-)
Thanks,
Mathieu
>
> -- Steve
>
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2010-05-07 20:59 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-04 3:40 [PATCH 0/9 - v2][RFC] tracing: Lowering the footprint of TRACE_EVENTs Steven Rostedt
2010-05-04 3:40 ` [PATCH 1/9 - v2][RFC] tracing: Create class struct for events Steven Rostedt
2010-05-07 4:21 ` Frederic Weisbecker
2010-05-04 3:40 ` [PATCH 2/9 - v2][RFC] tracing: Let tracepoints have data passed to tracepoint callbacks Steven Rostedt
2010-05-07 3:52 ` Frederic Weisbecker
2010-05-07 14:09 ` Steven Rostedt
2010-05-07 18:06 ` Frederic Weisbecker
2010-05-07 19:10 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 3/9 - v2][RFC] tracing: Remove per event trace registering Steven Rostedt
2010-05-07 4:20 ` Frederic Weisbecker
2010-05-07 12:42 ` Steven Rostedt
2010-05-07 14:54 ` Mathieu Desnoyers
2010-05-07 15:12 ` Steven Rostedt
2010-05-07 15:31 ` Mathieu Desnoyers
2010-05-07 15:43 ` Steven Rostedt
2010-05-07 18:01 ` Frederic Weisbecker
2010-05-07 19:08 ` Steven Rostedt
2010-05-07 20:03 ` Frederic Weisbecker
2010-05-07 20:58 ` Mathieu Desnoyers
2010-05-07 8:20 ` Li Zefan
2010-05-07 12:59 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 4/9 - v2][RFC] tracing: Move fields from event to class structure Steven Rostedt
2010-05-07 4:49 ` Frederic Weisbecker
2010-05-07 12:57 ` Steven Rostedt
2010-05-04 3:40 ` [PATCH 5/9 - v2][RFC] tracing: Move raw_init from events to class Steven Rostedt
2010-05-04 3:40 ` [PATCH 6/9 - v2][RFC] tracing: Allow events to share their print functions Steven Rostedt
2010-05-04 3:40 ` [PATCH 7/9 - v2][RFC] tracing: Move print functions into event class Steven Rostedt
2010-05-04 3:40 ` [PATCH 8/9 - v2][RFC] tracing: Remove duplicate id information in event structure Steven Rostedt
2010-05-04 3:40 ` [PATCH 9/9 - v2][RFC] tracing: Combine event filter_active and enable into single flags field Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.