* [PATCH 1/4] ksym_tracer: Fix to make the tracer work
@ 2009-12-30 6:22 Li Zefan
2009-12-30 6:23 ` [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter Li Zefan
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Li Zefan @ 2009-12-30 6:22 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Steven Rostedt, Ingo Molnar, K.Prasad, LKML
ksym tracer doesn't work:
# echo tasklist_lock:rw- > ksym_trace_filter
-bash: echo: write error: No such device
It's because we pass to perf_event_create_kernel_counter()
a cpu number which is not present.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/hw_breakpoint.c | 10 +++++++---
kernel/trace/trace_ksym.c | 1 -
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index dbcbf6a..50dbd59 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -40,6 +40,7 @@
#include <linux/percpu.h>
#include <linux/sched.h>
#include <linux/init.h>
+#include <linux/cpu.h>
#include <linux/smp.h>
#include <linux/hw_breakpoint.h>
@@ -388,7 +389,8 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
if (!cpu_events)
return ERR_PTR(-ENOMEM);
- for_each_possible_cpu(cpu) {
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
pevent = per_cpu_ptr(cpu_events, cpu);
bp = perf_event_create_kernel_counter(attr, cpu, -1, triggered);
@@ -399,18 +401,20 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
goto fail;
}
}
+ put_online_cpus();
return cpu_events;
fail:
- for_each_possible_cpu(cpu) {
+ for_each_online_cpu(cpu) {
pevent = per_cpu_ptr(cpu_events, cpu);
if (IS_ERR(*pevent))
break;
unregister_hw_breakpoint(*pevent);
}
+ put_online_cpus();
+
free_percpu(cpu_events);
- /* return the error if any */
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(register_wide_hw_breakpoint);
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index faf37fa..340b6ff 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -197,7 +197,6 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
entry->attr.bp_addr = addr;
entry->attr.bp_len = HW_BREAKPOINT_LEN_4;
- ret = -EAGAIN;
entry->ksym_hbp = register_wide_hw_breakpoint(&entry->attr,
ksym_hbp_handler);
--
1.6.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
@ 2009-12-30 6:23 ` Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 6:23 ` [PATCH 3/4] ksym_tracer: Fix race when incrementing count Li Zefan
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Li Zefan @ 2009-12-30 6:23 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Steven Rostedt, Ingo Molnar, K.Prasad, LKML, Thomas Gleixner
It used to work, but now doesn't:
# echo > ksym_filter
bash: echo: write error: Invalid argument
It's caused by d954fbf0ff6b5fdfb32350e85a2f15d3db976506
("tracing: Fix wrong usage of strstrip in trace_ksyms").
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_ksym.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 340b6ff..160a8d8 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -299,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 (!buf[0] || !strcmp(buf, "0") ||
- !strcmp(buf, "*:---")) {
+ if (!input_string[0] || !strcmp(input_string, "0") ||
+ !strcmp(input_string, "*:---")) {
__ksym_trace_reset();
ret = 0;
goto out;
--
1.6.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] ksym_tracer: Fix race when incrementing count
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
2009-12-30 6:23 ` [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter Li Zefan
@ 2009-12-30 6:23 ` Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 6:24 ` [PATCH 4/4] ksym_tracer: Remove trace_stat Li Zefan
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Li Zefan @ 2009-12-30 6:23 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Steven Rostedt, Ingo Molnar, K.Prasad, LKML
We are under rcu read section but not holding the write lock, so
count++ is not atomic. Use atomic64_t instead.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_ksym.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 160a8d8..67d79f7 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -32,6 +32,8 @@
#include <linux/hw_breakpoint.h>
#include <asm/hw_breakpoint.h>
+#include <asm/atomic.h>
+
/*
* For now, let us restrict the no. of symbols traced simultaneously to number
* of available hardware breakpoint registers.
@@ -44,7 +46,7 @@ struct trace_ksym {
struct perf_event **ksym_hbp;
struct perf_event_attr attr;
#ifdef CONFIG_PROFILE_KSYM_TRACER
- unsigned long counter;
+ atomic64_t counter;
#endif
struct hlist_node ksym_hlist;
};
@@ -69,9 +71,8 @@ void ksym_collect_stats(unsigned long hbp_hit_addr)
rcu_read_lock();
hlist_for_each_entry_rcu(entry, node, &ksym_filter_head, ksym_hlist) {
- if ((entry->attr.bp_addr == hbp_hit_addr) &&
- (entry->counter <= MAX_UL_INT)) {
- entry->counter++;
+ if (entry->attr.bp_addr == hbp_hit_addr) {
+ atomic64_inc(&entry->counter);
break;
}
}
@@ -501,7 +502,8 @@ static int ksym_tracer_stat_show(struct seq_file *m, void *v)
seq_printf(m, " %-36s", fn_name);
else
seq_printf(m, " %-36s", "<NA>");
- seq_printf(m, " %15lu\n", entry->counter);
+ seq_printf(m, " %15llu\n",
+ (unsigned long long)atomic64_read(&entry->counter));
return 0;
}
--
1.6.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] ksym_tracer: Remove trace_stat
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
2009-12-30 6:23 ` [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter Li Zefan
2009-12-30 6:23 ` [PATCH 3/4] ksym_tracer: Fix race when incrementing count Li Zefan
@ 2009-12-30 6:24 ` Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 9:30 ` [tip:tracing/urgent] ksym_tracer: Fix to make the tracer work tip-bot for Li Zefan
2009-12-30 15:05 ` [PATCH 1/4] " K.Prasad
4 siblings, 1 reply; 11+ messages in thread
From: Li Zefan @ 2009-12-30 6:24 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Steven Rostedt, Ingo Molnar, K.Prasad, LKML
trace_stat is problematic. Don't use it, use seqfile instead.
This fixes a race that reading the stat file is not protected by
any lock, which can lead to use after free.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_ksym.c | 127 ++++++++++++++++++---------------------------
1 files changed, 50 insertions(+), 77 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 67d79f7..94103cd 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -26,7 +26,6 @@
#include <linux/fs.h>
#include "trace_output.h"
-#include "trace_stat.h"
#include "trace.h"
#include <linux/hw_breakpoint.h>
@@ -444,103 +443,77 @@ struct tracer ksym_tracer __read_mostly =
.print_line = ksym_trace_output
};
-__init static int init_ksym_trace(void)
-{
- struct dentry *d_tracer;
- struct dentry *entry;
-
- d_tracer = tracing_init_dentry();
- ksym_filter_entry_count = 0;
-
- entry = debugfs_create_file("ksym_trace_filter", 0644, d_tracer,
- NULL, &ksym_tracing_fops);
- if (!entry)
- pr_warning("Could not create debugfs "
- "'ksym_trace_filter' file\n");
-
- return register_tracer(&ksym_tracer);
-}
-device_initcall(init_ksym_trace);
-
-
#ifdef CONFIG_PROFILE_KSYM_TRACER
-static int ksym_tracer_stat_headers(struct seq_file *m)
+static int ksym_profile_show(struct seq_file *m, void *v)
{
+ struct hlist_node *node;
+ struct trace_ksym *entry;
+ int access_type = 0;
+ char fn_name[KSYM_NAME_LEN];
+
seq_puts(m, " Access Type ");
seq_puts(m, " Symbol Counter\n");
seq_puts(m, " ----------- ");
seq_puts(m, " ------ -------\n");
- return 0;
-}
-static int ksym_tracer_stat_show(struct seq_file *m, void *v)
-{
- struct hlist_node *stat = v;
- struct trace_ksym *entry;
- int access_type = 0;
- char fn_name[KSYM_NAME_LEN];
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(entry, node, &ksym_filter_head, ksym_hlist) {
- entry = hlist_entry(stat, struct trace_ksym, ksym_hlist);
+ access_type = entry->attr.bp_type;
- access_type = entry->attr.bp_type;
+ switch (access_type) {
+ case HW_BREAKPOINT_R:
+ seq_puts(m, " R ");
+ break;
+ case HW_BREAKPOINT_W:
+ seq_puts(m, " W ");
+ break;
+ case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
+ seq_puts(m, " RW ");
+ break;
+ default:
+ seq_puts(m, " NA ");
+ }
- switch (access_type) {
- case HW_BREAKPOINT_R:
- seq_puts(m, " R ");
- break;
- case HW_BREAKPOINT_W:
- seq_puts(m, " W ");
- break;
- case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
- seq_puts(m, " RW ");
- break;
- default:
- seq_puts(m, " NA ");
+ if (lookup_symbol_name(entry->attr.bp_addr, fn_name) >= 0)
+ seq_printf(m, " %-36s", fn_name);
+ else
+ seq_printf(m, " %-36s", "<NA>");
+ seq_printf(m, " %15llu\n",
+ (unsigned long long)atomic64_read(&entry->counter));
}
-
- if (lookup_symbol_name(entry->attr.bp_addr, fn_name) >= 0)
- seq_printf(m, " %-36s", fn_name);
- else
- seq_printf(m, " %-36s", "<NA>");
- seq_printf(m, " %15llu\n",
- (unsigned long long)atomic64_read(&entry->counter));
+ rcu_read_unlock();
return 0;
}
-static void *ksym_tracer_stat_start(struct tracer_stat *trace)
+static int ksym_profile_open(struct inode *node, struct file *file)
{
- return ksym_filter_head.first;
-}
-
-static void *
-ksym_tracer_stat_next(void *v, int idx)
-{
- struct hlist_node *stat = v;
-
- return stat->next;
+ return single_open(file, ksym_profile_show, NULL);
}
-static struct tracer_stat ksym_tracer_stats = {
- .name = "ksym_tracer",
- .stat_start = ksym_tracer_stat_start,
- .stat_next = ksym_tracer_stat_next,
- .stat_headers = ksym_tracer_stat_headers,
- .stat_show = ksym_tracer_stat_show
+static const struct file_operations ksym_profile_fops = {
+ .open = ksym_profile_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
};
+#endif /* CONFIG_PROFILE_KSYM_TRACER */
-__init static int ksym_tracer_stat_init(void)
+__init static int init_ksym_trace(void)
{
- int ret;
+ struct dentry *d_tracer;
- ret = register_stat_tracer(&ksym_tracer_stats);
- if (ret) {
- printk(KERN_WARNING "Warning: could not register "
- "ksym tracer stats\n");
- return 1;
- }
+ d_tracer = tracing_init_dentry();
- return 0;
+ trace_create_file("ksym_trace_filter", 0644, d_tracer,
+ NULL, &ksym_tracing_fops);
+
+#ifdef CONFIG_PROFILE_KSYM_TRACER
+ trace_create_file("ksym_profile", 0444, d_tracer,
+ NULL, &ksym_profile_fops);
+#endif
+
+ return register_tracer(&ksym_tracer);
}
-fs_initcall(ksym_tracer_stat_init);
-#endif /* CONFIG_PROFILE_KSYM_TRACER */
+device_initcall(init_ksym_trace);
-- 1.6.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:tracing/urgent] ksym_tracer: Fix to make the tracer work
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
` (2 preceding siblings ...)
2009-12-30 6:24 ` [PATCH 4/4] ksym_tracer: Remove trace_stat Li Zefan
@ 2009-12-30 9:30 ` tip-bot for Li Zefan
2009-12-30 15:05 ` [PATCH 1/4] " K.Prasad
4 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Li Zefan @ 2009-12-30 9:30 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, lizf, fweisbec, rostedt, tglx, mingo, prasad
Commit-ID: 88f7a890d74137ab0d126a5d65679cd620f1a289
Gitweb: http://git.kernel.org/tip/88f7a890d74137ab0d126a5d65679cd620f1a289
Author: Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Wed, 30 Dec 2009 14:22:22 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 30 Dec 2009 07:50:47 +0100
ksym_tracer: Fix to make the tracer work
ksym tracer doesn't work:
# echo tasklist_lock:rw- > ksym_trace_filter
-bash: echo: write error: No such device
It's because we pass to perf_event_create_kernel_counter()
a cpu number which is not present.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <4B3AF19E.1010201@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/hw_breakpoint.c | 10 +++++++---
kernel/trace/trace_ksym.c | 1 -
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index 366eedf..48fb0bb 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -40,6 +40,7 @@
#include <linux/percpu.h>
#include <linux/sched.h>
#include <linux/init.h>
+#include <linux/cpu.h>
#include <linux/smp.h>
#include <linux/hw_breakpoint.h>
@@ -388,7 +389,8 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
if (!cpu_events)
return ERR_PTR(-ENOMEM);
- for_each_possible_cpu(cpu) {
+ get_online_cpus();
+ for_each_online_cpu(cpu) {
pevent = per_cpu_ptr(cpu_events, cpu);
bp = perf_event_create_kernel_counter(attr, cpu, -1, triggered);
@@ -399,18 +401,20 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
goto fail;
}
}
+ put_online_cpus();
return cpu_events;
fail:
- for_each_possible_cpu(cpu) {
+ for_each_online_cpu(cpu) {
pevent = per_cpu_ptr(cpu_events, cpu);
if (IS_ERR(*pevent))
break;
unregister_hw_breakpoint(*pevent);
}
+ put_online_cpus();
+
free_percpu(cpu_events);
- /* return the error if any */
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(register_wide_hw_breakpoint);
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index faf37fa..340b6ff 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -197,7 +197,6 @@ int process_new_ksym_entry(char *ksymname, int op, unsigned long addr)
entry->attr.bp_addr = addr;
entry->attr.bp_len = HW_BREAKPOINT_LEN_4;
- ret = -EAGAIN;
entry->ksym_hbp = register_wide_hw_breakpoint(&entry->attr,
ksym_hbp_handler);
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:tracing/urgent] ksym_tracer: Fix to allow writing newline to ksym_trace_filter
2009-12-30 6:23 ` [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter Li Zefan
@ 2009-12-30 9:31 ` tip-bot for Li Zefan
0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Li Zefan @ 2009-12-30 9:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, lizf, fweisbec, rostedt, tglx, mingo, prasad
Commit-ID: 3d13ec2efdb5843ad91e57b60d50b44d922cf063
Gitweb: http://git.kernel.org/tip/3d13ec2efdb5843ad91e57b60d50b44d922cf063
Author: Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Wed, 30 Dec 2009 14:23:19 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 30 Dec 2009 07:50:49 +0100
ksym_tracer: Fix to allow writing newline to ksym_trace_filter
It used to work, but now doesn't:
# echo > ksym_filter
bash: echo: write error: Invalid argument
It's caused by d954fbf0ff6b5fdfb32350e85a2f15d3db976506
("tracing: Fix wrong usage of strstrip in trace_ksyms").
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <4B3AF1D7.5040400@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/trace/trace_ksym.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 340b6ff..160a8d8 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -299,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 (!buf[0] || !strcmp(buf, "0") ||
- !strcmp(buf, "*:---")) {
+ if (!input_string[0] || !strcmp(input_string, "0") ||
+ !strcmp(input_string, "*:---")) {
__ksym_trace_reset();
ret = 0;
goto out;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:tracing/urgent] ksym_tracer: Fix race when incrementing count
2009-12-30 6:23 ` [PATCH 3/4] ksym_tracer: Fix race when incrementing count Li Zefan
@ 2009-12-30 9:31 ` tip-bot for Li Zefan
0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Li Zefan @ 2009-12-30 9:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, lizf, fweisbec, rostedt, tglx, mingo, prasad
Commit-ID: e6d9491bf8ba6728cc86aeabbc688d20ec0563b5
Gitweb: http://git.kernel.org/tip/e6d9491bf8ba6728cc86aeabbc688d20ec0563b5
Author: Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Wed, 30 Dec 2009 14:23:40 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 30 Dec 2009 07:50:49 +0100
ksym_tracer: Fix race when incrementing count
We are under rcu read section but not holding the write lock, so
count++ is not atomic. Use atomic64_t instead.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <4B3AF1EC.9010608@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/trace/trace_ksym.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 160a8d8..67d79f7 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -32,6 +32,8 @@
#include <linux/hw_breakpoint.h>
#include <asm/hw_breakpoint.h>
+#include <asm/atomic.h>
+
/*
* For now, let us restrict the no. of symbols traced simultaneously to number
* of available hardware breakpoint registers.
@@ -44,7 +46,7 @@ struct trace_ksym {
struct perf_event **ksym_hbp;
struct perf_event_attr attr;
#ifdef CONFIG_PROFILE_KSYM_TRACER
- unsigned long counter;
+ atomic64_t counter;
#endif
struct hlist_node ksym_hlist;
};
@@ -69,9 +71,8 @@ void ksym_collect_stats(unsigned long hbp_hit_addr)
rcu_read_lock();
hlist_for_each_entry_rcu(entry, node, &ksym_filter_head, ksym_hlist) {
- if ((entry->attr.bp_addr == hbp_hit_addr) &&
- (entry->counter <= MAX_UL_INT)) {
- entry->counter++;
+ if (entry->attr.bp_addr == hbp_hit_addr) {
+ atomic64_inc(&entry->counter);
break;
}
}
@@ -501,7 +502,8 @@ static int ksym_tracer_stat_show(struct seq_file *m, void *v)
seq_printf(m, " %-36s", fn_name);
else
seq_printf(m, " %-36s", "<NA>");
- seq_printf(m, " %15lu\n", entry->counter);
+ seq_printf(m, " %15llu\n",
+ (unsigned long long)atomic64_read(&entry->counter));
return 0;
}
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [tip:tracing/urgent] ksym_tracer: Remove trace_stat
2009-12-30 6:24 ` [PATCH 4/4] ksym_tracer: Remove trace_stat Li Zefan
@ 2009-12-30 9:31 ` tip-bot for Li Zefan
0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Li Zefan @ 2009-12-30 9:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, lizf, fweisbec, rostedt, tglx, mingo, prasad
Commit-ID: 53ab668064edaeef99c0ee22799483d45f4c81f6
Gitweb: http://git.kernel.org/tip/53ab668064edaeef99c0ee22799483d45f4c81f6
Author: Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Wed, 30 Dec 2009 14:24:03 +0800
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 30 Dec 2009 07:50:50 +0100
ksym_tracer: Remove trace_stat
trace_stat is problematic. Don't use it, use seqfile instead.
This fixes a race that reading the stat file is not protected by
any lock, which can lead to use after free.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <4B3AF203.40200@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/trace/trace_ksym.c | 127 ++++++++++++++++++---------------------------
1 files changed, 50 insertions(+), 77 deletions(-)
diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
index 67d79f7..94103cd 100644
--- a/kernel/trace/trace_ksym.c
+++ b/kernel/trace/trace_ksym.c
@@ -26,7 +26,6 @@
#include <linux/fs.h>
#include "trace_output.h"
-#include "trace_stat.h"
#include "trace.h"
#include <linux/hw_breakpoint.h>
@@ -444,103 +443,77 @@ struct tracer ksym_tracer __read_mostly =
.print_line = ksym_trace_output
};
-__init static int init_ksym_trace(void)
-{
- struct dentry *d_tracer;
- struct dentry *entry;
-
- d_tracer = tracing_init_dentry();
- ksym_filter_entry_count = 0;
-
- entry = debugfs_create_file("ksym_trace_filter", 0644, d_tracer,
- NULL, &ksym_tracing_fops);
- if (!entry)
- pr_warning("Could not create debugfs "
- "'ksym_trace_filter' file\n");
-
- return register_tracer(&ksym_tracer);
-}
-device_initcall(init_ksym_trace);
-
-
#ifdef CONFIG_PROFILE_KSYM_TRACER
-static int ksym_tracer_stat_headers(struct seq_file *m)
+static int ksym_profile_show(struct seq_file *m, void *v)
{
+ struct hlist_node *node;
+ struct trace_ksym *entry;
+ int access_type = 0;
+ char fn_name[KSYM_NAME_LEN];
+
seq_puts(m, " Access Type ");
seq_puts(m, " Symbol Counter\n");
seq_puts(m, " ----------- ");
seq_puts(m, " ------ -------\n");
- return 0;
-}
-static int ksym_tracer_stat_show(struct seq_file *m, void *v)
-{
- struct hlist_node *stat = v;
- struct trace_ksym *entry;
- int access_type = 0;
- char fn_name[KSYM_NAME_LEN];
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(entry, node, &ksym_filter_head, ksym_hlist) {
- entry = hlist_entry(stat, struct trace_ksym, ksym_hlist);
+ access_type = entry->attr.bp_type;
- access_type = entry->attr.bp_type;
+ switch (access_type) {
+ case HW_BREAKPOINT_R:
+ seq_puts(m, " R ");
+ break;
+ case HW_BREAKPOINT_W:
+ seq_puts(m, " W ");
+ break;
+ case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
+ seq_puts(m, " RW ");
+ break;
+ default:
+ seq_puts(m, " NA ");
+ }
- switch (access_type) {
- case HW_BREAKPOINT_R:
- seq_puts(m, " R ");
- break;
- case HW_BREAKPOINT_W:
- seq_puts(m, " W ");
- break;
- case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
- seq_puts(m, " RW ");
- break;
- default:
- seq_puts(m, " NA ");
+ if (lookup_symbol_name(entry->attr.bp_addr, fn_name) >= 0)
+ seq_printf(m, " %-36s", fn_name);
+ else
+ seq_printf(m, " %-36s", "<NA>");
+ seq_printf(m, " %15llu\n",
+ (unsigned long long)atomic64_read(&entry->counter));
}
-
- if (lookup_symbol_name(entry->attr.bp_addr, fn_name) >= 0)
- seq_printf(m, " %-36s", fn_name);
- else
- seq_printf(m, " %-36s", "<NA>");
- seq_printf(m, " %15llu\n",
- (unsigned long long)atomic64_read(&entry->counter));
+ rcu_read_unlock();
return 0;
}
-static void *ksym_tracer_stat_start(struct tracer_stat *trace)
+static int ksym_profile_open(struct inode *node, struct file *file)
{
- return ksym_filter_head.first;
-}
-
-static void *
-ksym_tracer_stat_next(void *v, int idx)
-{
- struct hlist_node *stat = v;
-
- return stat->next;
+ return single_open(file, ksym_profile_show, NULL);
}
-static struct tracer_stat ksym_tracer_stats = {
- .name = "ksym_tracer",
- .stat_start = ksym_tracer_stat_start,
- .stat_next = ksym_tracer_stat_next,
- .stat_headers = ksym_tracer_stat_headers,
- .stat_show = ksym_tracer_stat_show
+static const struct file_operations ksym_profile_fops = {
+ .open = ksym_profile_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
};
+#endif /* CONFIG_PROFILE_KSYM_TRACER */
-__init static int ksym_tracer_stat_init(void)
+__init static int init_ksym_trace(void)
{
- int ret;
+ struct dentry *d_tracer;
- ret = register_stat_tracer(&ksym_tracer_stats);
- if (ret) {
- printk(KERN_WARNING "Warning: could not register "
- "ksym tracer stats\n");
- return 1;
- }
+ d_tracer = tracing_init_dentry();
- return 0;
+ trace_create_file("ksym_trace_filter", 0644, d_tracer,
+ NULL, &ksym_tracing_fops);
+
+#ifdef CONFIG_PROFILE_KSYM_TRACER
+ trace_create_file("ksym_profile", 0444, d_tracer,
+ NULL, &ksym_profile_fops);
+#endif
+
+ return register_tracer(&ksym_tracer);
}
-fs_initcall(ksym_tracer_stat_init);
-#endif /* CONFIG_PROFILE_KSYM_TRACER */
+device_initcall(init_ksym_trace);
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] ksym_tracer: Fix to make the tracer work
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
` (3 preceding siblings ...)
2009-12-30 9:30 ` [tip:tracing/urgent] ksym_tracer: Fix to make the tracer work tip-bot for Li Zefan
@ 2009-12-30 15:05 ` K.Prasad
2009-12-31 5:54 ` Li Zefan
4 siblings, 1 reply; 11+ messages in thread
From: K.Prasad @ 2009-12-30 15:05 UTC (permalink / raw)
To: Li Zefan; +Cc: Frederic Weisbecker, Steven Rostedt, Ingo Molnar, LKML
On Wed, Dec 30, 2009 at 02:22:22PM +0800, Li Zefan wrote:
> ksym tracer doesn't work:
>
> # echo tasklist_lock:rw- > ksym_trace_filter
> -bash: echo: write error: No such device
>
> It's because we pass to perf_event_create_kernel_counter()
> a cpu number which is not present.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
> kernel/hw_breakpoint.c | 10 +++++++---
> kernel/trace/trace_ksym.c | 1 -
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
> index dbcbf6a..50dbd59 100644
> --- a/kernel/hw_breakpoint.c
> +++ b/kernel/hw_breakpoint.c
> @@ -40,6 +40,7 @@
> #include <linux/percpu.h>
> #include <linux/sched.h>
> #include <linux/init.h>
> +#include <linux/cpu.h>
> #include <linux/smp.h>
>
> #include <linux/hw_breakpoint.h>
> @@ -388,7 +389,8 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
> if (!cpu_events)
> return ERR_PTR(-ENOMEM);
>
> - for_each_possible_cpu(cpu) {
> + get_online_cpus();
> + for_each_online_cpu(cpu) {
> pevent = per_cpu_ptr(cpu_events, cpu);
> bp = perf_event_create_kernel_counter(attr, cpu, -1, triggered);
>
Frederic must have used for_each_possible_cpu() to account for CPUs that
are offline at the time of registration, but may eventually turn online.
Since register_wide_hw_breakpoint() interface is designed to deliver
system-wide breakpoints, the debug registers of a new online CPU will
should have the breakpoints populated to comprehensively notify all
memory accesses over target address.
I'd rather wait to hear from Frederic to know why
perf_event_create_kernel_counter() returns an error when run for an
offline cpu and how it can be solved.
Thanks,
K.Prasad
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] ksym_tracer: Fix to make the tracer work
2009-12-30 15:05 ` [PATCH 1/4] " K.Prasad
@ 2009-12-31 5:54 ` Li Zefan
2010-01-10 3:10 ` Frederic Weisbecker
0 siblings, 1 reply; 11+ messages in thread
From: Li Zefan @ 2009-12-31 5:54 UTC (permalink / raw)
To: prasad; +Cc: Frederic Weisbecker, Steven Rostedt, Ingo Molnar, LKML
K.Prasad wrote:
> On Wed, Dec 30, 2009 at 02:22:22PM +0800, Li Zefan wrote:
>> ksym tracer doesn't work:
>>
>> # echo tasklist_lock:rw- > ksym_trace_filter
>> -bash: echo: write error: No such device
>>
>> It's because we pass to perf_event_create_kernel_counter()
>> a cpu number which is not present.
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> ---
>> kernel/hw_breakpoint.c | 10 +++++++---
>> kernel/trace/trace_ksym.c | 1 -
>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
>> index dbcbf6a..50dbd59 100644
>> --- a/kernel/hw_breakpoint.c
>> +++ b/kernel/hw_breakpoint.c
>> @@ -40,6 +40,7 @@
>> #include <linux/percpu.h>
>> #include <linux/sched.h>
>> #include <linux/init.h>
>> +#include <linux/cpu.h>
>> #include <linux/smp.h>
>>
>> #include <linux/hw_breakpoint.h>
>> @@ -388,7 +389,8 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
>> if (!cpu_events)
>> return ERR_PTR(-ENOMEM);
>>
>> - for_each_possible_cpu(cpu) {
>> + get_online_cpus();
>> + for_each_online_cpu(cpu) {
>> pevent = per_cpu_ptr(cpu_events, cpu);
>> bp = perf_event_create_kernel_counter(attr, cpu, -1, triggered);
>>
>
> Frederic must have used for_each_possible_cpu() to account for CPUs that
> are offline at the time of registration, but may eventually turn online.
> Since register_wide_hw_breakpoint() interface is designed to deliver
> system-wide breakpoints, the debug registers of a new online CPU will
> should have the breakpoints populated to comprehensively notify all
> memory accesses over target address.
>
> I'd rather wait to hear from Frederic to know why
> perf_event_create_kernel_counter() returns an error when run for an
> offline cpu and how it can be solved.
>
See the comment in find_get_context() in kernel/perf_event.c:
/*
* We could be clever and allow to attach a event to an
* offline CPU and activate it when the CPU comes up, but
* that's for later.
*/
if (!cpu_online(cpu))
return ERR_PTR(-ENODEV);
So I think we can use for_each_possible_cpu() in the future, but not now.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] ksym_tracer: Fix to make the tracer work
2009-12-31 5:54 ` Li Zefan
@ 2010-01-10 3:10 ` Frederic Weisbecker
0 siblings, 0 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-01-10 3:10 UTC (permalink / raw)
To: Li Zefan; +Cc: prasad, Steven Rostedt, Ingo Molnar, LKML
On Thu, Dec 31, 2009 at 01:54:35PM +0800, Li Zefan wrote:
> K.Prasad wrote:
> > Frederic must have used for_each_possible_cpu() to account for CPUs that
> > are offline at the time of registration, but may eventually turn online.
> > Since register_wide_hw_breakpoint() interface is designed to deliver
> > system-wide breakpoints, the debug registers of a new online CPU will
> > should have the breakpoints populated to comprehensively notify all
> > memory accesses over target address.
> >
> > I'd rather wait to hear from Frederic to know why
> > perf_event_create_kernel_counter() returns an error when run for an
> > offline cpu and how it can be solved.
> >
>
> See the comment in find_get_context() in kernel/perf_event.c:
>
> /*
> * We could be clever and allow to attach a event to an
> * offline CPU and activate it when the CPU comes up, but
> * that's for later.
> */
> if (!cpu_online(cpu))
> return ERR_PTR(-ENODEV);
>
> So I think we can use for_each_possible_cpu() in the future, but not now.
>
Ah, right I indeed missed that.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-01-10 3:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-30 6:22 [PATCH 1/4] ksym_tracer: Fix to make the tracer work Li Zefan
2009-12-30 6:23 ` [PATCH 2/4] ksym_tracer: Fix to allow writing newline to ksym_trace_filter Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 6:23 ` [PATCH 3/4] ksym_tracer: Fix race when incrementing count Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 6:24 ` [PATCH 4/4] ksym_tracer: Remove trace_stat Li Zefan
2009-12-30 9:31 ` [tip:tracing/urgent] " tip-bot for Li Zefan
2009-12-30 9:30 ` [tip:tracing/urgent] ksym_tracer: Fix to make the tracer work tip-bot for Li Zefan
2009-12-30 15:05 ` [PATCH 1/4] " K.Prasad
2009-12-31 5:54 ` Li Zefan
2010-01-10 3:10 ` Frederic Weisbecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).