2009-12-07 07:40:18

by Li Zefan

[permalink] [raw]
Subject: [PATCH 0/13] tracing: various cleanups and small fixes

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

---
include/linux/ftrace_event.h | 3 +-
include/linux/syscalls.h | 2 -
include/trace/ftrace.h | 55 ++++--------
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 | 24 ++++--
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, 138 insertions(+), 217 deletions(-)


2009-12-07 07:40:37

by Li Zefan

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

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

text data bss dec hex filename
5459553 2005772 7103796 14569121 de4ea1 vmlinux.o.old
5456157 2005772 7103796 14565725 de415d vmlinux.o

Signed-off-by: Li Zefan <[email protected]>
---
include/trace/ftrace.h | 34 +++++++++++++++-------------------
1 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index d1b3de9..4d114c9 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -647,9 +647,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) \
@@ -716,6 +713,18 @@ static void ftrace_raw_event_id_##call(struct ftrace_event_call *event_call, \
if (!filter_current_check_discard(buffer, event_call, entry, event)) \
trace_nowake_buffer_unlock_commit(buffer, \
event, irq_flags, pc); \
+} \
+ \
+static int ftrace_raw_init_event_##call(struct ftrace_event_call *event_call)\
+{ \
+ int id; \
+ \
+ id = register_ftrace_event(event_call->event); \
+ if (!id) \
+ return -ENODEV; \
+ event_call->id = id; \
+ INIT_LIST_HEAD(&event_call->fields); \
+ return 0; \
}

#undef DEFINE_EVENT
@@ -744,19 +753,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 +773,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 = ftrace_raw_init_event_##template, \
.regfunc = ftrace_raw_reg_event_##call, \
.unregfunc = ftrace_raw_unreg_event_##call, \
.show_format = ftrace_format_##template, \
@@ -793,7 +790,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 = ftrace_raw_init_event_##template, \
.regfunc = ftrace_raw_reg_event_##call, \
.unregfunc = ftrace_raw_unreg_event_##call, \
.show_format = ftrace_format_##call, \
@@ -953,7 +950,6 @@ end: \
perf_swevent_put_recursion_context(rctx); \
end_recursion: \
local_irq_restore(irq_flags); \
- \
}

#undef DEFINE_EVENT
--
1.6.3

2009-12-07 07:40:57

by Li Zefan

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

Call trace_define_common_fields() in event_create_dir() only.

text data bss dec hex filename
5456157 2005772 7103796 14565725 de415d vmlinux.o
5454538 2005772 7103796 14564106 de3b0a vmlinux.o

Signed-off-by: Li Zefan <[email protected]>
---
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 47bbdf9..02de70f 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -157,7 +157,6 @@ enum {
FILTER_PTR_STRING,
};

-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 4d114c9..84d6f23 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 1d18315..9fa6736 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)
{
@@ -913,7 +912,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

2009-12-07 07:41:14

by Li Zefan

[permalink] [raw]
Subject: [PATCH 03/13] 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().

See how much space this saves:

text data bss dec hex filename
5454538 2005772 7103796 14564106 de3b0a vmlinux.o
5440766 2005772 7103796 14550334 de053e vmlinux.o

Signed-off-by: Li Zefan <[email protected]>
---
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 84d6f23..4aac981 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)
@@ -733,13 +727,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 9fa6736..f22eaec 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -104,9 +104,11 @@ void trace_destroy_fields(struct ftrace_event_call *call)
}
}

-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) {
@@ -117,12 +119,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
break;
case 1:
if (!call->enabled) {
+ 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);
+ ret = call->regfunc(call);
}
break;
}
+
+ return ret;
}

static void ftrace_clear_events(void)
@@ -401,7 +410,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;

@@ -411,7 +420,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

2009-12-07 07:41:36

by Li Zefan

[permalink] [raw]
Subject: [PATCH 04/13] 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 <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
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

2009-12-07 07:41:49

by Li Zefan

[permalink] [raw]
Subject: [PATCH 05/13] 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.

Signed-off-by: Li Zefan <[email protected]>
---
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

2009-12-07 07:42:07

by Li Zefan

[permalink] [raw]
Subject: [PATCH 06/13] 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 <[email protected]>
---
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

2009-12-07 07:42:32

by Li Zefan

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

Code simplification for reading trace_options.

Signed-off-by: Li Zefan <[email protected]>
---
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

2009-12-07 07:42:53

by Li Zefan

[permalink] [raw]
Subject: [PATCH 08/13] 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 types. Use seq file instead.

Signed-off-by: Li Zefan <[email protected]>
---
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

2009-12-07 07:43:13

by Li Zefan

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

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

Signed-off-by: Li Zefan <[email protected]>
---
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

2009-12-07 07:43:40

by Li Zefan

[permalink] [raw]
Subject: [PATCH 10/13] 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 <[email protected]>
---
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

2009-12-07 07:44:09

by Li Zefan

[permalink] [raw]
Subject: [PATCH 11/13] 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 <[email protected]>
---
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 02de70f..22a04d9 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 e79e2f3..d3ecfb9 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 4aac981..51f045b 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -640,7 +640,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

2009-12-07 07:44:41

by Li Zefan

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

trace_power_start and trace_power_end are only 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 <[email protected]>
---
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

2009-12-07 07:45:24

by Li Zefan

[permalink] [raw]
Subject: [PATCH 13/13] ksym_tracer: Fix compile warnings

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 <[email protected]>
---
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

2009-12-07 14:27:01

by Frederic Weisbecker

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

2009/12/7 Li Zefan <[email protected]>:
> +static int ftrace_raw_init_event_##call(struct ftrace_event_call *event_call)\> +{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\> + ? ? ? int id; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \> + ? ? ? id = register_ftrace_event(event_call->event); ? ? ? ? ? ? ? ? ?\> + ? ? ? if (!id) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\> + ? ? ? ? ? ? ? return -ENODEV; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \> + ? ? ? event_call->id = id; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\> + ? ? ? INIT_LIST_HEAD(&event_call->fields); ? ? ? ? ? ? ? ? ? ? ? ? ? ?\> + ? ? ? return 0; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \> ?}

This function doesn't vary anymore in this form.May be can we define a generic one in trace_event.c and only referencethis one?
Or even better, may be can we drop this callback field and statically call thiscode when we intialize an event. IIRC, the syscall raw_init_event has thesame callback, may be it's even the same for kprobes events (I can'tcheck right now).????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2009-12-07 14:43:12

by Frederic Weisbecker

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

2009/12/7 Li Zefan <[email protected]>:
> ?# 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 <[email protected]>


Acked-by: Frederic Weisbecker <[email protected]>


> ---
> ?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
>
>

2009-12-07 15:00:43

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 12/13] tracing/power: Remove two exports

On Mon, 07 Dec 2009 15:44:19 +0800
Li Zefan <[email protected]> wrote:

> trace_power_start and trace_power_end are only 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.
>

they were originally exported if some other arch wanted to use them
modular... since nobody does

Acked-by: Arjan van de Ven <[email protected]>


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-12-07 20:39:21

by Frederic Weisbecker

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

On Mon, Dec 07, 2009 at 03:40:35PM +0800, Li Zefan wrote:
> Call trace_define_common_fields() in event_create_dir() only.
>
> text data bss dec hex filename
> 5456157 2005772 7103796 14565725 de415d vmlinux.o
> 5454538 2005772 7103796 14564106 de3b0a vmlinux.o
>
> Signed-off-by: Li Zefan <[email protected]>


Acked-by: Frederic Weisbecker <[email protected]>

2009-12-07 21:32:29

by Frederic Weisbecker

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

On Mon, Dec 07, 2009 at 03:40:51PM +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
> 5454538 2005772 7103796 14564106 de3b0a vmlinux.o
> 5440766 2005772 7103796 14550334 de053e vmlinux.o
>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> 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 84d6f23..4aac981 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)
> @@ -733,13 +727,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 9fa6736..f22eaec 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -104,9 +104,11 @@ void trace_destroy_fields(struct ftrace_event_call *call)
> }
> }
>
> -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) {
> @@ -117,12 +119,19 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call,
> break;
> case 1:
> if (!call->enabled) {
> + 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);
> + ret = call->regfunc(call);



Heh, I'm pretty sure I will never see this warning ;-)

2009-12-08 00:55:07

by Li Zefan

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

Frederic Weisbecker wrote:
> 2009/12/7 Li Zefan <[email protected]>:
>
>> +static int ftrace_raw_init_event_##call(struct ftrace_event_call *event_call)\
>> +{ \
>> + int id; \
>> + \
>> + id = register_ftrace_event(event_call->event); \
>> + if (!id) \
>> + return -ENODEV; \
>> + event_call->id = id; \
>> + INIT_LIST_HEAD(&event_call->fields); \
>> + return 0; \
>> }
>
>
> This function doesn't vary anymore in this form.
> May be can we define a generic one in trace_event.c and only reference
> this one?
>

Ah, you're right. I should have noticed this.

I'll make a patch to do this later on.

> Or even better, may be can we drop this callback field and statically call this
> code when we intialize an event. IIRC, the syscall raw_init_event has the
> same callback, may be it's even the same for kprobes events (I can't
> check right now).

I'll check it.

2009-12-08 01:01:08

by Li Zefan

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

>> case 1:
>> if (!call->enabled) {
>> + 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);
>> + ret = call->regfunc(call);
>
> Heh, I'm pretty sure I will never see this warning ;-)
>

oops! I don't know how this mistake came.