[PATCH 00/13] tracing: various cleanups and small fixes (updated)

[PATCH 00/13] tracing: various cleanups and small fixes (updated)

[Li Zefan]

Updates:

- took a further step to save more spaces (01/13)
- fixed a mistake in (03/13)
- collected some ACKs


Here are some patches that I've collected. All are cleanups and
small fixes, no change in functionality.

As a result, 24K bytes are saved, and 87 lines of code are
reduced:

   text    data     bss     dec     hex filename
5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
5331208 1961864 7103260 14396332         dbabac vmlinux.o

---
 include/linux/ftrace_event.h       |    4 +-
 include/linux/syscalls.h           |    6 +-
 include/trace/ftrace.h             |   56 ++-----------
 kernel/trace/ftrace.c              |   30 ++++---
 kernel/trace/power-traces.c        |    2 -
 kernel/trace/trace.c               |  166 +++++++++++++-----------------------
 kernel/trace/trace.h               |   23 +++---
 kernel/trace/trace_event_profile.c |    6 +-
 kernel/trace/trace_events.c        |   38 +++++++--
 kernel/trace/trace_export.c        |    4 -
 kernel/trace/trace_kprobe.c        |    9 --
 kernel/trace/trace_ksym.c          |   13 ++--
 kernel/trace/trace_syscalls.c      |   18 +----
 13 files changed, 144 insertions(+), 231 deletions(-)

[PATCH 01/13] tracing: Extract duplicate ftrace_raw_init_event_foo()

[Li Zefan]

Define ftrace_raw_init_event_foo() for each event class rather
than for each event:

   text    data     bss     dec     hex filename
5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
5346802 1961864 7103260 14411926         dbe896 vmlinux.o

raw_init can't be removed, because ftrace events and kprobe events
use different raw_init callbacks. Though it's possible to totally
remove raw_init, I choose to leave it as it is for now.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 include/linux/ftrace_event.h |    1 +
 include/linux/syscalls.h     |    4 ++--
 include/trace/ftrace.h       |   35 ++++-------------------------------
 kernel/trace/trace_events.c  |   14 ++++++++++++++
 4 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 47bbdf9..cdea8a9 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -157,6 +157,7 @@ enum {
 	FILTER_PTR_STRING,
 };
 
+extern int trace_event_raw_init(struct ftrace_event_call *call);
 extern int trace_define_common_fields(struct ftrace_event_call *call);
 extern int trace_define_field(struct ftrace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index e79e2f3..55e7108 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -144,7 +144,7 @@ struct perf_event_attr;
 		.name                   = "sys_enter"#sname,		\
 		.system                 = "syscalls",			\
 		.event                  = &enter_syscall_print_##sname,	\
-		.raw_init		= init_syscall_trace,		\
+		.raw_init		= trace_event_raw_init,		\
 		.show_format		= syscall_enter_format,		\
 		.define_fields		= syscall_enter_define_fields,	\
 		.regfunc		= reg_event_syscall_enter,	\
@@ -166,7 +166,7 @@ struct perf_event_attr;
 		.name                   = "sys_exit"#sname,		\
 		.system                 = "syscalls",			\
 		.event                  = &exit_syscall_print_##sname,	\
-		.raw_init		= init_syscall_trace,		\
+		.raw_init		= trace_event_raw_init,		\
 		.show_format		= syscall_exit_format,		\
 		.define_fields		= syscall_exit_define_fields,	\
 		.regfunc		= reg_event_syscall_exit,	\
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index d1b3de9..f46ad26 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -623,23 +623,12 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *	.trace			= ftrace_raw_output_<call>, <-- stage 2
  * };
  *
- * static int ftrace_raw_init_event_<call>(struct ftrace_event_call *unused)
- * {
- *	int id;
- *
- *	id = register_ftrace_event(&ftrace_event_type_<call>);
- *	if (!id)
- *		return -ENODEV;
- *	event_<call>.id = id;
- *	return 0;
- * }
- *
  * static struct ftrace_event_call __used
  * __attribute__((__aligned__(4)))
  * __attribute__((section("_ftrace_events"))) event_<call> = {
  *	.name			= "<call>",
  *	.system			= "<system>",
- *	.raw_init		= ftrace_raw_init_event_<call>,
+ *	.raw_init		= trace_event_raw_init,
  *	.regfunc		= ftrace_reg_event_<call>,
  *	.unregfunc		= ftrace_unreg_event_<call>,
  *	.show_format		= ftrace_format_<call>,
@@ -647,9 +636,6 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *
  */
 
-#undef TP_FMT
-#define TP_FMT(fmt, args...)	fmt "\n", ##args
-
 #ifdef CONFIG_EVENT_PROFILE
 
 #define _TRACE_PROFILE_INIT(call)					\
@@ -744,19 +730,7 @@ static void ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused)\
 									\
 static struct trace_event ftrace_event_type_##call = {			\
 	.trace			= ftrace_raw_output_##call,		\
-};									\
-									\
-static int ftrace_raw_init_event_##call(struct ftrace_event_call *unused)\
-{									\
-	int id;								\
-									\
-	id = register_ftrace_event(&ftrace_event_type_##call);		\
-	if (!id)							\
-		return -ENODEV;						\
-	event_##call.id = id;						\
-	INIT_LIST_HEAD(&event_##call.fields);				\
-	return 0;							\
-}
+};
 
 #undef DEFINE_EVENT_PRINT
 #define DEFINE_EVENT_PRINT(template, name, proto, args, print)	\
@@ -776,7 +750,7 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
 	.name			= #call,				\
 	.system			= __stringify(TRACE_SYSTEM),		\
 	.event			= &ftrace_event_type_##call,		\
-	.raw_init		= ftrace_raw_init_event_##call,		\
+	.raw_init		= trace_event_raw_init,			\
 	.regfunc		= ftrace_raw_reg_event_##call,		\
 	.unregfunc		= ftrace_raw_unreg_event_##call,	\
 	.show_format		= ftrace_format_##template,		\
@@ -793,7 +767,7 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
 	.name			= #call,				\
 	.system			= __stringify(TRACE_SYSTEM),		\
 	.event			= &ftrace_event_type_##call,		\
-	.raw_init		= ftrace_raw_init_event_##call,		\
+	.raw_init		= trace_event_raw_init,			\
 	.regfunc		= ftrace_raw_reg_event_##call,		\
 	.unregfunc		= ftrace_raw_unreg_event_##call,	\
 	.show_format		= ftrace_format_##call,			\
@@ -953,7 +927,6 @@ end:									\
 	perf_swevent_put_recursion_context(rctx);			\
 end_recursion:								\
 	local_irq_restore(irq_flags);					\
-									\
 }
 
 #undef DEFINE_EVENT
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 1d18315..8ed66e0 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -105,6 +105,20 @@ void trace_destroy_fields(struct ftrace_event_call *call)
 	}
 }
 
+int trace_event_raw_init(struct ftrace_event_call *call)
+{
+	int id;
+
+	id = register_ftrace_event(call->event);
+	if (!id)
+		return -ENODEV;
+	call->id = id;
+	INIT_LIST_HEAD(&call->fields);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(trace_event_raw_init);
+
 static void ftrace_event_enable_disable(struct ftrace_event_call *call,
 					int enable)
 {
-- 
1.6.3

[PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Li Zefan]

Call trace_define_common_fields() in event_create_dir() only.

   text    data     bss     dec     hex filename
5346802 1961864 7103260 14411926         dbe896 vmlinux.o.old
5345151 1961864 7103260 14410275         dbe223 vmlinux.o

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/ftrace_event.h  |    1 -
 include/trace/ftrace.h        |    4 ----
 kernel/trace/trace_events.c   |    7 ++++---
 kernel/trace/trace_export.c   |    4 ----
 kernel/trace/trace_kprobe.c   |    8 --------
 kernel/trace/trace_syscalls.c |    8 --------
 6 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index cdea8a9..f883406 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -158,7 +158,6 @@ enum {
 };
 
 extern int trace_event_raw_init(struct ftrace_event_call *call);
-extern int trace_define_common_fields(struct ftrace_event_call *call);
 extern 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);
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index f46ad26..f978376 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -436,10 +436,6 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call)	\
 	struct ftrace_raw_##call field;					\
 	int ret;							\
 									\
-	ret = trace_define_common_fields(event_call);			\
-	if (ret)							\
-		return ret;						\
-									\
 	tstruct;							\
 									\
 	return ret;							\
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 8ed66e0..97b0b3a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -78,7 +78,7 @@ EXPORT_SYMBOL_GPL(trace_define_field);
 	if (ret)							\
 		return ret;
 
-int trace_define_common_fields(struct ftrace_event_call *call)
+static int trace_define_common_fields(struct ftrace_event_call *call)
 {
 	int ret;
 	struct trace_entry ent;
@@ -91,7 +91,6 @@ int trace_define_common_fields(struct ftrace_event_call *call)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(trace_define_common_fields);
 
 void trace_destroy_fields(struct ftrace_event_call *call)
 {
@@ -927,7 +926,9 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 		 		  id);
 
 	if (call->define_fields) {
-		ret = call->define_fields(call);
+		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);
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index dff8c84..458e5bf 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -184,10 +184,6 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call)	\
 	struct struct_name field;					\
 	int ret;							\
 									\
-	ret = trace_define_common_fields(event_call);			\
-	if (ret)							\
-		return ret;						\
-									\
 	tstruct;							\
 									\
 	return ret;							\
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aff5f80..e3c80e9 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
 	struct kprobe_trace_entry field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
-	ret = trace_define_common_fields(event_call);
-	if (!ret)
-		return ret;
-
 	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
 	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
 	/* Set argument names as fields */
@@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
 	struct kretprobe_trace_entry field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
-	ret = trace_define_common_fields(event_call);
-	if (!ret)
-		return ret;
-
 	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
 	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
 	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 57501d9..b957edd 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -217,10 +217,6 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
 	int i;
 	int offset = offsetof(typeof(trace), args);
 
-	ret = trace_define_common_fields(call);
-	if (ret)
-		return ret;
-
 	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
 	if (ret)
 		return ret;
@@ -241,10 +237,6 @@ int syscall_exit_define_fields(struct ftrace_event_call *call)
 	struct syscall_trace_exit trace;
 	int ret;
 
-	ret = trace_define_common_fields(call);
-	if (ret)
-		return ret;
-
 	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
 	if (ret)
 		return ret;
-- 
1.6.3

[PATCH 03/13] tracing: Move a printk out of ftrace_raw_reg_event_foo()

[Li Zefan]

Move the printk from each ftrace_raw_reg_event_foo() to
its caller ftrace_event_enable_disable().

See how much space this saves:

   text    data     bss     dec     hex filename
5345151 1961864 7103260 14410275         dbe223 vmlinux.o.old
5331487 1961864 7103260 14396611         dbacc3 vmlinux.o

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 include/trace/ftrace.h        |   16 ++--------------
 kernel/trace/trace_events.c   |   17 +++++++++++++----
 kernel/trace/trace_syscalls.c |   10 ++--------
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index f978376..0c21af8 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -555,13 +555,7 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *
  * static int ftrace_reg_event_<call>(struct ftrace_event_call *unused)
  * {
- *	int ret;
- *
- *	ret = register_trace_<call>(ftrace_event_<call>);
- *	if (!ret)
- *		pr_info("event trace: Could not activate trace point "
- *			"probe to  <call>");
- *	return ret;
+ *	return register_trace_<call>(ftrace_event_<call>);
  * }
  *
  * static void ftrace_unreg_event_<call>(struct ftrace_event_call *unused)
@@ -710,13 +704,7 @@ static void ftrace_raw_event_##call(proto)				\
 									\
 static int ftrace_raw_reg_event_##call(struct ftrace_event_call *unused)\
 {									\
-	int ret;							\
-									\
-	ret = register_trace_##call(ftrace_raw_event_##call);		\
-	if (ret)							\
-		pr_info("event trace: Could not activate trace point "	\
-			"probe to " #call "\n");			\
-	return ret;							\
+	return register_trace_##call(ftrace_raw_event_##call);		\
 }									\
 									\
 static void ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused)\
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 97b0b3a..bf2f1b9 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -118,9 +118,11 @@ int trace_event_raw_init(struct ftrace_event_call *call)
 }
 EXPORT_SYMBOL_GPL(trace_event_raw_init);
 
-static void ftrace_event_enable_disable(struct ftrace_event_call *call,
+static int ftrace_event_enable_disable(struct ftrace_event_call *call,
 					int enable)
 {
+	int ret = 0;
+
 	switch (enable) {
 	case 0:
 		if (call->enabled) {
@@ -131,12 +133,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
 		break;
 	case 1:
 		if (!call->enabled) {
+			ret = call->regfunc(call);
+			if (ret) {
+				pr_info("event trace: Could not enable event "
+					"%s\n", call->name);
+				break;
+			}
 			call->enabled = 1;
 			tracing_start_cmdline_record();
-			call->regfunc(call);
 		}
 		break;
 	}
+
+	return ret;
 }
 
 static void ftrace_clear_events(void)
@@ -415,7 +424,7 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	case 0:
 	case 1:
 		mutex_lock(&event_mutex);
-		ftrace_event_enable_disable(call, val);
+		ret = ftrace_event_enable_disable(call, val);
 		mutex_unlock(&event_mutex);
 		break;
 
@@ -425,7 +434,7 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
 
 	*ppos += cnt;
 
-	return cnt;
+	return ret ? ret : cnt;
 }
 
 static ssize_t
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index b957edd..75289f3 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -325,10 +325,7 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_enter)
 		ret = register_trace_sys_enter(ftrace_syscall_enter);
-	if (ret) {
-		pr_info("event trace: Could not activate"
-				"syscall entry trace point");
-	} else {
+	if (!ret) {
 		set_bit(num, enabled_enter_syscalls);
 		sys_refcount_enter++;
 	}
@@ -362,10 +359,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_exit)
 		ret = register_trace_sys_exit(ftrace_syscall_exit);
-	if (ret) {
-		pr_info("event trace: Could not activate"
-				"syscall exit trace point");
-	} else {
+	if (!ret) {
 		set_bit(num, enabled_exit_syscalls);
 		sys_refcount_exit++;
 	}
-- 
1.6.3

[PATCH 04/13] ftrace: Return EINVAL when writing invalid val to set_ftrace_filter

[Li Zefan]

Currently it doesn't warn user on invald value:

 # echo nonexist_symbol > set_ftrace_filter
or:
 # echo 'nonexist_symbol:mod:fuse' > set_ftrace_filter

Better make it return failure.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/ftrace.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e51a1bc..08a3fb5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1724,7 +1724,7 @@ ftrace_match_record(struct dyn_ftrace *rec, char *regex, int len, int type)
 	return ftrace_match(str, regex, len, type);
 }
 
-static void ftrace_match_records(char *buff, int len, int enable)
+static int ftrace_match_records(char *buff, int len, int enable)
 {
 	unsigned int search_len;
 	struct ftrace_page *pg;
@@ -1733,6 +1733,7 @@ static void ftrace_match_records(char *buff, int len, int enable)
 	char *search;
 	int type;
 	int not;
+	int found = 0;
 
 	flag = enable ? FTRACE_FL_FILTER : FTRACE_FL_NOTRACE;
 	type = filter_parse_regex(buff, len, &search, &not);
@@ -1750,6 +1751,7 @@ static void ftrace_match_records(char *buff, int len, int enable)
 				rec->flags &= ~flag;
 			else
 				rec->flags |= flag;
+			found = 1;
 		}
 		/*
 		 * Only enable filtering if we have a function that
@@ -1759,6 +1761,8 @@ static void ftrace_match_records(char *buff, int len, int enable)
 			ftrace_filtered = 1;
 	} while_for_each_ftrace_rec();
 	mutex_unlock(&ftrace_lock);
+
+	return found;
 }
 
 static int
@@ -1780,7 +1784,7 @@ ftrace_match_module_record(struct dyn_ftrace *rec, char *mod,
 		return 1;
 }
 
-static void ftrace_match_module_records(char *buff, char *mod, int enable)
+static int ftrace_match_module_records(char *buff, char *mod, int enable)
 {
 	unsigned search_len = 0;
 	struct ftrace_page *pg;
@@ -1789,6 +1793,7 @@ static void ftrace_match_module_records(char *buff, char *mod, int enable)
 	char *search = buff;
 	unsigned long flag;
 	int not = 0;
+	int found = 0;
 
 	flag = enable ? FTRACE_FL_FILTER : FTRACE_FL_NOTRACE;
 
@@ -1819,12 +1824,15 @@ static void ftrace_match_module_records(char *buff, char *mod, int enable)
 				rec->flags &= ~flag;
 			else
 				rec->flags |= flag;
+			found = 1;
 		}
 		if (enable && (rec->flags & FTRACE_FL_FILTER))
 			ftrace_filtered = 1;
 
 	} while_for_each_ftrace_rec();
 	mutex_unlock(&ftrace_lock);
+
+	return found;
 }
 
 /*
@@ -1853,8 +1861,9 @@ ftrace_mod_callback(char *func, char *cmd, char *param, int enable)
 	if (!strlen(mod))
 		return -EINVAL;
 
-	ftrace_match_module_records(func, mod, enable);
-	return 0;
+	if (ftrace_match_module_records(func, mod, enable))
+		return 0;
+	return -EINVAL;
 }
 
 static struct ftrace_func_command ftrace_mod_cmd = {
@@ -2151,8 +2160,9 @@ static int ftrace_process_regex(char *buff, int len, int enable)
 	func = strsep(&next, ":");
 
 	if (!next) {
-		ftrace_match_records(func, len, enable);
-		return 0;
+		if (ftrace_match_records(func, len, enable))
+			return 0;
+		return ret;
 	}
 
 	/* command found */
-- 
1.6.3

[PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Li Zefan]

I found a weird behavior:

  # echo 'fuse:*' > set_ftrace_filter
  bash: echo: write error: Invalid argument
  # cat set_ftrace_filter
  fuse_dev_fasync
  fuse_dev_poll
  fuse_copy_do

We should call trace_parser_clear() no matter ftrace_process_regex()
returns 0 or -errno.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/ftrace.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 08a3fb5..ff8aecd 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2208,10 +2208,9 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
 	    !trace_parser_cont(parser)) {
 		ret = ftrace_process_regex(parser->buffer,
 					   parser->idx, enable);
+		trace_parser_clear(parser);
 		if (ret)
 			goto out_unlock;
-
-		trace_parser_clear(parser);
 	}
 
 	ret = read;
-- 
1.6.3

[PATCH 06/13] function-graph: Allow writing the same val to set_graph_function

[Li Zefan]

 # echo 'do_open' > set_graph_function
 # echo 'do_open' >> set_graph_function
 bash: echo: write error: Invalid argument

Make it valid to write the same value to set_graph_function,
which is consistent with set_ftrace_filter interface.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/ftrace.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ff8aecd..7968762 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2552,10 +2552,9 @@ ftrace_set_func(unsigned long *array, int *idx, char *buffer)
 					exists = true;
 					break;
 				}
-			if (!exists) {
+			if (!exists)
 				array[(*idx)++] = rec->ip;
-				found = 1;
-			}
+			found = 1;
 		}
 	} while_for_each_ftrace_rec();
 
-- 
1.6.3

[PATCH 07/13] tracing: Use seq file for trace_options

[Li Zefan]

Code simplification for reading trace_options.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace.c |   60 ++++++++++++++-----------------------------------
 1 files changed, 17 insertions(+), 43 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 874f289..e989fc9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2291,67 +2291,32 @@ static const struct file_operations tracing_cpumask_fops = {
 	.write		= tracing_cpumask_write,
 };
 
-static ssize_t
-tracing_trace_options_read(struct file *filp, char __user *ubuf,
-		       size_t cnt, loff_t *ppos)
+static int tracing_trace_options_show(struct seq_file *m, void *v)
 {
 	struct tracer_opt *trace_opts;
 	u32 tracer_flags;
-	int len = 0;
-	char *buf;
-	int r = 0;
 	int i;
 
-
-	/* calculate max size */
-	for (i = 0; trace_options[i]; i++) {
-		len += strlen(trace_options[i]);
-		len += 3; /* "no" and newline */
-	}
-
 	mutex_lock(&trace_types_lock);
 	tracer_flags = current_trace->flags->val;
 	trace_opts = current_trace->flags->opts;
 
-	/*
-	 * Increase the size with names of options specific
-	 * of the current tracer.
-	 */
-	for (i = 0; trace_opts[i].name; i++) {
-		len += strlen(trace_opts[i].name);
-		len += 3; /* "no" and newline */
-	}
-
-	/* +1 for \0 */
-	buf = kmalloc(len + 1, GFP_KERNEL);
-	if (!buf) {
-		mutex_unlock(&trace_types_lock);
-		return -ENOMEM;
-	}
-
 	for (i = 0; trace_options[i]; i++) {
 		if (trace_flags & (1 << i))
-			r += sprintf(buf + r, "%s\n", trace_options[i]);
+			seq_printf(m, "%s\n", trace_options[i]);
 		else
-			r += sprintf(buf + r, "no%s\n", trace_options[i]);
+			seq_printf(m, "no%s\n", trace_options[i]);
 	}
 
 	for (i = 0; trace_opts[i].name; i++) {
 		if (tracer_flags & trace_opts[i].bit)
-			r += sprintf(buf + r, "%s\n",
-				trace_opts[i].name);
+			seq_printf(m, "%s\n", trace_opts[i].name);
 		else
-			r += sprintf(buf + r, "no%s\n",
-				trace_opts[i].name);
+			seq_printf(m, "no%s\n", trace_opts[i].name);
 	}
 	mutex_unlock(&trace_types_lock);
 
-	WARN_ON(r >= len + 1);
-
-	r = simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
-
-	kfree(buf);
-	return r;
+	return 0;
 }
 
 /* Try to assign a tracer specific option */
@@ -2446,9 +2411,18 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
+static int tracing_trace_options_open(struct inode *inode, struct file *file)
+{
+	if (tracing_disabled)
+		return -ENODEV;
+	return single_open(file, tracing_trace_options_show, NULL);
+}
+
 static const struct file_operations tracing_iter_fops = {
-	.open		= tracing_open_generic,
-	.read		= tracing_trace_options_read,
+	.open		= tracing_trace_options_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
 	.write		= tracing_trace_options_write,
 };
 
-- 
1.6.3

[PATCH 08/13] tracing: Use seq file for trace_clock

[Li Zefan]

The buffer for the output is as small as 64 bytes, so it'll
overflow if we add more clock type. Use seq file instead.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e989fc9..75871e5 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3327,21 +3327,18 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
-static ssize_t tracing_clock_read(struct file *filp, char __user *ubuf,
-				  size_t cnt, loff_t *ppos)
+static int tracing_clock_show(struct seq_file *m, void *v)
 {
-	char buf[64];
-	int bufiter = 0;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(trace_clocks); i++)
-		bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter,
+		seq_printf(m,
 			"%s%s%s%s", i ? " " : "",
 			i == trace_clock_id ? "[" : "", trace_clocks[i].name,
 			i == trace_clock_id ? "]" : "");
-	bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter, "\n");
+	seq_putc(m, '\n');
 
-	return simple_read_from_buffer(ubuf, cnt, ppos, buf, bufiter);
+	return 0;
 }
 
 static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
@@ -3383,6 +3380,13 @@ static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
+static int tracing_clock_open(struct inode *inode, struct file *file)
+{
+	if (tracing_disabled)
+		return -ENODEV;
+	return single_open(file, tracing_clock_show, NULL);
+}
+
 static const struct file_operations tracing_max_lat_fops = {
 	.open		= tracing_open_generic,
 	.read		= tracing_max_lat_read,
@@ -3421,8 +3425,10 @@ static const struct file_operations tracing_mark_fops = {
 };
 
 static const struct file_operations trace_clock_fops = {
-	.open		= tracing_open_generic,
-	.read		= tracing_clock_read,
+	.open		= tracing_clock_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
 	.write		= tracing_clock_write,
 };
 
-- 
1.6.3

[PATCH 09/13] tracing: Remove useless trace option

[Li Zefan]

Since commit 4d9493c90f8e6e1b164aede3814010a290161abb
("ftrace: remove add-hoc code"), option "sched-tree"
has become useless.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace.c |    1 -
 kernel/trace/trace.h |   23 +++++++++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 75871e5..2801b0b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -313,7 +313,6 @@ static const char *trace_options[] = {
 	"bin",
 	"block",
 	"stacktrace",
-	"sched-tree",
 	"trace_printk",
 	"ftrace_preempt",
 	"branch",
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index ea5dda4..60f9471 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -597,18 +597,17 @@ enum trace_iterator_flags {
 	TRACE_ITER_BIN			= 0x40,
 	TRACE_ITER_BLOCK		= 0x80,
 	TRACE_ITER_STACKTRACE		= 0x100,
-	TRACE_ITER_SCHED_TREE		= 0x200,
-	TRACE_ITER_PRINTK		= 0x400,
-	TRACE_ITER_PREEMPTONLY		= 0x800,
-	TRACE_ITER_BRANCH		= 0x1000,
-	TRACE_ITER_ANNOTATE		= 0x2000,
-	TRACE_ITER_USERSTACKTRACE       = 0x4000,
-	TRACE_ITER_SYM_USEROBJ          = 0x8000,
-	TRACE_ITER_PRINTK_MSGONLY	= 0x10000,
-	TRACE_ITER_CONTEXT_INFO		= 0x20000, /* Print pid/cpu/time */
-	TRACE_ITER_LATENCY_FMT		= 0x40000,
-	TRACE_ITER_SLEEP_TIME		= 0x80000,
-	TRACE_ITER_GRAPH_TIME		= 0x100000,
+	TRACE_ITER_PRINTK		= 0x200,
+	TRACE_ITER_PREEMPTONLY		= 0x400,
+	TRACE_ITER_BRANCH		= 0x800,
+	TRACE_ITER_ANNOTATE		= 0x1000,
+	TRACE_ITER_USERSTACKTRACE       = 0x2000,
+	TRACE_ITER_SYM_USEROBJ          = 0x4000,
+	TRACE_ITER_PRINTK_MSGONLY	= 0x8000,
+	TRACE_ITER_CONTEXT_INFO		= 0x10000, /* Print pid/cpu/time */
+	TRACE_ITER_LATENCY_FMT		= 0x20000,
+	TRACE_ITER_SLEEP_TIME		= 0x40000,
+	TRACE_ITER_GRAPH_TIME		= 0x80000,
 };
 
 /*
-- 
1.6.3

[PATCH 10/13] tracing: Simplify trace_option_write()

[Li Zefan]

- remove duplicate code inside trace_options_write()
- extract duplicate code in trace_options_write() and set_tracer_option()

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace.c |   85 ++++++++++++++++++-------------------------------
 1 files changed, 31 insertions(+), 54 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2801b0b..9359b74 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2318,38 +2318,39 @@ static int tracing_trace_options_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int __set_tracer_option(struct tracer *trace,
+			       struct tracer_flags *tracer_flags,
+			       struct tracer_opt *opts, int neg)
+{
+	int ret;
+
+	ret = trace->set_flag(tracer_flags->val, opts->bit, !neg);
+	if (ret)
+		return ret;
+
+	if (neg)
+		tracer_flags->val &= ~opts->bit;
+	else
+		tracer_flags->val |= opts->bit;
+	return 0;
+}
+
 /* Try to assign a tracer specific option */
 static int set_tracer_option(struct tracer *trace, char *cmp, int neg)
 {
 	struct tracer_flags *tracer_flags = trace->flags;
 	struct tracer_opt *opts = NULL;
-	int ret = 0, i = 0;
-	int len;
+	int i;
 
 	for (i = 0; tracer_flags->opts[i].name; i++) {
 		opts = &tracer_flags->opts[i];
-		len = strlen(opts->name);
 
-		if (strncmp(cmp, opts->name, len) == 0) {
-			ret = trace->set_flag(tracer_flags->val,
-				opts->bit, !neg);
-			break;
-		}
+		if (strcmp(cmp, opts->name) == 0)
+			return __set_tracer_option(trace, trace->flags,
+						   opts, neg);
 	}
-	/* Not found */
-	if (!tracer_flags->opts[i].name)
-		return -EINVAL;
 
-	/* Refused to handle */
-	if (ret)
-		return ret;
-
-	if (neg)
-		tracer_flags->val &= ~opts->bit;
-	else
-		tracer_flags->val |= opts->bit;
-
-	return 0;
+	return -EINVAL;
 }
 
 static void set_tracer_flags(unsigned int mask, int enabled)
@@ -2369,7 +2370,7 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
 {
 	char buf[64];
-	char *cmp = buf;
+	char *cmp;
 	int neg = 0;
 	int ret;
 	int i;
@@ -2381,16 +2382,15 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 		return -EFAULT;
 
 	buf[cnt] = 0;
+	cmp = strstrip(buf);
 
-	if (strncmp(buf, "no", 2) == 0) {
+	if (strncmp(cmp, "no", 2) == 0) {
 		neg = 1;
 		cmp += 2;
 	}
 
 	for (i = 0; trace_options[i]; i++) {
-		int len = strlen(trace_options[i]);
-
-		if (strncmp(cmp, trace_options[i], len) == 0) {
+		if (strcmp(cmp, trace_options[i]) == 0) {
 			set_tracer_flags(1 << i, !neg);
 			break;
 		}
@@ -3888,39 +3888,16 @@ trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	if (ret < 0)
 		return ret;
 
-	ret = 0;
-	switch (val) {
-	case 0:
-		/* do nothing if already cleared */
-		if (!(topt->flags->val & topt->opt->bit))
-			break;
-
-		mutex_lock(&trace_types_lock);
-		if (current_trace->set_flag)
-			ret = current_trace->set_flag(topt->flags->val,
-						      topt->opt->bit, 0);
-		mutex_unlock(&trace_types_lock);
-		if (ret)
-			return ret;
-		topt->flags->val &= ~topt->opt->bit;
-		break;
-	case 1:
-		/* do nothing if already set */
-		if (topt->flags->val & topt->opt->bit)
-			break;
+	if (val != 0 && val != 1)
+		return -EINVAL;
 
+	if (!!(topt->flags->val & topt->opt->bit) != val) {
 		mutex_lock(&trace_types_lock);
-		if (current_trace->set_flag)
-			ret = current_trace->set_flag(topt->flags->val,
-						      topt->opt->bit, 1);
+		ret = __set_tracer_option(current_trace, topt->flags,
+					  topt->opt, val);
 		mutex_unlock(&trace_types_lock);
 		if (ret)
 			return ret;
-		topt->flags->val |= topt->opt->bit;
-		break;
-
-	default:
-		return -EINVAL;
 	}
 
 	*ppos += cnt;
-- 
1.6.3

[PATCH 11/13] tracing: Change event->profile_count to be int type

[Li Zefan]

Like total_profile_count, struct ftrace_event_call::profile_count
is protected by event_mutex, so it doesn't need to be atomic_t.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 include/linux/ftrace_event.h       |    2 +-
 include/linux/syscalls.h           |    2 --
 include/trace/ftrace.h             |    1 -
 kernel/trace/trace_event_profile.c |    6 +++---
 kernel/trace/trace_kprobe.c        |    1 -
 5 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index f883406..52b88d4 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -130,7 +130,7 @@ struct ftrace_event_call {
 	void			*mod;
 	void			*data;
 
-	atomic_t		profile_count;
+	int			profile_count;
 	int			(*profile_enable)(struct ftrace_event_call *);
 	void			(*profile_disable)(struct ftrace_event_call *);
 };
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 55e7108..960567f 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -101,12 +101,10 @@ struct perf_event_attr;
 #ifdef CONFIG_EVENT_PROFILE
 
 #define TRACE_SYS_ENTER_PROFILE_INIT(sname)				       \
-	.profile_count = ATOMIC_INIT(-1),				       \
 	.profile_enable = prof_sysenter_enable,				       \
 	.profile_disable = prof_sysenter_disable,
 
 #define TRACE_SYS_EXIT_PROFILE_INIT(sname)				       \
-	.profile_count = ATOMIC_INIT(-1),				       \
 	.profile_enable = prof_sysexit_enable,				       \
 	.profile_disable = prof_sysexit_disable,
 #else
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 0c21af8..7352315 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -629,7 +629,6 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
 #ifdef CONFIG_EVENT_PROFILE
 
 #define _TRACE_PROFILE_INIT(call)					\
-	.profile_count = ATOMIC_INIT(-1),				\
 	.profile_enable = ftrace_profile_enable_##call,			\
 	.profile_disable = ftrace_profile_disable_##call,
 
diff --git a/kernel/trace/trace_event_profile.c b/kernel/trace/trace_event_profile.c
index d9c60f8..9e25573 100644
--- a/kernel/trace/trace_event_profile.c
+++ b/kernel/trace/trace_event_profile.c
@@ -25,7 +25,7 @@ static int ftrace_profile_enable_event(struct ftrace_event_call *event)
 	char *buf;
 	int ret = -ENOMEM;
 
-	if (atomic_inc_return(&event->profile_count))
+	if (event->profile_count++ > 0)
 		return 0;
 
 	if (!total_profile_count) {
@@ -56,7 +56,7 @@ fail_buf_nmi:
 		perf_trace_buf = NULL;
 	}
 fail_buf:
-	atomic_dec(&event->profile_count);
+	event->profile_count--;
 
 	return ret;
 }
@@ -83,7 +83,7 @@ static void ftrace_profile_disable_event(struct ftrace_event_call *event)
 {
 	char *buf, *nmi_buf;
 
-	if (!atomic_add_negative(-1, &event->profile_count))
+	if (--event->profile_count > 0)
 		return;
 
 	event->profile_disable(event);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e3c80e9..6ed2234 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1426,7 +1426,6 @@ static int register_probe_event(struct trace_probe *tp)
 	call->unregfunc = probe_event_disable;
 
 #ifdef CONFIG_EVENT_PROFILE
-	atomic_set(&call->profile_count, -1);
 	call->profile_enable = probe_profile_enable;
 	call->profile_disable = probe_profile_disable;
 #endif
-- 
1.6.3

[PATCH 12/13] tracing/power: Remove two exports

[Li Zefan]

trace_power_start and trace_power_end are used in
arch/x86/kernel/power.c, and this file can't be compiled
as a module, so these two tracepoints needn't to be
exported.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Arjan van de Ven <arjan@linux.intel.com>
---
 kernel/trace/power-traces.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/power-traces.c b/kernel/trace/power-traces.c
index e06c6e3..9f4f565 100644
--- a/kernel/trace/power-traces.c
+++ b/kernel/trace/power-traces.c
@@ -14,7 +14,5 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/power.h>
 
-EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
-EXPORT_TRACEPOINT_SYMBOL_GPL(power_end);
 EXPORT_TRACEPOINT_SYMBOL_GPL(power_frequency);
 
-- 
1.6.3

[PATCH 13/13] ksym_tracer: Fix compile warnings

[Li Zefan]

Fix these 2 warnings:

kernel/trace/trace_ksym.c: In function 'ksym_trace_filter_read':
kernel/trace/trace_ksym.c:239: warning: cast to pointer from integer of different size
kernel/trace/trace_ksym.c: In function 'ksym_trace_filter_write':
kernel/trace/trace_ksym.c:295: warning: ignoring return value of 'strstrip', declared with attribute warn_unused_result

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace_ksym.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index acb87d4..e393146 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -236,7 +236,8 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
 	mutex_lock(&ksym_tracer_mutex);
 
 	hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
-		ret = trace_seq_printf(s, "%pS:", (void *)entry->attr.bp_addr);
+		ret = trace_seq_printf(s, "%pS:",
+				(void *)(unsigned long)entry->attr.bp_addr);
 		if (entry->attr.bp_type == HW_BREAKPOINT_R)
 			ret = trace_seq_puts(s, "r--\n");
 		else if (entry->attr.bp_type == HW_BREAKPOINT_W)
@@ -278,7 +279,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
 {
 	struct trace_ksym *entry;
 	struct hlist_node *node;
-	char *input_string, *ksymname = NULL;
+	char *input_string, *buf, *ksymname = NULL;
 	unsigned long ksym_addr = 0;
 	int ret, op, changed = 0;
 
@@ -292,7 +293,7 @@ static ssize_t ksym_trace_filter_write(struct file *file,
 	}
 	input_string[count] = '\0';
 
-	strstrip(input_string);
+	buf = strstrip(input_string);
 
 	/*
 	 * Clear all breakpoints if:
@@ -300,14 +301,14 @@ static ssize_t ksym_trace_filter_write(struct file *file,
 	 * 2: echo 0 > ksym_trace_filter
 	 * 3: echo "*:---" > ksym_trace_filter
 	 */
-	if (!input_string[0] || !strcmp(input_string, "0") ||
-	    !strcmp(input_string, "*:---")) {
+	if (!buf[0] || !strcmp(buf, "0") ||
+	    !strcmp(buf, "*:---")) {
 		__ksym_trace_reset();
 		kfree(input_string);
 		return count;
 	}
 
-	ret = op = parse_ksym_trace_str(input_string, &ksymname, &ksym_addr);
+	ret = op = parse_ksym_trace_str(buf, &ksymname, &ksym_addr);
 	if (ret < 0) {
 		kfree(input_string);
 		return ret;
-- 
1.6.3

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Lai Jiangshan]

Li Zefan wrote:
> Call trace_define_common_fields() in event_create_dir() only.
> 
>    text    data     bss     dec     hex filename
> 5346802 1961864 7103260 14411926         dbe896 vmlinux.o.old
> 5345151 1961864 7103260 14410275         dbe223 vmlinux.o
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

I think this is a urgent patch for mainline.

I have found a regression at trace_kprobe, and I'm lucky to find that this
patch can also fix this regression.


> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
>  	struct kprobe_trace_entry field;
>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
>  
> -	ret = trace_define_common_fields(event_call);
> -	if (!ret)
> -		return ret;
> -
>  	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
>  	/* Set argument names as fields */
> @@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
>  	struct kretprobe_trace_entry field;
>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
>  
> -	ret = trace_define_common_fields(event_call);
> -	if (!ret)
> -		return ret;
> -
>  	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
>  	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);

"if (!ret)" is wrong. trace_define_common_fields() returns zero when success.
So "unsigned long, func", "unsigned long, ret_ip" ...etc are NOT "defined"
and filters can not be applied for trace_kprobe.

Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Re: [PATCH 01/13] tracing: Extract duplicate ftrace_raw_init_event_foo()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 11:14:20AM +0800, Li Zefan wrote:
> Define ftrace_raw_init_event_foo() for each event class rather
> than for each event:
> 
>    text    data     bss     dec     hex filename
> 5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
> 5346802 1961864 7103260 14411926         dbe896 vmlinux.o
> 
> raw_init can't be removed, because ftrace events and kprobe events
> use different raw_init callbacks. Though it's possible to totally
> remove raw_init, I choose to leave it as it is for now.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  include/linux/ftrace_event.h |    1 +
>  include/linux/syscalls.h     |    4 ++--
>  include/trace/ftrace.h       |   35 ++++-------------------------------
>  kernel/trace/trace_events.c  |   14 ++++++++++++++
>  4 files changed, 21 insertions(+), 33 deletions(-)
> 
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 47bbdf9..cdea8a9 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -157,6 +157,7 @@ enum {
>  	FILTER_PTR_STRING,
>  };
>  
> +extern int trace_event_raw_init(struct ftrace_event_call *call);
>  extern int trace_define_common_fields(struct ftrace_event_call *call);
>  extern int trace_define_field(struct ftrace_event_call *call, const char *type,
>  			      const char *name, int offset, int size,
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e79e2f3..55e7108 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -144,7 +144,7 @@ struct perf_event_attr;
>  		.name                   = "sys_enter"#sname,		\
>  		.system                 = "syscalls",			\
>  		.event                  = &enter_syscall_print_##sname,	\
> -		.raw_init		= init_syscall_trace,		\
> +		.raw_init		= trace_event_raw_init,		\
>  		.show_format		= syscall_enter_format,		\
>  		.define_fields		= syscall_enter_define_fields,	\
>  		.regfunc		= reg_event_syscall_enter,	\
> @@ -166,7 +166,7 @@ struct perf_event_attr;
>  		.name                   = "sys_exit"#sname,		\
>  		.system                 = "syscalls",			\
>  		.event                  = &exit_syscall_print_##sname,	\
> -		.raw_init		= init_syscall_trace,		\
> +		.raw_init		= trace_event_raw_init,		\



Nice cleanup.
BTW, init_syscall_trace is left unused now. Could you please
remove it too?

And add my Acked-by. Thanks!

Re: [PATCH 03/13] tracing: Move a printk out of ftrace_raw_reg_event_foo()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 11:14:52AM +0800, Li Zefan wrote:
> Move the printk from each ftrace_raw_reg_event_foo() to
> its caller ftrace_event_enable_disable().
> 
> See how much space this saves:
> 
>    text    data     bss     dec     hex filename
> 5345151 1961864 7103260 14410275         dbe223 vmlinux.o.old
> 5331487 1961864 7103260 14396611         dbacc3 vmlinux.o
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  include/trace/ftrace.h        |   16 ++--------------
>  kernel/trace/trace_events.c   |   17 +++++++++++++----
>  kernel/trace/trace_syscalls.c |   10 ++--------
>  3 files changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index f978376..0c21af8 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -555,13 +555,7 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
>   *
>   * static int ftrace_reg_event_<call>(struct ftrace_event_call *unused)
>   * {
> - *	int ret;
> - *
> - *	ret = register_trace_<call>(ftrace_event_<call>);
> - *	if (!ret)
> - *		pr_info("event trace: Could not activate trace point "
> - *			"probe to  <call>");
> - *	return ret;
> + *	return register_trace_<call>(ftrace_event_<call>);
>   * }
>   *
>   * static void ftrace_unreg_event_<call>(struct ftrace_event_call *unused)
> @@ -710,13 +704,7 @@ static void ftrace_raw_event_##call(proto)				\
>  									\
>  static int ftrace_raw_reg_event_##call(struct ftrace_event_call *unused)\
>  {									\
> -	int ret;							\
> -									\
> -	ret = register_trace_##call(ftrace_raw_event_##call);		\
> -	if (ret)							\
> -		pr_info("event trace: Could not activate trace point "	\
> -			"probe to " #call "\n");			\
> -	return ret;							\
> +	return register_trace_##call(ftrace_raw_event_##call);		\
>  }									\
>  									\
>  static void ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused)\
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 97b0b3a..bf2f1b9 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -118,9 +118,11 @@ int trace_event_raw_init(struct ftrace_event_call *call)
>  }
>  EXPORT_SYMBOL_GPL(trace_event_raw_init);
>  
> -static void ftrace_event_enable_disable(struct ftrace_event_call *call,
> +static int ftrace_event_enable_disable(struct ftrace_event_call *call,
>  					int enable)
>  {
> +	int ret = 0;
> +
>  	switch (enable) {
>  	case 0:
>  		if (call->enabled) {
> @@ -131,12 +133,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
>  		break;
>  	case 1:
>  		if (!call->enabled) {
> +			ret = call->regfunc(call);
> +			if (ret) {
> +				pr_info("event trace: Could not enable event "
> +					"%s\n", call->name);
> +				break;
> +			}
>  			call->enabled = 1;
>  			tracing_start_cmdline_record();
> -			call->regfunc(call);



That seems to open a tiny window during which we'll lose
some pid -> comm resolution records.

We should probably keep the previous call order.

Other than that:

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Thanks.

Re: [PATCH 03/13] tracing: Move a printk out of ftrace_raw_reg_event_foo()

[Li Zefan]

>> -static void ftrace_event_enable_disable(struct ftrace_event_call *call,
>> +static int ftrace_event_enable_disable(struct ftrace_event_call *call,
>>  					int enable)
>>  {
>> +	int ret = 0;
>> +
>>  	switch (enable) {
>>  	case 0:
>>  		if (call->enabled) {
>> @@ -131,12 +133,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
>>  		break;
>>  	case 1:
>>  		if (!call->enabled) {
>> +			ret = call->regfunc(call);
>> +			if (ret) {
>> +				pr_info("event trace: Could not enable event "
>> +					"%s\n", call->name);
>> +				break;
>> +			}
>>  			call->enabled = 1;
>>  			tracing_start_cmdline_record();
>> -			call->regfunc(call);
> 
> That seems to open a tiny window during which we'll lose
> some pid -> comm resolution records.
> 
> We should probably keep the previous call order.
> 

Hm, actually the tiny window is already there in that
tracing_stop_cmdline_record() is called before call->unregfunc()..

So I can send a fix-up patch later.

> Other than that:
> 
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 

Thanks.

Re: [PATCH 01/13] tracing: Extract duplicate ftrace_raw_init_event_foo()

[Li Zefan]

>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index e79e2f3..55e7108 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -144,7 +144,7 @@ struct perf_event_attr;
>>  		.name                   = "sys_enter"#sname,		\
>>  		.system                 = "syscalls",			\
>>  		.event                  = &enter_syscall_print_##sname,	\
>> -		.raw_init		= init_syscall_trace,		\
>> +		.raw_init		= trace_event_raw_init,		\
>>  		.show_format		= syscall_enter_format,		\
>>  		.define_fields		= syscall_enter_define_fields,	\
>>  		.regfunc		= reg_event_syscall_enter,	\
>> @@ -166,7 +166,7 @@ struct perf_event_attr;
>>  		.name                   = "sys_exit"#sname,		\
>>  		.system                 = "syscalls",			\
>>  		.event                  = &exit_syscall_print_##sname,	\
>> -		.raw_init		= init_syscall_trace,		\
>> +		.raw_init		= trace_event_raw_init,		\
> 
> 
> 
> Nice cleanup.
> BTW, init_syscall_trace is left unused now. Could you please
> remove it too?
> 

I'd like to see the whole patchset be accepted first. I don't
won't to spam LKML with another 14 emails. ;)

Though I can just resend this patch, Ingo might be confused
which patch he should take.

btw, Lai Jiangshan just told me he has a pending patch that
will modify init_syscall_trace, so init_syscall_trace will
come back to use again.

> And add my Acked-by. Thanks!
> 

Thanks!

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 01:36:20PM +0800, Lai Jiangshan wrote:
> Li Zefan wrote:
> > Call trace_define_common_fields() in event_create_dir() only.
> > 
> >    text    data     bss     dec     hex filename
> > 5346802 1961864 7103260 14411926         dbe896 vmlinux.o.old
> > 5345151 1961864 7103260 14410275         dbe223 vmlinux.o
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> I think this is a urgent patch for mainline.
> 
> I have found a regression at trace_kprobe, and I'm lucky to find that this
> patch can also fix this regression.
> 
> 
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
> >  	struct kprobe_trace_entry field;
> >  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
> >  
> > -	ret = trace_define_common_fields(event_call);
> > -	if (!ret)
> > -		return ret;
> > -
> >  	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
> >  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
> >  	/* Set argument names as fields */
> > @@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
> >  	struct kretprobe_trace_entry field;
> >  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
> >  
> > -	ret = trace_define_common_fields(event_call);
> > -	if (!ret)
> > -		return ret;
> > -
> >  	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
> >  	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
> >  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
> 
> "if (!ret)" is wrong. trace_define_common_fields() returns zero when success.
> So "unsigned long, func", "unsigned long, ret_ip" ...etc are NOT "defined"
> and filters can not be applied for trace_kprobe.
> 
> Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> 


Oh right!

Ok, I'm queueing this one for the tracing fixes.
Thanks!

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Li Zefan]

>>> --- a/kernel/trace/trace_kprobe.c
>>> +++ b/kernel/trace/trace_kprobe.c
>>> @@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
>>>  	struct kprobe_trace_entry field;
>>>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
>>>  
>>> -	ret = trace_define_common_fields(event_call);
>>> -	if (!ret)
>>> -		return ret;
>>> -
>>>  	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
>>>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
>>>  	/* Set argument names as fields */
>>> @@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
>>>  	struct kretprobe_trace_entry field;
>>>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
>>>  
>>> -	ret = trace_define_common_fields(event_call);
>>> -	if (!ret)
>>> -		return ret;
>>> -
>>>  	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
>>>  	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
>>>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
>> "if (!ret)" is wrong. trace_define_common_fields() returns zero when success.
>> So "unsigned long, func", "unsigned long, ret_ip" ...etc are NOT "defined"
>> and filters can not be applied for trace_kprobe.
>>
>> Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>>
> 
> Oh right!
> 
> Ok, I'm queueing this one for the tracing fixes.
> Thanks!
> 

This patch has dependency on the previous patch..So can you queue
all other patches and send a pull request to Ingo? We're still
in the early merge window, so those patches should be fine for
.33.

And as this patch fixes a bug "acidentally", the changelog
needs a bit revision.

Re: [PATCH 01/13] tracing: Extract duplicate ftrace_raw_init_event_foo()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 03:49:29PM +0800, Li Zefan wrote:
> >> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> >> index e79e2f3..55e7108 100644
> >> --- a/include/linux/syscalls.h
> >> +++ b/include/linux/syscalls.h
> >> @@ -144,7 +144,7 @@ struct perf_event_attr;
> >>  		.name                   = "sys_enter"#sname,		\
> >>  		.system                 = "syscalls",			\
> >>  		.event                  = &enter_syscall_print_##sname,	\
> >> -		.raw_init		= init_syscall_trace,		\
> >> +		.raw_init		= trace_event_raw_init,		\
> >>  		.show_format		= syscall_enter_format,		\
> >>  		.define_fields		= syscall_enter_define_fields,	\
> >>  		.regfunc		= reg_event_syscall_enter,	\
> >> @@ -166,7 +166,7 @@ struct perf_event_attr;
> >>  		.name                   = "sys_exit"#sname,		\
> >>  		.system                 = "syscalls",			\
> >>  		.event                  = &exit_syscall_print_##sname,	\
> >> -		.raw_init		= init_syscall_trace,		\
> >> +		.raw_init		= trace_event_raw_init,		\
> > 
> > 
> > 
> > Nice cleanup.
> > BTW, init_syscall_trace is left unused now. Could you please
> > remove it too?
> > 
> 
> I'd like to see the whole patchset be accepted first. I don't
> won't to spam LKML with another 14 emails. ;)



You can just answer the pervious patch with a new one, putting
a v2 prefix in the title :)


 
> Though I can just resend this patch, Ingo might be confused
> which patch he should take.
> 
> btw, Lai Jiangshan just told me he has a pending patch that
> will modify init_syscall_trace, so init_syscall_trace will
> come back to use again.


Ah ok.

Thanks.

Re: [PATCH 03/13] tracing: Move a printk out of ftrace_raw_reg_event_foo()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 03:49:27PM +0800, Li Zefan wrote:
> >> -static void ftrace_event_enable_disable(struct ftrace_event_call *call,
> >> +static int ftrace_event_enable_disable(struct ftrace_event_call *call,
> >>  					int enable)
> >>  {
> >> +	int ret = 0;
> >> +
> >>  	switch (enable) {
> >>  	case 0:
> >>  		if (call->enabled) {
> >> @@ -131,12 +133,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
> >>  		break;
> >>  	case 1:
> >>  		if (!call->enabled) {
> >> +			ret = call->regfunc(call);
> >> +			if (ret) {
> >> +				pr_info("event trace: Could not enable event "
> >> +					"%s\n", call->name);
> >> +				break;
> >> +			}
> >>  			call->enabled = 1;
> >>  			tracing_start_cmdline_record();
> >> -			call->regfunc(call);
> > 
> > That seems to open a tiny window during which we'll lose
> > some pid -> comm resolution records.
> > 
> > We should probably keep the previous call order.
> > 
> 
> Hm, actually the tiny window is already there in that
> tracing_stop_cmdline_record() is called before call->unregfunc()..
> 
> So I can send a fix-up patch later.


The presence of a previous race window doesn't justify we add
a new one :)

I'll fix that while applying the patch.

Thanks.

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 04:03:58PM +0800, Li Zefan wrote:
> >>> --- a/kernel/trace/trace_kprobe.c
> >>> +++ b/kernel/trace/trace_kprobe.c
> >>> @@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
> >>>  	struct kprobe_trace_entry field;
> >>>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
> >>>  
> >>> -	ret = trace_define_common_fields(event_call);
> >>> -	if (!ret)
> >>> -		return ret;
> >>> -
> >>>  	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
> >>>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
> >>>  	/* Set argument names as fields */
> >>> @@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
> >>>  	struct kretprobe_trace_entry field;
> >>>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
> >>>  
> >>> -	ret = trace_define_common_fields(event_call);
> >>> -	if (!ret)
> >>> -		return ret;
> >>> -
> >>>  	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
> >>>  	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
> >>>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
> >> "if (!ret)" is wrong. trace_define_common_fields() returns zero when success.
> >> So "unsigned long, func", "unsigned long, ret_ip" ...etc are NOT "defined"
> >> and filters can not be applied for trace_kprobe.
> >>
> >> Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> >>
> > 
> > Oh right!
> > 
> > Ok, I'm queueing this one for the tracing fixes.
> > Thanks!
> > 
> 
> This patch has dependency on the previous patch..So can you queue
> all other patches and send a pull request to Ingo? We're still
> in the early merge window, so those patches should be fine for
> .33.
> 
> And as this patch fixes a bug "acidentally", the changelog
> needs a bit revision.


The problem is that we can't know in advance Linus will take
a second round of tracing features in this merge window.

I'd rather be careful and keep separating tracing fixes and
tracing features.

I'm preparing a separate fix.

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Li Zefan]

>>>>> @@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
>>>>>  	struct kretprobe_trace_entry field;
>>>>>  	struct trace_probe *tp = (struct trace_probe *)event_call->data;
>>>>>  
>>>>> -	ret = trace_define_common_fields(event_call);
>>>>> -	if (!ret)
>>>>> -		return ret;
>>>>> -
>>>>>  	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
>>>>>  	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
>>>>>  	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
>>>> "if (!ret)" is wrong. trace_define_common_fields() returns zero when success.
>>>> So "unsigned long, func", "unsigned long, ret_ip" ...etc are NOT "defined"
>>>> and filters can not be applied for trace_kprobe.
>>>>
>>>> Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>>>>
>>> Oh right!
>>>
>>> Ok, I'm queueing this one for the tracing fixes.
>>> Thanks!
>>>
>> This patch has dependency on the previous patch..So can you queue
>> all other patches and send a pull request to Ingo? We're still
>> in the early merge window, so those patches should be fine for
>> .33.
>>
>> And as this patch fixes a bug "acidentally", the changelog
>> needs a bit revision.
> 
> The problem is that we can't know in advance Linus will take
> a second round of tracing features in this merge window.
> 
> I'd rather be careful and keep separating tracing fixes and
> tracing features.
> 
> I'm preparing a separate fix.
> 

But there is no functionality change or new feature in this
patchset, all are cleanups and fixes.

That said, I don't mind how you separate them. :)

Re: [PATCH 02/13] tracing: Extract calls to trace_define_common_fields()

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 04:21:14PM +0800, Li Zefan wrote:
> > The problem is that we can't know in advance Linus will take
> > a second round of tracing features in this merge window.
> > 
> > I'd rather be careful and keep separating tracing fixes and
> > tracing features.
> > 
> > I'm preparing a separate fix.
> > 
> 
> But there is no functionality change or new feature in this
> patchset, all are cleanups and fixes.


They are not _supposed_ to have functionality change :)
They are mostly cleanups, reorganization, better handling
of interfaces, etc...

All that is in the family of "features". They are improvements
that could possibly induce new bugs.

OTOH, pure fixes are only surgical patches and should not
expand their scope to a reorg or a code cleanup.

Well, it depends, I'm not preaching hard rules. But the
point is we never know if Linus will take another round
of the first category of patches in this merge window.
So it's better to keep pure fixes isolated, at least
they have good chances to keep beeing eligible
after -rc1.

Re: [PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 11:15:30AM +0800, Li Zefan wrote:
> I found a weird behavior:
> 
>   # echo 'fuse:*' > set_ftrace_filter
>   bash: echo: write error: Invalid argument
>   # cat set_ftrace_filter
>   fuse_dev_fasync
>   fuse_dev_poll
>   fuse_copy_do
> 
> We should call trace_parser_clear() no matter ftrace_process_regex()
> returns 0 or -errno.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  kernel/trace/ftrace.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 08a3fb5..ff8aecd 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2208,10 +2208,9 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
>  	    !trace_parser_cont(parser)) {
>  		ret = ftrace_process_regex(parser->buffer,
>  					   parser->idx, enable);
> +		trace_parser_clear(parser);
>  		if (ret)
>  			goto out_unlock;
> -
> -		trace_parser_clear(parser);
>  	}


I'm missing something. How that can happen. Anytime we reopen
the file, the parser is re-allocated.

I guess that happens if you open in rw mode? But not using the
example in the changelog?

Re: [PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Li Zefan]

Frederic Weisbecker wrote:
> On Tue, Dec 08, 2009 at 11:15:30AM +0800, Li Zefan wrote:
>> I found a weird behavior:
>>
>>   # echo 'fuse:*' > set_ftrace_filter
>>   bash: echo: write error: Invalid argument
>>   # cat set_ftrace_filter
>>   fuse_dev_fasync
>>   fuse_dev_poll
>>   fuse_copy_do
>>
>> We should call trace_parser_clear() no matter ftrace_process_regex()
>> returns 0 or -errno.
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> ---
>>  kernel/trace/ftrace.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 08a3fb5..ff8aecd 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -2208,10 +2208,9 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
>>  	    !trace_parser_cont(parser)) {
>>  		ret = ftrace_process_regex(parser->buffer,
>>  					   parser->idx, enable);
>> +		trace_parser_clear(parser);
>>  		if (ret)
>>  			goto out_unlock;
>> -
>> -		trace_parser_clear(parser);
>>  	}
> 
> 
> I'm missing something. How that can happen. Anytime we reopen
> the file, the parser is re-allocated.
> 

It happened at file closing..

static int
ftrace_regex_release(struct inode *inode, struct file *file, int enable)
{
	...
        parser = &iter->parser;
        if (trace_parser_loaded(parser)) {
                parser->buffer[parser->idx] = 0;
		/* here ! */
                ftrace_match_records(parser->buffer, parser->idx, enable);
        }
	...
}

> I guess that happens if you open in rw mode? But not using the
> example in the changelog?

I've confirmed that example I was using can reveal this bug.

Re: [PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 06:03:46PM +0800, Li Zefan wrote:
> Frederic Weisbecker wrote:
> > I'm missing something. How that can happen. Anytime we reopen
> > the file, the parser is re-allocated.
> > 
> 
> It happened at file closing..
> 
> static int
> ftrace_regex_release(struct inode *inode, struct file *file, int enable)
> {
> 	...
>         parser = &iter->parser;
>         if (trace_parser_loaded(parser)) {
>                 parser->buffer[parser->idx] = 0;
> 		/* here ! */
>                 ftrace_match_records(parser->buffer, parser->idx, enable);
>         }
> 	...
> }


Aah, ok.

 
> > I guess that happens if you open in rw mode? But not using the
> > example in the changelog?
> 
> I've confirmed that example I was using can reveal this bug.

Yeah indeed.

Re: [PATCH 00/13] tracing: various cleanups and small fixes (updated)

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 11:13:58AM +0800, Li Zefan wrote:
> Updates:
> 
> - took a further step to save more spaces (01/13)
> - fixed a mistake in (03/13)
> - collected some ACKs
> 
> 
> Here are some patches that I've collected. All are cleanups and
> small fixes, no change in functionality.
> 
> As a result, 24K bytes are saved, and 87 lines of code are
> reduced:
> 
>    text    data     bss     dec     hex filename
> 5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
> 5331208 1961864 7103260 14396332         dbabac vmlinux.o
> 
> ---
>  include/linux/ftrace_event.h       |    4 +-
>  include/linux/syscalls.h           |    6 +-
>  include/trace/ftrace.h             |   56 ++-----------
>  kernel/trace/ftrace.c              |   30 ++++---
>  kernel/trace/power-traces.c        |    2 -
>  kernel/trace/trace.c               |  166 +++++++++++++-----------------------
>  kernel/trace/trace.h               |   23 +++---
>  kernel/trace/trace_event_profile.c |    6 +-
>  kernel/trace/trace_events.c        |   38 +++++++--
>  kernel/trace/trace_export.c        |    4 -
>  kernel/trace/trace_kprobe.c        |    9 --
>  kernel/trace/trace_ksym.c          |   13 ++--
>  kernel/trace/trace_syscalls.c      |   18 +----
>  13 files changed, 144 insertions(+), 231 deletions(-)


I'm queuing these patches.

Thanks.

Re: [PATCH 01/13] tracing: Extract duplicate ftrace_raw_init_event_foo()

[Steven Rostedt]

On Tue, 2009-12-08 at 15:49 +0800, Li Zefan wrote:

> > 
> > Nice cleanup.
> > BTW, init_syscall_trace is left unused now. Could you please
> > remove it too?
> > 
> 
> I'd like to see the whole patchset be accepted first. I don't
> won't to spam LKML with another 14 emails. ;)

Yeah, I agree. The removing of that can be a later patch.

I'll allocate time today to review this series, and if I don't come up
with anything major, I'll pull it in.

Thanks!

-- Steve

> 
> Though I can just resend this patch, Ingo might be confused
> which patch he should take.
> 
> btw, Lai Jiangshan just told me he has a pending patch that
> will modify init_syscall_trace, so init_syscall_trace will
> come back to use again.
> 
> > And add my Acked-by. Thanks!
> > 
> 
> Thanks!

Re: [PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Steven Rostedt]

On Tue, 2009-12-08 at 18:03 +0800, Li Zefan wrote:

> > 
> > I'm missing something. How that can happen. Anytime we reopen
> > the file, the parser is re-allocated.
> > 
> 
> It happened at file closing..
> 
> static int
> ftrace_regex_release(struct inode *inode, struct file *file, int enable)
> {
> 	...
>         parser = &iter->parser;
>         if (trace_parser_loaded(parser)) {
>                 parser->buffer[parser->idx] = 0;
> 		/* here ! */
>                 ftrace_match_records(parser->buffer, parser->idx, enable);
>         }
> 	...
> }
> 
> > I guess that happens if you open in rw mode? But not using the
> > example in the changelog?
> 
> I've confirmed that example I was using can reveal this bug.

This explanation should have been in the change log. I'll add it.

Thanks,

-- Steve

Re: [PATCH 00/13] tracing: various cleanups and small fixes (updated)

[Steven Rostedt]

On Tue, 2009-12-08 at 13:23 +0100, Frederic Weisbecker wrote:
> On Tue, Dec 08, 2009 at 11:13:58AM +0800, Li Zefan wrote:
> > Updates:
> > 
> > - took a further step to save more spaces (01/13)
> > - fixed a mistake in (03/13)
> > - collected some ACKs
> > 
> > 
> > Here are some patches that I've collected. All are cleanups and
> > small fixes, no change in functionality.
> > 
> > As a result, 24K bytes are saved, and 87 lines of code are
> > reduced:
> > 
> >    text    data     bss     dec     hex filename
> > 5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
> > 5331208 1961864 7103260 14396332         dbabac vmlinux.o
> > 
> > ---
> >  include/linux/ftrace_event.h       |    4 +-
> >  include/linux/syscalls.h           |    6 +-
> >  include/trace/ftrace.h             |   56 ++-----------
> >  kernel/trace/ftrace.c              |   30 ++++---
> >  kernel/trace/power-traces.c        |    2 -
> >  kernel/trace/trace.c               |  166 +++++++++++++-----------------------
> >  kernel/trace/trace.h               |   23 +++---
> >  kernel/trace/trace_event_profile.c |    6 +-
> >  kernel/trace/trace_events.c        |   38 +++++++--
> >  kernel/trace/trace_export.c        |    4 -
> >  kernel/trace/trace_kprobe.c        |    9 --
> >  kernel/trace/trace_ksym.c          |   13 ++--
> >  kernel/trace/trace_syscalls.c      |   18 +----
> >  13 files changed, 144 insertions(+), 231 deletions(-)
> 
> 
> I'm queuing these patches.

I don't mind you queing them, but I still want to have a deeper look.
Could you hold on a bit. I'll look at them today, and give an ack, or
comments later on.

Thanks,

-- Steve

Re: [PATCH 00/13] tracing: various cleanups and small fixes (updated)

[Frederic Weisbecker]

2009/12/8 Steven Rostedt <rostedt@goodmis.org>:
> On Tue, 2009-12-08 at 13:23 +0100, Frederic Weisbecker wrote:
>> On Tue, Dec 08, 2009 at 11:13:58AM +0800, Li Zefan wrote:
>> > Updates:
>> >
>> > - took a further step to save more spaces (01/13)
>> > - fixed a mistake in (03/13)
>> > - collected some ACKs
>> >
>> >
>> > Here are some patches that I've collected. All are cleanups and
>> > small fixes, no change in functionality.
>> >
>> > As a result, 24K bytes are saved, and 87 lines of code are
>> > reduced:
>> >
>> >    text    data     bss     dec     hex filename
>> > 5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
>> > 5331208 1961864 7103260 14396332         dbabac vmlinux.o
>> >
>> > ---
>> >  include/linux/ftrace_event.h       |    4 +-
>> >  include/linux/syscalls.h           |    6 +-
>> >  include/trace/ftrace.h             |   56 ++-----------
>> >  kernel/trace/ftrace.c              |   30 ++++---
>> >  kernel/trace/power-traces.c        |    2 -
>> >  kernel/trace/trace.c               |  166 +++++++++++++-----------------------
>> >  kernel/trace/trace.h               |   23 +++---
>> >  kernel/trace/trace_event_profile.c |    6 +-
>> >  kernel/trace/trace_events.c        |   38 +++++++--
>> >  kernel/trace/trace_export.c        |    4 -
>> >  kernel/trace/trace_kprobe.c        |    9 --
>> >  kernel/trace/trace_ksym.c          |   13 ++--
>> >  kernel/trace/trace_syscalls.c      |   18 +----
>> >  13 files changed, 144 insertions(+), 231 deletions(-)
>>
>>
>> I'm queuing these patches.
>
> I don't mind you queing them, but I still want to have a deeper look.
> Could you hold on a bit. I'll look at them today, and give an ack, or
> comments later on.
>
> Thanks,


Sure.

Re: [PATCH 05/13] ftrace: Call trace_parser_clear() properly

[Steven Rostedt]

On Tue, 2009-12-08 at 11:15 +0800, Li Zefan wrote:
> I found a weird behavior:
> 
>   # echo 'fuse:*' > set_ftrace_filter
>   bash: echo: write error: Invalid argument
>   # cat set_ftrace_filter
>   fuse_dev_fasync
>   fuse_dev_poll
>   fuse_copy_do
> 
> We should call trace_parser_clear() no matter ftrace_process_regex()
> returns 0 or -errno.

This confused me at first. I applied this patch and I saw that I still
got the warning, but the functions did not got through. Then I realized
that there was no ":mod:" in that filtering.

Yeah, that was a weird side effect. Good catch!

-- Steve

Re: [PATCH 00/13] tracing: various cleanups and small fixes (updated)

[Steven Rostedt]

On Tue, 2009-12-08 at 11:13 +0800, Li Zefan wrote:
> Updates:
> 
> - took a further step to save more spaces (01/13)
> - fixed a mistake in (03/13)
> - collected some ACKs

Frederic,

I reviewed the entire patch set. You can add my "Acked-by" to all
patches and apply them, with the changes that you said need to be fixed.

Thanks!

-- Steve

[tip:tracing/urgent] tracing: Extract duplicate ftrace_raw_init_event_foo()

[tip-bot for Li Zefan]

Commit-ID:  87d9b4e1c52867a45331a9a5495f6448e0c68b23
Gitweb:     http://git.kernel.org/tip/87d9b4e1c52867a45331a9a5495f6448e0c68b23
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:14:20 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:34:23 +0100

tracing: Extract duplicate ftrace_raw_init_event_foo()

Use a generic trace_event_raw_init() function for all event's raw_init
callbacks (but kprobes) instead of defining the same version for each
of these.
This shrinks the kernel code:

   text    data     bss     dec     hex filename
5355293 1961928 7103260 14420481         dc0a01 vmlinux.o.old
5346802 1961864 7103260 14411926         dbe896 vmlinux.o

raw_init can't be removed, because ftrace events and kprobe events
use different raw_init callbacks. Though it's possible to totally
remove raw_init, I choose to leave it as it is for now.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
LKML-Reference: <4B1DC48C.7080603@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/ftrace_event.h |    1 +
 include/linux/syscalls.h     |    4 ++--
 include/trace/ftrace.h       |   35 ++++-------------------------------
 kernel/trace/trace_events.c  |   14 ++++++++++++++
 4 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 38f8d65..ea44b89 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -158,6 +158,7 @@ enum {
 	FILTER_PTR_STRING,
 };
 
+extern int trace_event_raw_init(struct ftrace_event_call *call);
 extern int trace_define_common_fields(struct ftrace_event_call *call);
 extern int trace_define_field(struct ftrace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index bc70c58..94ac284 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -145,7 +145,7 @@ struct perf_event_attr;
 		.name                   = "sys_enter"#sname,		\
 		.system                 = "syscalls",			\
 		.event                  = &enter_syscall_print_##sname,	\
-		.raw_init		= init_syscall_trace,		\
+		.raw_init		= trace_event_raw_init,		\
 		.show_format		= syscall_enter_format,		\
 		.define_fields		= syscall_enter_define_fields,	\
 		.regfunc		= reg_event_syscall_enter,	\
@@ -167,7 +167,7 @@ struct perf_event_attr;
 		.name                   = "sys_exit"#sname,		\
 		.system                 = "syscalls",			\
 		.event                  = &exit_syscall_print_##sname,	\
-		.raw_init		= init_syscall_trace,		\
+		.raw_init		= trace_event_raw_init,		\
 		.show_format		= syscall_exit_format,		\
 		.define_fields		= syscall_exit_define_fields,	\
 		.regfunc		= reg_event_syscall_exit,	\
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index c4eca38..6055b06 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -623,23 +623,12 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *	.trace			= ftrace_raw_output_<call>, <-- stage 2
  * };
  *
- * static int ftrace_raw_init_event_<call>(struct ftrace_event_call *unused)
- * {
- *	int id;
- *
- *	id = register_ftrace_event(&ftrace_event_type_<call>);
- *	if (!id)
- *		return -ENODEV;
- *	event_<call>.id = id;
- *	return 0;
- * }
- *
  * static struct ftrace_event_call __used
  * __attribute__((__aligned__(4)))
  * __attribute__((section("_ftrace_events"))) event_<call> = {
  *	.name			= "<call>",
  *	.system			= "<system>",
- *	.raw_init		= ftrace_raw_init_event_<call>,
+ *	.raw_init		= trace_event_raw_init,
  *	.regfunc		= ftrace_reg_event_<call>,
  *	.unregfunc		= ftrace_unreg_event_<call>,
  *	.show_format		= ftrace_format_<call>,
@@ -647,9 +636,6 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *
  */
 
-#undef TP_FMT
-#define TP_FMT(fmt, args...)	fmt "\n", ##args
-
 #ifdef CONFIG_EVENT_PROFILE
 
 #define _TRACE_PROFILE_INIT(call)					\
@@ -744,19 +730,7 @@ static void ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused)\
 									\
 static struct trace_event ftrace_event_type_##call = {			\
 	.trace			= ftrace_raw_output_##call,		\
-};									\
-									\
-static int ftrace_raw_init_event_##call(struct ftrace_event_call *unused)\
-{									\
-	int id;								\
-									\
-	id = register_ftrace_event(&ftrace_event_type_##call);		\
-	if (!id)							\
-		return -ENODEV;						\
-	event_##call.id = id;						\
-	INIT_LIST_HEAD(&event_##call.fields);				\
-	return 0;							\
-}
+};
 
 #undef DEFINE_EVENT_PRINT
 #define DEFINE_EVENT_PRINT(template, name, proto, args, print)	\
@@ -776,7 +750,7 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
 	.name			= #call,				\
 	.system			= __stringify(TRACE_SYSTEM),		\
 	.event			= &ftrace_event_type_##call,		\
-	.raw_init		= ftrace_raw_init_event_##call,		\
+	.raw_init		= trace_event_raw_init,			\
 	.regfunc		= ftrace_raw_reg_event_##call,		\
 	.unregfunc		= ftrace_raw_unreg_event_##call,	\
 	.show_format		= ftrace_format_##template,		\
@@ -793,7 +767,7 @@ __attribute__((section("_ftrace_events"))) event_##call = {		\
 	.name			= #call,				\
 	.system			= __stringify(TRACE_SYSTEM),		\
 	.event			= &ftrace_event_type_##call,		\
-	.raw_init		= ftrace_raw_init_event_##call,		\
+	.raw_init		= trace_event_raw_init,			\
 	.regfunc		= ftrace_raw_reg_event_##call,		\
 	.unregfunc		= ftrace_raw_unreg_event_##call,	\
 	.show_format		= ftrace_format_##call,			\
@@ -953,7 +927,6 @@ end:									\
 	perf_swevent_put_recursion_context(rctx);			\
 end_recursion:								\
 	local_irq_restore(irq_flags);					\
-									\
 }
 
 #undef DEFINE_EVENT
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 1d18315..8ed66e0 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -105,6 +105,20 @@ void trace_destroy_fields(struct ftrace_event_call *call)
 	}
 }
 
+int trace_event_raw_init(struct ftrace_event_call *call)
+{
+	int id;
+
+	id = register_ftrace_event(call->event);
+	if (!id)
+		return -ENODEV;
+	call->id = id;
+	INIT_LIST_HEAD(&call->fields);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(trace_event_raw_init);
+
 static void ftrace_event_enable_disable(struct ftrace_event_call *call,
 					int enable)
 {

[tip:tracing/urgent] tracing: Pull up calls to trace_define_common_fields()

[tip-bot for Li Zefan]

Commit-ID:  614a71a26ba3d97e9fa85649db69a682b78e407d
Gitweb:     http://git.kernel.org/tip/614a71a26ba3d97e9fa85649db69a682b78e407d
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:14:36 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:34:23 +0100

tracing: Pull up calls to trace_define_common_fields()

Call trace_define_common_fields() in event_create_dir() only.
This avoids trace events to handle it from their define_fields
callbacks and shrinks the kernel code size:

   text    data     bss     dec     hex filename
5346802 1961864 7103260 14411926         dbe896 vmlinux.o.old
5345151 1961864 7103260 14410275         dbe223 vmlinux.o

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
LKML-Reference: <4B1DC49C.8000107@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/ftrace_event.h  |    1 -
 include/trace/ftrace.h        |    4 ----
 kernel/trace/trace_events.c   |    7 ++++---
 kernel/trace/trace_export.c   |    4 ----
 kernel/trace/trace_kprobe.c   |    8 --------
 kernel/trace/trace_syscalls.c |    8 --------
 6 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index ea44b89..db97c64 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -159,7 +159,6 @@ enum {
 };
 
 extern int trace_event_raw_init(struct ftrace_event_call *call);
-extern int trace_define_common_fields(struct ftrace_event_call *call);
 extern 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);
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 6055b06..2af2f7a 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -436,10 +436,6 @@ ftrace_define_fields_##call(struct ftrace_event_call *event_call)	\
 	struct ftrace_raw_##call field;					\
 	int ret;							\
 									\
-	ret = trace_define_common_fields(event_call);			\
-	if (ret)							\
-		return ret;						\
-									\
 	tstruct;							\
 									\
 	return ret;							\
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 8ed66e0..97b0b3a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -78,7 +78,7 @@ EXPORT_SYMBOL_GPL(trace_define_field);
 	if (ret)							\
 		return ret;
 
-int trace_define_common_fields(struct ftrace_event_call *call)
+static int trace_define_common_fields(struct ftrace_event_call *call)
 {
 	int ret;
 	struct trace_entry ent;
@@ -91,7 +91,6 @@ int trace_define_common_fields(struct ftrace_event_call *call)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(trace_define_common_fields);
 
 void trace_destroy_fields(struct ftrace_event_call *call)
 {
@@ -927,7 +926,9 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 		 		  id);
 
 	if (call->define_fields) {
-		ret = call->define_fields(call);
+		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);
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index dff8c84..458e5bf 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -184,10 +184,6 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call)	\
 	struct struct_name field;					\
 	int ret;							\
 									\
-	ret = trace_define_common_fields(event_call);			\
-	if (ret)							\
-		return ret;						\
-									\
 	tstruct;							\
 									\
 	return ret;							\
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index aff5f80..e3c80e9 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1113,10 +1113,6 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
 	struct kprobe_trace_entry field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
-	ret = trace_define_common_fields(event_call);
-	if (!ret)
-		return ret;
-
 	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
 	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
 	/* Set argument names as fields */
@@ -1131,10 +1127,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
 	struct kretprobe_trace_entry field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
-	ret = trace_define_common_fields(event_call);
-	if (!ret)
-		return ret;
-
 	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
 	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
 	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 57501d9..b957edd 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -217,10 +217,6 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
 	int i;
 	int offset = offsetof(typeof(trace), args);
 
-	ret = trace_define_common_fields(call);
-	if (ret)
-		return ret;
-
 	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
 	if (ret)
 		return ret;
@@ -241,10 +237,6 @@ int syscall_exit_define_fields(struct ftrace_event_call *call)
 	struct syscall_trace_exit trace;
 	int ret;
 
-	ret = trace_define_common_fields(call);
-	if (ret)
-		return ret;
-
 	ret = trace_define_field(call, SYSCALL_FIELD(int, nr), FILTER_OTHER);
 	if (ret)
 		return ret;

[tip:tracing/urgent] tracing: Move a printk out of ftrace_raw_reg_event_foo()

[tip-bot for Li Zefan]

Commit-ID:  3b8e4273814a7f9e9a74ece517d9206fea919aaa
Gitweb:     http://git.kernel.org/tip/3b8e4273814a7f9e9a74ece517d9206fea919aaa
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:14:52 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:25 +0100

tracing: Move a printk out of ftrace_raw_reg_event_foo()

Move the printk from each ftrace_raw_reg_event_foo() to
its caller ftrace_event_enable_disable(). This avoids each
regfunc trace event callbacks to handle a same error report
that can be carried from the caller.

See how much space this saves:

   text    data     bss     dec     hex filename
5345151 1961864 7103260 14410275         dbe223 vmlinux.o.old
5331487 1961864 7103260 14396611         dbacc3 vmlinux.o

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Baron <jbaron@redhat.com>
LKML-Reference: <4B1DC4AC.802@cn.fujitsu.com>
[start cmdline record before calling regfunc to avoid lost
window of pid to comm resolution]
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/trace/ftrace.h        |   16 ++--------------
 kernel/trace/trace_events.c   |   20 +++++++++++++++-----
 kernel/trace/trace_syscalls.c |   10 ++--------
 3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 2af2f7a..0c21af8 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -555,13 +555,7 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
  *
  * static int ftrace_reg_event_<call>(struct ftrace_event_call *unused)
  * {
- *	int ret;
- *
- *	ret = register_trace_<call>(ftrace_event_<call>);
- *	if (!ret)
- *		pr_info("event trace: Could not activate trace point "
- *			"probe to  <call>");
- *	return ret;
+ *	return register_trace_<call>(ftrace_event_<call>);
  * }
  *
  * static void ftrace_unreg_event_<call>(struct ftrace_event_call *unused)
@@ -710,13 +704,7 @@ static void ftrace_raw_event_##call(proto)				\
 									\
 static int ftrace_raw_reg_event_##call(struct ftrace_event_call *unused)\
 {									\
-	int ret;							\
-									\
-	ret = register_trace_##call(ftrace_raw_event_##call);		\
-	if (ret)							\
-		pr_info("event trace: Could not activate trace point "	\
-			"probe to %s\n", #call);			\
-	return ret;							\
+	return register_trace_##call(ftrace_raw_event_##call);		\
 }									\
 									\
 static void ftrace_raw_unreg_event_##call(struct ftrace_event_call *unused)\
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 97b0b3a..189b09b 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -118,9 +118,11 @@ int trace_event_raw_init(struct ftrace_event_call *call)
 }
 EXPORT_SYMBOL_GPL(trace_event_raw_init);
 
-static void ftrace_event_enable_disable(struct ftrace_event_call *call,
+static int ftrace_event_enable_disable(struct ftrace_event_call *call,
 					int enable)
 {
+	int ret = 0;
+
 	switch (enable) {
 	case 0:
 		if (call->enabled) {
@@ -131,12 +133,20 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
 		break;
 	case 1:
 		if (!call->enabled) {
-			call->enabled = 1;
 			tracing_start_cmdline_record();
-			call->regfunc(call);
+			ret = call->regfunc(call);
+			if (ret) {
+				tracing_stop_cmdline_record();
+				pr_info("event trace: Could not enable event "
+					"%s\n", call->name);
+				break;
+			}
+			call->enabled = 1;
 		}
 		break;
 	}
+
+	return ret;
 }
 
 static void ftrace_clear_events(void)
@@ -415,7 +425,7 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	case 0:
 	case 1:
 		mutex_lock(&event_mutex);
-		ftrace_event_enable_disable(call, val);
+		ret = ftrace_event_enable_disable(call, val);
 		mutex_unlock(&event_mutex);
 		break;
 
@@ -425,7 +435,7 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
 
 	*ppos += cnt;
 
-	return cnt;
+	return ret ? ret : cnt;
 }
 
 static ssize_t
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index b957edd..75289f3 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -325,10 +325,7 @@ int reg_event_syscall_enter(struct ftrace_event_call *call)
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_enter)
 		ret = register_trace_sys_enter(ftrace_syscall_enter);
-	if (ret) {
-		pr_info("event trace: Could not activate"
-				"syscall entry trace point");
-	} else {
+	if (!ret) {
 		set_bit(num, enabled_enter_syscalls);
 		sys_refcount_enter++;
 	}
@@ -362,10 +359,7 @@ int reg_event_syscall_exit(struct ftrace_event_call *call)
 	mutex_lock(&syscall_trace_lock);
 	if (!sys_refcount_exit)
 		ret = register_trace_sys_exit(ftrace_syscall_exit);
-	if (ret) {
-		pr_info("event trace: Could not activate"
-				"syscall exit trace point");
-	} else {
+	if (!ret) {
 		set_bit(num, enabled_exit_syscalls);
 		sys_refcount_exit++;
 	}

[tip:tracing/urgent] ftrace: Return EINVAL when writing invalid val to set_ftrace_filter

[tip-bot for Li Zefan]

Commit-ID:  311d16da575f53c3367099579736c1d233efe0dc
Gitweb:     http://git.kernel.org/tip/311d16da575f53c3367099579736c1d233efe0dc
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:15:11 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:25 +0100

ftrace: Return EINVAL when writing invalid val to set_ftrace_filter

Currently it doesn't warn user on invald value:

 # echo nonexist_symbol > set_ftrace_filter
or:
 # echo 'nonexist_symbol:mod:fuse' > set_ftrace_filter

Better make it return failure.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC4BF.2070003@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/ftrace.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e51a1bc..08a3fb5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1724,7 +1724,7 @@ ftrace_match_record(struct dyn_ftrace *rec, char *regex, int len, int type)
 	return ftrace_match(str, regex, len, type);
 }
 
-static void ftrace_match_records(char *buff, int len, int enable)
+static int ftrace_match_records(char *buff, int len, int enable)
 {
 	unsigned int search_len;
 	struct ftrace_page *pg;
@@ -1733,6 +1733,7 @@ static void ftrace_match_records(char *buff, int len, int enable)
 	char *search;
 	int type;
 	int not;
+	int found = 0;
 
 	flag = enable ? FTRACE_FL_FILTER : FTRACE_FL_NOTRACE;
 	type = filter_parse_regex(buff, len, &search, &not);
@@ -1750,6 +1751,7 @@ static void ftrace_match_records(char *buff, int len, int enable)
 				rec->flags &= ~flag;
 			else
 				rec->flags |= flag;
+			found = 1;
 		}
 		/*
 		 * Only enable filtering if we have a function that
@@ -1759,6 +1761,8 @@ static void ftrace_match_records(char *buff, int len, int enable)
 			ftrace_filtered = 1;
 	} while_for_each_ftrace_rec();
 	mutex_unlock(&ftrace_lock);
+
+	return found;
 }
 
 static int
@@ -1780,7 +1784,7 @@ ftrace_match_module_record(struct dyn_ftrace *rec, char *mod,
 		return 1;
 }
 
-static void ftrace_match_module_records(char *buff, char *mod, int enable)
+static int ftrace_match_module_records(char *buff, char *mod, int enable)
 {
 	unsigned search_len = 0;
 	struct ftrace_page *pg;
@@ -1789,6 +1793,7 @@ static void ftrace_match_module_records(char *buff, char *mod, int enable)
 	char *search = buff;
 	unsigned long flag;
 	int not = 0;
+	int found = 0;
 
 	flag = enable ? FTRACE_FL_FILTER : FTRACE_FL_NOTRACE;
 
@@ -1819,12 +1824,15 @@ static void ftrace_match_module_records(char *buff, char *mod, int enable)
 				rec->flags &= ~flag;
 			else
 				rec->flags |= flag;
+			found = 1;
 		}
 		if (enable && (rec->flags & FTRACE_FL_FILTER))
 			ftrace_filtered = 1;
 
 	} while_for_each_ftrace_rec();
 	mutex_unlock(&ftrace_lock);
+
+	return found;
 }
 
 /*
@@ -1853,8 +1861,9 @@ ftrace_mod_callback(char *func, char *cmd, char *param, int enable)
 	if (!strlen(mod))
 		return -EINVAL;
 
-	ftrace_match_module_records(func, mod, enable);
-	return 0;
+	if (ftrace_match_module_records(func, mod, enable))
+		return 0;
+	return -EINVAL;
 }
 
 static struct ftrace_func_command ftrace_mod_cmd = {
@@ -2151,8 +2160,9 @@ static int ftrace_process_regex(char *buff, int len, int enable)
 	func = strsep(&next, ":");
 
 	if (!next) {
-		ftrace_match_records(func, len, enable);
-		return 0;
+		if (ftrace_match_records(func, len, enable))
+			return 0;
+		return ret;
 	}
 
 	/* command found */

[tip:tracing/urgent] ftrace: Call trace_parser_clear() properly

[tip-bot for Li Zefan]

Commit-ID:  313254a9400d388b46150c0f355e216418a2f598
Gitweb:     http://git.kernel.org/tip/313254a9400d388b46150c0f355e216418a2f598
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:15:30 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:26 +0100

ftrace: Call trace_parser_clear() properly

I found a weird behavior:

  # echo 'fuse:*' > set_ftrace_filter
  bash: echo: write error: Invalid argument
  # cat set_ftrace_filter
  fuse_dev_fasync
  fuse_dev_poll
  fuse_copy_do

We should call trace_parser_clear() no matter ftrace_process_regex()
returns 0 or -errno, otherwise we will actually take the unaccepted
records from ftrace_regex_release().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC4D2.3000406@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/ftrace.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 08a3fb5..ff8aecd 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2208,10 +2208,9 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
 	    !trace_parser_cont(parser)) {
 		ret = ftrace_process_regex(parser->buffer,
 					   parser->idx, enable);
+		trace_parser_clear(parser);
 		if (ret)
 			goto out_unlock;
-
-		trace_parser_clear(parser);
 	}
 
 	ret = read;

[tip:tracing/urgent] function-graph: Allow writing the same val to set_graph_function

[tip-bot for Li Zefan]

Commit-ID:  91baf6285be7282cfa487de92f836c50749dffb9
Gitweb:     http://git.kernel.org/tip/91baf6285be7282cfa487de92f836c50749dffb9
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:15:45 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:26 +0100

function-graph: Allow writing the same val to set_graph_function

# echo 'do_open' > set_graph_function
 # echo 'do_open' >> set_graph_function
 bash: echo: write error: Invalid argument

Make it valid to write the same value to set_graph_function,
which is consistent with set_ftrace_filter interface.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-reference: <4B1DC4E1.1060303@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/ftrace.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ff8aecd..7968762 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2552,10 +2552,9 @@ ftrace_set_func(unsigned long *array, int *idx, char *buffer)
 					exists = true;
 					break;
 				}
-			if (!exists) {
+			if (!exists)
 				array[(*idx)++] = rec->ip;
-				found = 1;
-			}
+			found = 1;
 		}
 	} while_for_each_ftrace_rec();
 

[tip:tracing/urgent] tracing: Use seq file for trace_options

[tip-bot for Li Zefan]

Commit-ID:  fdb372ed4cadbfe9dbba0e932a77d0523682e690
Gitweb:     http://git.kernel.org/tip/fdb372ed4cadbfe9dbba0e932a77d0523682e690
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:15:59 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:27 +0100

tracing: Use seq file for trace_options

Code simplification for reading trace_options.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-reference: <4B1DC4EF.3090106@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c |   60 ++++++++++++++-----------------------------------
 1 files changed, 17 insertions(+), 43 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 88bd9ae..a6c41cc 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2316,67 +2316,32 @@ static const struct file_operations tracing_cpumask_fops = {
 	.write		= tracing_cpumask_write,
 };
 
-static ssize_t
-tracing_trace_options_read(struct file *filp, char __user *ubuf,
-		       size_t cnt, loff_t *ppos)
+static int tracing_trace_options_show(struct seq_file *m, void *v)
 {
 	struct tracer_opt *trace_opts;
 	u32 tracer_flags;
-	int len = 0;
-	char *buf;
-	int r = 0;
 	int i;
 
-
-	/* calculate max size */
-	for (i = 0; trace_options[i]; i++) {
-		len += strlen(trace_options[i]);
-		len += 3; /* "no" and newline */
-	}
-
 	mutex_lock(&trace_types_lock);
 	tracer_flags = current_trace->flags->val;
 	trace_opts = current_trace->flags->opts;
 
-	/*
-	 * Increase the size with names of options specific
-	 * of the current tracer.
-	 */
-	for (i = 0; trace_opts[i].name; i++) {
-		len += strlen(trace_opts[i].name);
-		len += 3; /* "no" and newline */
-	}
-
-	/* +1 for \0 */
-	buf = kmalloc(len + 1, GFP_KERNEL);
-	if (!buf) {
-		mutex_unlock(&trace_types_lock);
-		return -ENOMEM;
-	}
-
 	for (i = 0; trace_options[i]; i++) {
 		if (trace_flags & (1 << i))
-			r += sprintf(buf + r, "%s\n", trace_options[i]);
+			seq_printf(m, "%s\n", trace_options[i]);
 		else
-			r += sprintf(buf + r, "no%s\n", trace_options[i]);
+			seq_printf(m, "no%s\n", trace_options[i]);
 	}
 
 	for (i = 0; trace_opts[i].name; i++) {
 		if (tracer_flags & trace_opts[i].bit)
-			r += sprintf(buf + r, "%s\n",
-				trace_opts[i].name);
+			seq_printf(m, "%s\n", trace_opts[i].name);
 		else
-			r += sprintf(buf + r, "no%s\n",
-				trace_opts[i].name);
+			seq_printf(m, "no%s\n", trace_opts[i].name);
 	}
 	mutex_unlock(&trace_types_lock);
 
-	WARN_ON(r >= len + 1);
-
-	r = simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
-
-	kfree(buf);
-	return r;
+	return 0;
 }
 
 /* Try to assign a tracer specific option */
@@ -2471,9 +2436,18 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
+static int tracing_trace_options_open(struct inode *inode, struct file *file)
+{
+	if (tracing_disabled)
+		return -ENODEV;
+	return single_open(file, tracing_trace_options_show, NULL);
+}
+
 static const struct file_operations tracing_iter_fops = {
-	.open		= tracing_open_generic,
-	.read		= tracing_trace_options_read,
+	.open		= tracing_trace_options_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
 	.write		= tracing_trace_options_write,
 };
 

[tip:tracing/urgent] tracing: Use seq file for trace_clock

[tip-bot for Li Zefan]

Commit-ID:  13f16d209161c95e92aef40e350cc6cf56ac440b
Gitweb:     http://git.kernel.org/tip/13f16d209161c95e92aef40e350cc6cf56ac440b
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:16:11 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:27 +0100

tracing: Use seq file for trace_clock

The buffer for the output is as small as 64 bytes, so it'll
overflow if we add more clock type. Use seq file instead.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC4FB.5030407@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a6c41cc..886268e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3366,21 +3366,18 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
-static ssize_t tracing_clock_read(struct file *filp, char __user *ubuf,
-				  size_t cnt, loff_t *ppos)
+static int tracing_clock_show(struct seq_file *m, void *v)
 {
-	char buf[64];
-	int bufiter = 0;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(trace_clocks); i++)
-		bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter,
+		seq_printf(m,
 			"%s%s%s%s", i ? " " : "",
 			i == trace_clock_id ? "[" : "", trace_clocks[i].name,
 			i == trace_clock_id ? "]" : "");
-	bufiter += snprintf(buf + bufiter, sizeof(buf) - bufiter, "\n");
+	seq_putc(m, '\n');
 
-	return simple_read_from_buffer(ubuf, cnt, ppos, buf, bufiter);
+	return 0;
 }
 
 static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
@@ -3422,6 +3419,13 @@ static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
 	return cnt;
 }
 
+static int tracing_clock_open(struct inode *inode, struct file *file)
+{
+	if (tracing_disabled)
+		return -ENODEV;
+	return single_open(file, tracing_clock_show, NULL);
+}
+
 static const struct file_operations tracing_max_lat_fops = {
 	.open		= tracing_open_generic,
 	.read		= tracing_max_lat_read,
@@ -3460,8 +3464,10 @@ static const struct file_operations tracing_mark_fops = {
 };
 
 static const struct file_operations trace_clock_fops = {
-	.open		= tracing_open_generic,
-	.read		= tracing_clock_read,
+	.open		= tracing_clock_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
 	.write		= tracing_clock_write,
 };
 

[tip:tracing/urgent] tracing: Remove useless trace option

[tip-bot for Li Zefan]

Commit-ID:  2cbafd68b826f8e0471875cf33cdfb8a1478aef1
Gitweb:     http://git.kernel.org/tip/2cbafd68b826f8e0471875cf33cdfb8a1478aef1
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:16:26 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:27 +0100

tracing: Remove useless trace option

Since commit 4d9493c90f8e6e1b164aede3814010a290161abb
("ftrace: remove add-hoc code"), option "sched-tree"
has become useless.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC50A.7040402@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c |    1 -
 kernel/trace/trace.h |   23 +++++++++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 886268e..898409d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -313,7 +313,6 @@ static const char *trace_options[] = {
 	"bin",
 	"block",
 	"stacktrace",
-	"sched-tree",
 	"trace_printk",
 	"ftrace_preempt",
 	"branch",
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7fa33ca..1b18cb2 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -597,18 +597,17 @@ enum trace_iterator_flags {
 	TRACE_ITER_BIN			= 0x40,
 	TRACE_ITER_BLOCK		= 0x80,
 	TRACE_ITER_STACKTRACE		= 0x100,
-	TRACE_ITER_SCHED_TREE		= 0x200,
-	TRACE_ITER_PRINTK		= 0x400,
-	TRACE_ITER_PREEMPTONLY		= 0x800,
-	TRACE_ITER_BRANCH		= 0x1000,
-	TRACE_ITER_ANNOTATE		= 0x2000,
-	TRACE_ITER_USERSTACKTRACE       = 0x4000,
-	TRACE_ITER_SYM_USEROBJ          = 0x8000,
-	TRACE_ITER_PRINTK_MSGONLY	= 0x10000,
-	TRACE_ITER_CONTEXT_INFO		= 0x20000, /* Print pid/cpu/time */
-	TRACE_ITER_LATENCY_FMT		= 0x40000,
-	TRACE_ITER_SLEEP_TIME		= 0x80000,
-	TRACE_ITER_GRAPH_TIME		= 0x100000,
+	TRACE_ITER_PRINTK		= 0x200,
+	TRACE_ITER_PREEMPTONLY		= 0x400,
+	TRACE_ITER_BRANCH		= 0x800,
+	TRACE_ITER_ANNOTATE		= 0x1000,
+	TRACE_ITER_USERSTACKTRACE       = 0x2000,
+	TRACE_ITER_SYM_USEROBJ          = 0x4000,
+	TRACE_ITER_PRINTK_MSGONLY	= 0x8000,
+	TRACE_ITER_CONTEXT_INFO		= 0x10000, /* Print pid/cpu/time */
+	TRACE_ITER_LATENCY_FMT		= 0x20000,
+	TRACE_ITER_SLEEP_TIME		= 0x40000,
+	TRACE_ITER_GRAPH_TIME		= 0x80000,
 };
 
 /*

[tip:tracing/urgent] tracing: Simplify trace_option_write()

[tip-bot for Li Zefan]

Commit-ID:  8d18eaaff5acaa58369be342c86e607643ce10c7
Gitweb:     http://git.kernel.org/tip/8d18eaaff5acaa58369be342c86e607643ce10c7
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:17:06 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:28 +0100

tracing: Simplify trace_option_write()

- remove duplicate code inside trace_options_write()
- extract duplicate code in trace_options_write() and set_tracer_option()

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC532.9010802@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace.c |   85 ++++++++++++++++++-------------------------------
 1 files changed, 31 insertions(+), 54 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 898409d..0507600 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2343,38 +2343,39 @@ static int tracing_trace_options_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int __set_tracer_option(struct tracer *trace,
+			       struct tracer_flags *tracer_flags,
+			       struct tracer_opt *opts, int neg)
+{
+	int ret;
+
+	ret = trace->set_flag(tracer_flags->val, opts->bit, !neg);
+	if (ret)
+		return ret;
+
+	if (neg)
+		tracer_flags->val &= ~opts->bit;
+	else
+		tracer_flags->val |= opts->bit;
+	return 0;
+}
+
 /* Try to assign a tracer specific option */
 static int set_tracer_option(struct tracer *trace, char *cmp, int neg)
 {
 	struct tracer_flags *tracer_flags = trace->flags;
 	struct tracer_opt *opts = NULL;
-	int ret = 0, i = 0;
-	int len;
+	int i;
 
 	for (i = 0; tracer_flags->opts[i].name; i++) {
 		opts = &tracer_flags->opts[i];
-		len = strlen(opts->name);
 
-		if (strncmp(cmp, opts->name, len) == 0) {
-			ret = trace->set_flag(tracer_flags->val,
-				opts->bit, !neg);
-			break;
-		}
+		if (strcmp(cmp, opts->name) == 0)
+			return __set_tracer_option(trace, trace->flags,
+						   opts, neg);
 	}
-	/* Not found */
-	if (!tracer_flags->opts[i].name)
-		return -EINVAL;
 
-	/* Refused to handle */
-	if (ret)
-		return ret;
-
-	if (neg)
-		tracer_flags->val &= ~opts->bit;
-	else
-		tracer_flags->val |= opts->bit;
-
-	return 0;
+	return -EINVAL;
 }
 
 static void set_tracer_flags(unsigned int mask, int enabled)
@@ -2394,7 +2395,7 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
 {
 	char buf[64];
-	char *cmp = buf;
+	char *cmp;
 	int neg = 0;
 	int ret;
 	int i;
@@ -2406,16 +2407,15 @@ tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 		return -EFAULT;
 
 	buf[cnt] = 0;
+	cmp = strstrip(buf);
 
-	if (strncmp(buf, "no", 2) == 0) {
+	if (strncmp(cmp, "no", 2) == 0) {
 		neg = 1;
 		cmp += 2;
 	}
 
 	for (i = 0; trace_options[i]; i++) {
-		int len = strlen(trace_options[i]);
-
-		if (strncmp(cmp, trace_options[i], len) == 0) {
+		if (strcmp(cmp, trace_options[i]) == 0) {
 			set_tracer_flags(1 << i, !neg);
 			break;
 		}
@@ -3927,39 +3927,16 @@ trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	if (ret < 0)
 		return ret;
 
-	ret = 0;
-	switch (val) {
-	case 0:
-		/* do nothing if already cleared */
-		if (!(topt->flags->val & topt->opt->bit))
-			break;
-
-		mutex_lock(&trace_types_lock);
-		if (current_trace->set_flag)
-			ret = current_trace->set_flag(topt->flags->val,
-						      topt->opt->bit, 0);
-		mutex_unlock(&trace_types_lock);
-		if (ret)
-			return ret;
-		topt->flags->val &= ~topt->opt->bit;
-		break;
-	case 1:
-		/* do nothing if already set */
-		if (topt->flags->val & topt->opt->bit)
-			break;
+	if (val != 0 && val != 1)
+		return -EINVAL;
 
+	if (!!(topt->flags->val & topt->opt->bit) != val) {
 		mutex_lock(&trace_types_lock);
-		if (current_trace->set_flag)
-			ret = current_trace->set_flag(topt->flags->val,
-						      topt->opt->bit, 1);
+		ret = __set_tracer_option(current_trace, topt->flags,
+					  topt->opt, val);
 		mutex_unlock(&trace_types_lock);
 		if (ret)
 			return ret;
-		topt->flags->val |= topt->opt->bit;
-		break;
-
-	default:
-		return -EINVAL;
 	}
 
 	*ppos += cnt;

[tip:tracing/urgent] tracing: Change event->profile_count to be int type

[tip-bot for Li Zefan]

Commit-ID:  e00bf2ec60605eb95687b7a0c3b83c87c48541dc
Gitweb:     http://git.kernel.org/tip/e00bf2ec60605eb95687b7a0c3b83c87c48541dc
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:17:29 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:28 +0100

tracing: Change event->profile_count to be int type

Like total_profile_count, struct ftrace_event_call::profile_count
is protected by event_mutex, so it doesn't need to be atomic_t.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <4B1DC549.5010705@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/ftrace_event.h       |    2 +-
 include/linux/syscalls.h           |    2 --
 include/trace/ftrace.h             |    1 -
 kernel/trace/trace_event_profile.c |    6 +++---
 kernel/trace/trace_kprobe.c        |    1 -
 5 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index db97c64..2233c98 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -131,7 +131,7 @@ struct ftrace_event_call {
 	void			*mod;
 	void			*data;
 
-	atomic_t		profile_count;
+	int			profile_count;
 	int			(*profile_enable)(struct ftrace_event_call *);
 	void			(*profile_disable)(struct ftrace_event_call *);
 };
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 94ac284..72d6986 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -102,12 +102,10 @@ struct perf_event_attr;
 #ifdef CONFIG_EVENT_PROFILE
 
 #define TRACE_SYS_ENTER_PROFILE_INIT(sname)				       \
-	.profile_count = ATOMIC_INIT(-1),				       \
 	.profile_enable = prof_sysenter_enable,				       \
 	.profile_disable = prof_sysenter_disable,
 
 #define TRACE_SYS_EXIT_PROFILE_INIT(sname)				       \
-	.profile_count = ATOMIC_INIT(-1),				       \
 	.profile_enable = prof_sysexit_enable,				       \
 	.profile_disable = prof_sysexit_disable,
 #else
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 0c21af8..7352315 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -629,7 +629,6 @@ static void ftrace_profile_disable_##name(struct ftrace_event_call *unused)\
 #ifdef CONFIG_EVENT_PROFILE
 
 #define _TRACE_PROFILE_INIT(call)					\
-	.profile_count = ATOMIC_INIT(-1),				\
 	.profile_enable = ftrace_profile_enable_##call,			\
 	.profile_disable = ftrace_profile_disable_##call,
 
diff --git a/kernel/trace/trace_event_profile.c b/kernel/trace/trace_event_profile.c
index d9c60f8..9e25573 100644
--- a/kernel/trace/trace_event_profile.c
+++ b/kernel/trace/trace_event_profile.c
@@ -25,7 +25,7 @@ static int ftrace_profile_enable_event(struct ftrace_event_call *event)
 	char *buf;
 	int ret = -ENOMEM;
 
-	if (atomic_inc_return(&event->profile_count))
+	if (event->profile_count++ > 0)
 		return 0;
 
 	if (!total_profile_count) {
@@ -56,7 +56,7 @@ fail_buf_nmi:
 		perf_trace_buf = NULL;
 	}
 fail_buf:
-	atomic_dec(&event->profile_count);
+	event->profile_count--;
 
 	return ret;
 }
@@ -83,7 +83,7 @@ static void ftrace_profile_disable_event(struct ftrace_event_call *event)
 {
 	char *buf, *nmi_buf;
 
-	if (!atomic_add_negative(-1, &event->profile_count))
+	if (--event->profile_count > 0)
 		return;
 
 	event->profile_disable(event);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e3c80e9..6ed2234 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1426,7 +1426,6 @@ static int register_probe_event(struct trace_probe *tp)
 	call->unregfunc = probe_event_disable;
 
 #ifdef CONFIG_EVENT_PROFILE
-	atomic_set(&call->profile_count, -1);
 	call->profile_enable = probe_profile_enable;
 	call->profile_disable = probe_profile_disable;
 #endif

[tip:tracing/urgent] tracing/power: Remove two exports

[tip-bot for Li Zefan]

Commit-ID:  472bbe02c92a7a8299d7b16946277d98bb8f4bb7
Gitweb:     http://git.kernel.org/tip/472bbe02c92a7a8299d7b16946277d98bb8f4bb7
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:17:51 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:37:28 +0100

tracing/power: Remove two exports

trace_power_start and trace_power_end are used in
arch/x86/kernel/power.c, and this file can't be compiled
as a module, so these two tracepoints don't need to be
exported.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <4B1DC55F.7060305@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/power-traces.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/power-traces.c b/kernel/trace/power-traces.c
index e06c6e3..9f4f565 100644
--- a/kernel/trace/power-traces.c
+++ b/kernel/trace/power-traces.c
@@ -14,7 +14,5 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/power.h>
 
-EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
-EXPORT_TRACEPOINT_SYMBOL_GPL(power_end);
 EXPORT_TRACEPOINT_SYMBOL_GPL(power_frequency);
 

[tip:tracing/urgent] ksym_tracer: Fix bad cast

[tip-bot for Li Zefan]

Commit-ID:  16620e0f1990fa6d896a639449c4b3d678458464
Gitweb:     http://git.kernel.org/tip/16620e0f1990fa6d896a639449c4b3d678458464
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 8 Dec 2009 11:18:16 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Sun, 13 Dec 2009 18:46:54 +0100

ksym_tracer: Fix bad cast

Fix this warning:

kernel/trace/trace_ksym.c: In function 'ksym_trace_filter_read':
kernel/trace/trace_ksym.c:239: warning: cast to pointer from integer of different size

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: "K.Prasad" <prasad@linux.vnet.ibm.com>
LKML-Reference: <4B1DC578.9020909@cn.fujitsu.com>
[remove the strstrip fix as tglx already fixed that]
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace_ksym.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 64e7a5b..48f1c6c 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -235,7 +235,8 @@ static ssize_t ksym_trace_filter_read(struct file *filp, char __user *ubuf,
 	mutex_lock(&ksym_tracer_mutex);
 
 	hlist_for_each_entry(entry, node, &ksym_filter_head, ksym_hlist) {
-		ret = trace_seq_printf(s, "%pS:", (void *)entry->attr.bp_addr);
+		ret = trace_seq_printf(s, "%pS:",
+				(void *)(unsigned long)entry->attr.bp_addr);
 		if (entry->attr.bp_type == HW_BREAKPOINT_R)
 			ret = trace_seq_puts(s, "r--\n");
 		else if (entry->attr.bp_type == HW_BREAKPOINT_W)
@@ -298,8 +299,8 @@ static ssize_t ksym_trace_filter_write(struct file *file,
 	 * 2: echo 0 > ksym_trace_filter
 	 * 3: echo "*:---" > ksym_trace_filter
 	 */
-	if (!input_string[0] || !strcmp(input_string, "0") ||
-	    !strcmp(input_string, "*:---")) {
+	if (!buf[0] || !strcmp(buf, "0") ||
+	    !strcmp(buf, "*:---")) {
 		__ksym_trace_reset();
 		ret = 0;
 		goto out;