Skip to contentSkip to conversation

[2.6.36] tracing: various updates

GIT PULL New Steven Rostedt 2 participants 11 messages Jun 29, 2010

Subsystems (4)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

Ingo,

I pushed some of these to you before, but you wanted to wait till after the merge window to pull them.

The last patch is the full fix for the perf kprobe regression. But since it was too big of a change for the -rc release we did a quick fix. This reverts that change back to the true fix for 2.6.36.

Please pull the latest tip/perf/core tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/core

Chase Douglas (1):
tracing/function-graph: Use correct string size for snprintf

Li Zefan (7):
tracing: Use a global field list for all syscall exit events
tracing: Don't allocate common fields for every trace events
tracing: Convert some timer events to DEFINE_EVENT
tracing: Convert more sched events to DEFINE_EVENT
tracing: Remove test of NULL define_fields callback
tracing: Remove redundant raw_init callbacks
tracing: Remove open-coded __trace_add_event_call()

Steven Rostedt (1):

      tracing: Use class->reg() for all registering of events

----

 include/linux/ftrace_event.h         |    3 +
 include/linux/syscalls.h             |    2 -
 include/trace/events/sched.h         |   32 +----
 include/trace/events/timer.h         |   80 ++++------
 include/trace/ftrace.h               |    2 +
 include/trace/syscall.h              |    1 -
 kernel/trace/trace.h                 |    2 +
 kernel/trace/trace_event_perf.c      |   19 +--
 kernel/trace/trace_events.c          |  264 +++++++++++++++++-----------------
 kernel/trace/trace_events_filter.c   |   27 ++--
 kernel/trace/trace_export.c          |    8 +-
 kernel/trace/trace_functions_graph.c |    3 +-
 kernel/trace/trace_kprobe.c          |   10 +--
 kernel/trace/trace_syscalls.c        |    7 +-
 14 files changed, 203 insertions(+), 257 deletions(-)
author maintainer
#1 lore raw

PATCH 1/9 tracing: Use a global field list for all syscall exit events

From: Li Zefan <[email protected]>

All syscall exit events have the same fields.

The kernel size drops 2.5K:

text data bss dec hex filename
7018612 2034376 7251132 16304120 f8c7f8 vmlinux.o.orig
7018612 2031888 7251132 16301632 f8be40 vmlinux.o

LKML-Reference: <[email protected]>

---
 include/linux/syscalls.h      |    2 --
 include/trace/syscall.h       |    1 -
 kernel/trace/trace_syscalls.c |    7 ++++---
 3 files changed, 4 insertions(+), 6 deletions(-)
include/linux/syscalls.h +0 -2
index 7f614ce..7994bd4 100644
@@ -165,7 +165,6 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 		.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)					\
@@ -180,7 +179,6 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 		.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
include/trace/syscall.h +0 -1
index 257e089..31966a4 100644
@@ -26,7 +26,6 @@ struct syscall_metadata {
 	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;
kernel/trace/trace_syscalls.c +4 -3
index 34e3580..bac752f 100644
@@ -23,6 +23,9 @@ static int syscall_exit_register(struct ftrace_event_call *event,
 static int syscall_enter_define_fields(struct ftrace_event_call *call);
 static int syscall_exit_define_fields(struct ftrace_event_call *call);
 
+/* All syscall exit events have the same fields */
+static LIST_HEAD(syscall_exit_fields);
+
 static struct list_head *
 syscall_get_enter_fields(struct ftrace_event_call *call)
 {
@@ -34,9 +37,7 @@ syscall_get_enter_fields(struct ftrace_event_call *call)
 static struct list_head *
 syscall_get_exit_fields(struct ftrace_event_call *call)
 {
-	struct syscall_metadata *entry = call->data;
-
-	return &entry->exit_fields;
+	return &syscall_exit_fields;
 }
 
 struct trace_event_functions enter_syscall_print_funcs = {
Signature · 1 line
1.7.1
author maintainer
#2 lore raw

PATCH 2/9 tracing: Dont allocate common fields for every trace events

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Every event has the same common fields, so it's a big waste of memory to have a copy of those fields for every event.

LKML-Reference: <[email protected]>

---
 kernel/trace/trace.h               |    2 +
 kernel/trace/trace_events.c        |  113 ++++++++++++++++++++----------------
 kernel/trace/trace_events_filter.c |   18 +++++-
 3 files changed, 81 insertions(+), 52 deletions(-)
kernel/trace/trace.h +2 -0
index 01ce088..cc90ccd 100644
@@ -698,6 +698,8 @@ struct filter_pred {
 	int 			pop_n;
 };
 
+extern struct list_head ftrace_common_fields;
+
 extern enum regex_type
 filter_parse_regex(char *buff, int len, char **search, int *not);
 extern void print_event_filter(struct ftrace_event_call *call,
kernel/trace/trace_events.c +64 -49

185 lines not on this page · raw message

kernel/trace/trace_events_filter.c +15 -3
index 57bb1bb..330fefd 100644
@@ -497,12 +497,10 @@ void print_subsystem_event_filter(struct event_subsystem *system,
 }
 
 static struct ftrace_event_field *
-find_event_field(struct ftrace_event_call *call, char *name)
+__find_event_field(struct list_head *head, char *name)
 {
 	struct ftrace_event_field *field;
-	struct list_head *head;
 
-	head = trace_get_fields(call);
 	list_for_each_entry(field, head, link) {
 		if (!strcmp(field->name, name))
 			return field;
@@ -511,6 +509,20 @@ find_event_field(struct ftrace_event_call *call, char *name)
 	return NULL;
 }
 
+static struct ftrace_event_field *
+find_event_field(struct ftrace_event_call *call, char *name)
+{
+	struct ftrace_event_field *field;
+	struct list_head *head;
+
+	field = __find_event_field(&ftrace_common_fields, name);
+	if (field)
+		return field;
+
+	head = trace_get_fields(call);
+	return __find_event_field(head, name);
+}
+
 static void filter_free_pred(struct filter_pred *pred)
 {
 	if (!pred)
Signature · 1 line
1.7.1
author maintainer
#3 lore raw

PATCH 3/9 tracing: Convert some timer events to DEFINE_EVENT

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Use DECLARE_EVENT_CLASS, and save ~2.3K:

text data bss dec hex filename
7018823 2031888 7251132 16301843 f8bf13 vmlinux.o.orig
7016727 2031696 7251132 16299555 f8b623 vmlinux.o

5 events are converted:

timer_class: timer_init, timer_expire_exit, timer_cancel
hrtimer_class: hrtimer_init, hrtimer_cancel

No change in functionality.

LKML-Reference: <[email protected]>

---
 include/trace/events/timer.h |   80 +++++++++++++++++-------------------------
 1 files changed, 32 insertions(+), 48 deletions(-)
include/trace/events/timer.h +32 -48
index 9496b96..c624126 100644
@@ -8,11 +8,7 @@
 #include <linux/hrtimer.h>
 #include <linux/timer.h>
 
-/**
- * timer_init - called when the timer is initialized
- * @timer:	pointer to struct timer_list
- */
-TRACE_EVENT(timer_init,
+DECLARE_EVENT_CLASS(timer_class,
 
 	TP_PROTO(struct timer_list *timer),
 
@@ -30,6 +26,17 @@ TRACE_EVENT(timer_init,
 );
 
 /**
+ * timer_init - called when the timer is initialized
+ * @timer:	pointer to struct timer_list
+ */
+DEFINE_EVENT(timer_class, timer_init,
+
+	TP_PROTO(struct timer_list *timer),
+
+	TP_ARGS(timer)
+);
+
+/**
  * timer_start - called when the timer is started
  * @timer:	pointer to struct timer_list
  * @expires:	the timers expiry time
@@ -94,42 +101,22 @@ TRACE_EVENT(timer_expire_entry,
  * NOTE: Do NOT derefernce timer in TP_fast_assign. The pointer might
  * be invalid. We solely track the pointer.
  */
-TRACE_EVENT(timer_expire_exit,
+DEFINE_EVENT(timer_class, timer_expire_exit,
 
 	TP_PROTO(struct timer_list *timer),
 
-	TP_ARGS(timer),
-
-	TP_STRUCT__entry(
-		__field(void *,	timer	)
-	),
-
-	TP_fast_assign(
-		__entry->timer	= timer;
-	),
-
-	TP_printk("timer=%p", __entry->timer)
+	TP_ARGS(timer)
 );
 
 /**
  * timer_cancel - called when the timer is canceled
  * @timer:	pointer to struct timer_list
  */
-TRACE_EVENT(timer_cancel,
+DEFINE_EVENT(timer_class, timer_cancel,
 
 	TP_PROTO(struct timer_list *timer),
 
-	TP_ARGS(timer),
-
-	TP_STRUCT__entry(
-		__field( void *,	timer	)
-	),
-
-	TP_fast_assign(
-		__entry->timer	= timer;
-	),
-
-	TP_printk("timer=%p", __entry->timer)
+	TP_ARGS(timer)
 );
 
 /**
@@ -224,14 +211,7 @@ TRACE_EVENT(hrtimer_expire_entry,
 		  (unsigned long long)ktime_to_ns((ktime_t) { .tv64 = __entry->now }))
  );
 
-/**
- * hrtimer_expire_exit - called immediately after the hrtimer callback returns
- * @timer:	pointer to struct hrtimer
- *
- * When used in combination with the hrtimer_expire_entry tracepoint we can
- * determine the runtime of the callback function.
- */
-TRACE_EVENT(hrtimer_expire_exit,
+DECLARE_EVENT_CLASS(hrtimer_class,
 
 	TP_PROTO(struct hrtimer *hrtimer),
 
@@ -249,24 +229,28 @@ TRACE_EVENT(hrtimer_expire_exit,
 );
 
 /**
- * hrtimer_cancel - called when the hrtimer is canceled
- * @hrtimer:	pointer to struct hrtimer
+ * hrtimer_expire_exit - called immediately after the hrtimer callback returns
+ * @timer:	pointer to struct hrtimer
+ *
+ * When used in combination with the hrtimer_expire_entry tracepoint we can
+ * determine the runtime of the callback function.
  */
-TRACE_EVENT(hrtimer_cancel,
+DEFINE_EVENT(hrtimer_class, hrtimer_expire_exit,
 
 	TP_PROTO(struct hrtimer *hrtimer),
 
-	TP_ARGS(hrtimer),
+	TP_ARGS(hrtimer)
+);
 
-	TP_STRUCT__entry(
-		__field( void *,	hrtimer	)
-	),
+/**
+ * hrtimer_cancel - called when the hrtimer is canceled
+ * @hrtimer:	pointer to struct hrtimer
+ */
+DEFINE_EVENT(hrtimer_class, hrtimer_cancel,
 
-	TP_fast_assign(
-		__entry->hrtimer	= hrtimer;
-	),
+	TP_PROTO(struct hrtimer *hrtimer),
 
-	TP_printk("hrtimer=%p", __entry->hrtimer)
+	TP_ARGS(hrtimer)
 );
 
 /**
Signature · 1 line
1.7.1
author maintainer
#4 lore raw

PATCH 4/9 tracing: Convert more sched events to DEFINE_EVENT

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Convert sched_wait_task to DEFINE_EVENT, and save ~1K:

   text    data     bss     dec     hex filename
 104595    9424    4992  119011   1d0e3 kernel/sched.o.orig
 103619    9344    4992  117955   1ccc3 kernel/sched.o

No change in functionality.

LKML-Reference: <[email protected]>

---
 include/trace/events/sched.h |   32 +++++++-------------------------
 1 files changed, 7 insertions(+), 25 deletions(-)
include/trace/events/sched.h +7 -25
index b9e1dd6..9208c92 100644
@@ -50,31 +50,6 @@ TRACE_EVENT(sched_kthread_stop_ret,
 );
 
 /*
- * Tracepoint for waiting on task to unschedule:
- */
-TRACE_EVENT(sched_wait_task,
-
-	TP_PROTO(struct task_struct *p),
-
-	TP_ARGS(p),
-
-	TP_STRUCT__entry(
-		__array(	char,	comm,	TASK_COMM_LEN	)
-		__field(	pid_t,	pid			)
-		__field(	int,	prio			)
-	),
-
-	TP_fast_assign(
-		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
-		__entry->pid	= p->pid;
-		__entry->prio	= p->prio;
-	),
-
-	TP_printk("comm=%s pid=%d prio=%d",
-		  __entry->comm, __entry->pid, __entry->prio)
-);
-
-/*
  * Tracepoint for waking up a task:
  */
 DECLARE_EVENT_CLASS(sched_wakeup_template,
@@ -240,6 +215,13 @@ DEFINE_EVENT(sched_process_template, sched_process_exit,
 	     TP_ARGS(p));
 
 /*
+ * Tracepoint for waiting on task to unschedule:
+ */
+DEFINE_EVENT(sched_process_template, sched_wait_task,
+	TP_PROTO(struct task_struct *p),
+	TP_ARGS(p));
+
+/*
  * Tracepoint for a waiting task:
  */
 TRACE_EVENT(sched_process_wait,
Signature · 1 line
1.7.1
author maintainer
#5 lore raw

PATCH 5/9 tracing: Remove test of NULL define_fields callback

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Every event (or event class) has it's define_fields callback, so the test is redundant.

LKML-Reference: <[email protected]>

---
 kernel/trace/trace_events.c        |   28 +++++++++++++---------------
 kernel/trace/trace_events_filter.c |    9 ---------
 2 files changed, 13 insertions(+), 24 deletions(-)
kernel/trace/trace_events.c +13 -15
index d3b4bdf..5bad9cb 100644
@@ -987,23 +987,21 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 		 		  id);
 #endif
 
-	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 = call->class->define_fields(call);
-			if (ret < 0) {
-				pr_warning("Could not initialize trace point"
-					   " events/%s\n", call->name);
-				return ret;
-			}
+	/*
+	 * 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 = 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);
 	}
+	trace_create_file("filter", 0644, call->dir, call,
+			  filter);
 
 	trace_create_file("format", 0444, call->dir, call,
 			  format);
kernel/trace/trace_events_filter.c +0 -9
index 330fefd..36d4010 100644
@@ -639,9 +639,6 @@ static int init_subsystem_preds(struct event_subsystem *system)
 	int err;
 
 	list_for_each_entry(call, &ftrace_events, list) {
-		if (!call->class || !call->class->define_fields)
-			continue;
-
 		if (strcmp(call->class->system, system->name) != 0)
 			continue;
 
@@ -658,9 +655,6 @@ 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->class || !call->class->define_fields)
-			continue;
-
 		if (strcmp(call->class->system, system->name) != 0)
 			continue;
 
@@ -1263,9 +1257,6 @@ 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->class || !call->class->define_fields)
-			continue;
-
 		if (strcmp(call->class->system, system->name) != 0)
 			continue;
 
Signature · 1 line
1.7.1
author maintainer
#6 lore raw

PATCH 6/9 tracing: Remove redundant raw_init callbacks

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

raw_init callback is optional.

LKML-Reference: <[email protected]>

---
 kernel/trace/trace_export.c |    8 +-------
 kernel/trace/trace_kprobe.c |   10 +---------
 2 files changed, 2 insertions(+), 16 deletions(-)
kernel/trace/trace_export.c +1 -7
index 8536e2a..4ba44de 100644
@@ -125,12 +125,6 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call)	\
 
 #include "trace_entries.h"
 
-static int ftrace_raw_init_event(struct ftrace_event_call *call)
-{
-	INIT_LIST_HEAD(&call->class->fields);
-	return 0;
-}
-
 #undef __entry
 #define __entry REC
 
@@ -158,7 +152,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,		\
+	.fields			= LIST_HEAD_INIT(event_class_ftrace_##call.fields),\
 };									\
 									\
 struct ftrace_event_call __used						\
kernel/trace/trace_kprobe.c +1 -9
index f52b5f5..3b831d8 100644
@@ -1214,11 +1214,6 @@ static void probe_event_disable(struct ftrace_event_call *call)
 	}
 }
 
-static int probe_event_raw_init(struct ftrace_event_call *event_call)
-{
-	return 0;
-}
-
 #undef DEFINE_FIELD
 #define DEFINE_FIELD(type, item, name, is_signed)			\
 	do {								\
@@ -1486,15 +1481,12 @@ static int register_probe_event(struct trace_probe *tp)
 	int ret;
 
 	/* Initialize ftrace_event_call */
+	INIT_LIST_HEAD(&call->class->fields);
 	if (probe_is_return(tp)) {
-		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);
 		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)
Signature · 1 line
1.7.1
author maintainer
#7 lore raw

PATCH 7/9 tracing: Remove open-coded __trace_add_event_call()

In reply to Steven Rostedt

From: Li Zefan <[email protected]>

Let trace_module_add_events() and event_trace_init() call __trace_add_event_call().

LKML-Reference: <[email protected]>

---
 kernel/trace/trace_events.c |   70 +++++++++++-------------------------------
 1 files changed, 19 insertions(+), 51 deletions(-)
kernel/trace/trace_events.c +19 -51
index 5bad9cb..69bee4c 100644
@@ -1009,11 +1009,17 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 	return 0;
 }
 
-static int __trace_add_event_call(struct ftrace_event_call *call)
+static int
+__trace_add_event_call(struct ftrace_event_call *call, struct module *mod,
+		       const struct file_operations *id,
+		       const struct file_operations *enable,
+		       const struct file_operations *filter,
+		       const struct file_operations *format)
 {
 	struct dentry *d_events;
 	int ret;
 
+	/* The linker may leave blanks */
 	if (!call->name)
 		return -EINVAL;
 
@@ -1021,8 +1027,8 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
 		ret = call->class->raw_init(call);
 		if (ret < 0) {
 			if (ret != -ENOSYS)
-				pr_warning("Could not initialize trace "
-				"events/%s\n", call->name);
+				pr_warning("Could not initialize trace events/%s\n",
+					   call->name);
 			return ret;
 		}
 	}
@@ -1031,11 +1037,10 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
 	if (!d_events)
 		return -ENOENT;
 
-	ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
-				&ftrace_enable_fops, &ftrace_event_filter_fops,
-				&ftrace_event_format_fops);
+	ret = event_create_dir(call, d_events, id, enable, filter, format);
 	if (!ret)
 		list_add(&call->list, &ftrace_events);
+	call->mod = mod;
 
 	return ret;
 }
@@ -1045,7 +1050,10 @@ int trace_add_event_call(struct ftrace_event_call *call)
 {
 	int ret;
 	mutex_lock(&event_mutex);
-	ret = __trace_add_event_call(call);
+	ret = __trace_add_event_call(call, NULL, &ftrace_event_id_fops,
+				     &ftrace_enable_fops,
+				     &ftrace_event_filter_fops,
+				     &ftrace_event_format_fops);
 	mutex_unlock(&event_mutex);
 	return ret;
 }
@@ -1162,8 +1170,6 @@ static void trace_module_add_events(struct module *mod)
 {
 	struct ftrace_module_file_ops *file_ops = NULL;
 	struct ftrace_event_call *call, *start, *end;
-	struct dentry *d_events;
-	int ret;
 
 	start = mod->trace_events;
 	end = mod->trace_events + mod->num_trace_events;
@@ -1171,38 +1177,14 @@ static void trace_module_add_events(struct module *mod)
 	if (start == end)
 		return;
 
-	d_events = event_trace_events_dir();
-	if (!d_events)
+	file_ops = trace_create_file_ops(mod);
+	if (!file_ops)
 		return;
 
 	for_each_event(call, start, end) {
-		/* The linker may leave blanks */
-		if (!call->name)
-			continue;
-		if (call->class->raw_init) {
-			ret = call->class->raw_init(call);
-			if (ret < 0) {
-				if (ret != -ENOSYS)
-					pr_warning("Could not initialize trace "
-					"point events/%s\n", call->name);
-				continue;
-			}
-		}
-		/*
-		 * This module has events, create file ops for this module
-		 * if not already done.
-		 */
-		if (!file_ops) {
-			file_ops = trace_create_file_ops(mod);
-			if (!file_ops)
-				return;
-		}
-		call->mod = mod;
-		ret = event_create_dir(call, d_events,
+		__trace_add_event_call(call, mod,
 				       &file_ops->id, &file_ops->enable,
 				       &file_ops->filter, &file_ops->format);
-		if (!ret)
-			list_add(&call->list, &ftrace_events);
 	}
 }
 
@@ -1333,24 +1315,10 @@ static __init int event_trace_init(void)
 		pr_warning("tracing: Failed to allocate common fields");
 
 	for_each_event(call, __start_ftrace_events, __stop_ftrace_events) {
-		/* The linker may leave blanks */
-		if (!call->name)
-			continue;
-		if (call->class->raw_init) {
-			ret = call->class->raw_init(call);
-			if (ret < 0) {
-				if (ret != -ENOSYS)
-					pr_warning("Could not initialize trace "
-					"point events/%s\n", call->name);
-				continue;
-			}
-		}
-		ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
+		__trace_add_event_call(call, NULL, &ftrace_event_id_fops,
 				       &ftrace_enable_fops,
 				       &ftrace_event_filter_fops,
 				       &ftrace_event_format_fops);
-		if (!ret)
-			list_add(&call->list, &ftrace_events);
 	}
 
 	while (true) {
Signature · 1 line
1.7.1
author maintainer
#8 lore raw

PATCH 8/9 tracing/function-graph: Use correct string size for snprintf

In reply to Steven Rostedt

From: Chase Douglas <[email protected]>

The nsecs_str string is a local variable defined as:

char nsecs_str[5];

It is possible for the snprintf call to use a size value larger than the size of the string. This should not cause a buffer overrun as it is written now due to the value for the string format "%03lu" can not be larger than 1000. However, this change makes it correct. By making the size correct we guard against potential future changes that could actually cause a buffer overrun.

LKML-Reference: <[email protected]>

[ added 'UL' to number 8 to fix gcc warning comparing it to sizeof() ]

---
 kernel/trace/trace_functions_graph.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
kernel/trace/trace_functions_graph.c +2 -1
index 79f4bac..6bff236 100644
@@ -641,7 +641,8 @@ trace_print_graph_duration(unsigned long long duration, struct trace_seq *s)
 
 	/* Print nsecs (we don't want to exceed 7 numbers) */
 	if (len < 7) {
-		snprintf(nsecs_str, 8 - len, "%03lu", nsecs_rem);
+		snprintf(nsecs_str, min(sizeof(nsecs_str), 8UL - len), "%03lu",
+			 nsecs_rem);
 		ret = trace_seq_printf(s, ".%s", nsecs_str);
 		if (!ret)
 			return TRACE_TYPE_PARTIAL_LINE;
Signature · 1 line
1.7.1
author maintainer
#9 lore raw

PATCH 9/9 tracing: Use class->reg() for all registering of events

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Because kprobes and syscalls need special processing to register events, the class->reg() method was created to handle the differences.

But instead of creating a default ->reg for perf and ftrace events, the code was scattered with:

	if (class->reg)
		class->reg();
	else
		default_reg();

This is messy and can also lead to bugs.

This patch cleans up this code and creates a default reg() entry for the events allowing for the code to directly call the class->reg() without the condition.

---
 include/linux/ftrace_event.h    |    3 ++
 include/trace/ftrace.h          |    2 +
 kernel/trace/trace_event_perf.c |   19 ++-----------
 kernel/trace/trace_events.c     |   55 +++++++++++++++++++++++++-------------
 4 files changed, 44 insertions(+), 35 deletions(-)
include/linux/ftrace_event.h +3 -0
index 0af31cd..01df7ca 100644
@@ -146,6 +146,9 @@ struct ftrace_event_class {
 	int			(*raw_init)(struct ftrace_event_call *);
 };
 
+extern int ftrace_event_reg(struct ftrace_event_call *event,
+			    enum trace_reg type);
+
 enum {
 	TRACE_EVENT_FL_ENABLED_BIT,
 	TRACE_EVENT_FL_FILTERED_BIT,
include/trace/ftrace.h +2 -0
index fc013a8..55c1fd1 100644
@@ -439,6 +439,7 @@ static inline notrace int ftrace_get_offsets_##call(			\
  *	.fields			= LIST_HEAD_INIT(event_class_##call.fields),
  *	.raw_init		= trace_event_raw_init,
  *	.probe			= ftrace_raw_event_##call,
+ *	.reg			= ftrace_event_reg,
  * };
  *
  * static struct ftrace_event_call __used
@@ -567,6 +568,7 @@ static struct ftrace_event_class __used event_class_##call = {		\
 	.fields			= LIST_HEAD_INIT(event_class_##call.fields),\
 	.raw_init		= trace_event_raw_init,			\
 	.probe			= ftrace_raw_event_##call,		\
+	.reg			= ftrace_event_reg,			\
 	_TRACE_PERF_INIT(call)						\
 };
 
kernel/trace/trace_event_perf.c +3 -16
index 6053982..2375165 100644
@@ -54,13 +54,7 @@ static int perf_trace_event_init(struct ftrace_event_call *tp_event,
 		}
 	}
 
-	if (tp_event->class->reg)
-		ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_REGISTER);
-	else
-		ret = tracepoint_probe_register(tp_event->name,
-						tp_event->class->perf_probe,
-						tp_event);
-
+	ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_REGISTER);
 	if (ret)
 		goto fail;
 
@@ -94,9 +88,7 @@ int perf_trace_init(struct perf_event *p_event)
 	mutex_lock(&event_mutex);
 	list_for_each_entry(tp_event, &ftrace_events, list) {
 		if (tp_event->event.type == event_id &&
-		    tp_event->class &&
-		    (tp_event->class->perf_probe ||
-		     tp_event->class->reg) &&
+		    tp_event->class && tp_event->class->reg &&
 		    try_module_get(tp_event->mod)) {
 			ret = perf_trace_event_init(tp_event, p_event);
 			break;
@@ -136,12 +128,7 @@ void perf_trace_destroy(struct perf_event *p_event)
 	if (--tp_event->perf_refcount > 0)
 		goto out;
 
-	if (tp_event->class->reg)
-		tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER);
-	else
-		tracepoint_probe_unregister(tp_event->name,
-					    tp_event->class->perf_probe,
-					    tp_event);
+	tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER);
 
 	/*
 	 * Ensure our callback won't be called anymore. See
kernel/trace/trace_events.c +36 -19

100 lines not on this page · raw message

Signature · 1 line
1.7.1
#10 lore raw
In reply to Steven Rostedt
Quoted 32 lines · Steven Rostedt

Ingo,

I pushed some of these to you before, but you wanted to wait till after the merge window to pull them.

The last patch is the full fix for the perf kprobe regression. But since it was too big of a change for the -rc release we did a quick fix. This reverts that change back to the true fix for 2.6.36.

Please pull the latest tip/perf/core tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/core

Chase Douglas (1):
tracing/function-graph: Use correct string size for snprintf

Li Zefan (7):
tracing: Use a global field list for all syscall exit events
tracing: Don't allocate common fields for every trace events
tracing: Convert some timer events to DEFINE_EVENT
tracing: Convert more sched events to DEFINE_EVENT
tracing: Remove test of NULL define_fields callback
tracing: Remove redundant raw_init callbacks
tracing: Remove open-coded __trace_add_event_call()

Steven Rostedt (1):

      tracing: Use class->reg() for all registering of events

----

Quoted 15 lines
 include/linux/ftrace_event.h         |    3 +
 include/linux/syscalls.h             |    2 -
 include/trace/events/sched.h         |   32 +----
 include/trace/events/timer.h         |   80 ++++------
 include/trace/ftrace.h               |    2 +
 include/trace/syscall.h              |    1 -
 kernel/trace/trace.h                 |    2 +
 kernel/trace/trace_event_perf.c      |   19 +--
 kernel/trace/trace_events.c          |  264 +++++++++++++++++-----------------
 kernel/trace/trace_events_filter.c   |   27 ++--
 kernel/trace/trace_export.c          |    8 +-
 kernel/trace/trace_functions_graph.c |    3 +-
 kernel/trace/trace_kprobe.c          |   10 +--
 kernel/trace/trace_syscalls.c        |    7 +-
 14 files changed, 203 insertions(+), 257 deletions(-)

Pulled, thanks a lot Steve!

Ingo