2009-09-14 20:47:15

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 0/6] tracing/kprobes: kprobe-based event tracer update and perf support

Hi,

Here are bugfixes for kprobe-based event tracer. Thank you for review
and reporting bugs! I fixed it. And also I decided to disable
kprobe event when defining, since the events outputs may suddenly
mess up the ftrace ring_buffer.

Frederic, could you see the patch 4/6? which is what I said on the
previous threads.


Thank you,

---

Masami Hiramatsu (6):
tracing/kprobes: Disable kprobe events by default
tracing/kprobes: Fix profiling alignment for perf_counter buffer
tracing/kprobes: Add probe handler dispatcher for support perf and ftrace
ftrace: Fix trace_remove_event_call() to lock trace_event_mutex
ftrace: Fix trace_add_event_call() to initialize list
tracing/kprobes: Fix trace_probe registration order


Documentation/trace/kprobetrace.txt | 11 ++-
kernel/trace/trace_events.c | 11 ++-
kernel/trace/trace_kprobe.c | 146 +++++++++++++++++++++++------------
3 files changed, 114 insertions(+), 54 deletions(-)

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]


2009-09-14 20:47:26

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 1/6] tracing/kprobes: Fix trace_probe registration order

Fix trace_probe registration order. ftrace_event_call and ftrace_event
must be registered before kprobe/kretprobe, because tracing/profiling
handlers use event-id.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_kprobe.c | 42 +++++++++++++++++++-----------------------
1 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cbc0870..ea0db8e 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -347,20 +347,15 @@ static struct trace_probe *find_probe_event(const char *event)
return NULL;
}

-static void __unregister_trace_probe(struct trace_probe *tp)
+/* Unregister a trace_probe and probe_event: call with locking probe_lock */
+static void unregister_trace_probe(struct trace_probe *tp)
{
if (probe_is_return(tp))
unregister_kretprobe(&tp->rp);
else
unregister_kprobe(&tp->rp.kp);
-}
-
-/* Unregister a trace_probe and probe_event: call with locking probe_lock */
-static void unregister_trace_probe(struct trace_probe *tp)
-{
- unregister_probe_event(tp);
- __unregister_trace_probe(tp);
list_del(&tp->list);
+ unregister_probe_event(tp);
}

/* Register a trace_probe and probe_event */
@@ -371,6 +366,19 @@ static int register_trace_probe(struct trace_probe *tp)

mutex_lock(&probe_lock);

+ /* register as an event */
+ old_tp = find_probe_event(tp->call.name);
+ if (old_tp) {
+ /* delete old event */
+ unregister_trace_probe(old_tp);
+ free_trace_probe(old_tp);
+ }
+ ret = register_probe_event(tp);
+ if (ret) {
+ pr_warning("Faild to register probe event(%d)\n", ret);
+ goto end;
+ }
+
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -384,21 +392,9 @@ static int register_trace_probe(struct trace_probe *tp)
tp->rp.kp.addr);
ret = -EINVAL;
}
- goto end;
- }
- /* register as an event */
- old_tp = find_probe_event(tp->call.name);
- if (old_tp) {
- /* delete old event */
- unregister_trace_probe(old_tp);
- free_trace_probe(old_tp);
- }
- ret = register_probe_event(tp);
- if (ret) {
- pr_warning("Faild to register probe event(%d)\n", ret);
- __unregister_trace_probe(tp);
- }
- list_add_tail(&tp->list, &probe_list);
+ unregister_probe_event(tp);
+ } else
+ list_add_tail(&tp->list, &probe_list);
end:
mutex_unlock(&probe_lock);
return ret;


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-14 20:47:34

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list

Initialize event_call.list and handle failuer path in trace_add_event_call()
for fixing below bug which occurred when I tried to add invalid event twice.

Could not create debugfs 'kmalloc' directory
Failed to register kprobe event: kmalloc
Faild to register probe event(-1)
------------[ cut here ]------------
WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
__list_add+0x27/0x5c()
Hardware name:
list_add corruption. next->prev should be prev (c07d78cc), but was
00001000. (next=d854236c).
Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
scsi_wait_scan]
Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
Call Trace:
[<c0438424>] warn_slowpath_common+0x65/0x7c
[<c05371b3>] ? __list_add+0x27/0x5c
[<c043846f>] warn_slowpath_fmt+0x24/0x27
[<c05371b3>] __list_add+0x27/0x5c
[<c047f050>] list_add+0xa/0xc
[<c047f8f5>] trace_add_event_call+0x60/0x97
[<c0483133>] command_trace_probe+0x42c/0x51b
[<c044a1b3>] ? remove_wait_queue+0x22/0x27
[<c042a9c0>] ? __wake_up+0x32/0x3b
[<c04832f6>] probes_write+0xd4/0x10a
[<c0483222>] ? probes_write+0x0/0x10a
[<c04b27a9>] vfs_write+0x80/0xdf
[<c04b289c>] sys_write+0x3b/0x5d
[<c0670d41>] syscall_call+0x7/0xb
---[ end trace 2b962b5dc1fdc07d ]---

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_events.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index ba34920..38e82a5 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
if (!d_events)
return -ENOENT;

+ INIT_LIST_HEAD(&call->list);
list_add(&call->list, &ftrace_events);
- return event_create_dir(call, d_events, &ftrace_event_id_fops,
+ ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
&ftrace_enable_fops, &ftrace_event_filter_fops,
&ftrace_event_format_fops);
+ if (ret < 0)
+ list_del(&call->list);
+ return ret;
}

/* Add an additional event_call dynamically */


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-14 20:47:43

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

Lock not only event_mutex but also trace_event_mutex in
trace_remove_event_call() for protecting __unregister_ftrace_event().

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_events.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 38e82a5..a3d6bad 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1055,6 +1055,9 @@ static void remove_subsystem_dir(const char *name)
}
}

+/*
+ * Must be called under locking both of event_mutex and trace_event_mutex.
+ */
static void __trace_remove_event_call(struct ftrace_event_call *call)
{
ftrace_event_enable_disable(call, 0);
@@ -1071,7 +1074,9 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
void trace_remove_event_call(struct ftrace_event_call *call)
{
mutex_lock(&event_mutex);
+ down_write(&trace_event_mutex);
__trace_remove_event_call(call);
+ up_write(&trace_event_mutex);
mutex_unlock(&event_mutex);
}



--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-14 20:47:58

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 4/6] tracing/kprobes: Add probe handler dispatcher for support perf and ftrace

Add kprobe_dispatcher and kretprobe_dispatcher for dispatching event
to both of profile handler and tracing handler.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_kprobe.c | 85 ++++++++++++++++++++++++++++++++-----------
1 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index ea0db8e..70b632c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -185,10 +185,15 @@ struct probe_arg {
const char *name;
};

+/* Flags for trace_probe */
+#define TP_FLAG_TRACE 1
+#define TP_FLAG_PROFILE 2
+
struct trace_probe {
struct list_head list;
struct kretprobe rp; /* Use rp.kp for kprobe use */
unsigned long nhit;
+ unsigned int flags; /* For TP_FLAG_* */
const char *symbol; /* symbol name */
struct ftrace_event_call call;
struct trace_event event;
@@ -200,10 +205,6 @@ struct trace_probe {
(offsetof(struct trace_probe, args) + \
(sizeof(struct probe_arg) * (n)))

-static int kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs);
-static int kretprobe_trace_func(struct kretprobe_instance *ri,
- struct pt_regs *regs);
-
static __kprobes int probe_is_return(struct trace_probe *tp)
{
return tp->rp.handler != NULL;
@@ -263,6 +264,10 @@ static void unregister_probe_event(struct trace_probe *tp);
static DEFINE_MUTEX(probe_lock);
static LIST_HEAD(probe_list);

+static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
+static int kretprobe_dispatcher(struct kretprobe_instance *ri,
+ struct pt_regs *regs);
+
/*
* Allocate new trace_probe and initialize it (including kprobes).
*/
@@ -288,11 +293,10 @@ static struct trace_probe *alloc_trace_probe(const char *group,
} else
tp->rp.kp.addr = addr;

- /* Set handler here for checking whether this probe is return or not. */
if (is_return)
- tp->rp.handler = kretprobe_trace_func;
+ tp->rp.handler = kretprobe_dispatcher;
else
- tp->rp.kp.pre_handler = kprobe_trace_func;
+ tp->rp.kp.pre_handler = kprobe_dispatcher;

if (!event)
goto error;
@@ -379,6 +383,7 @@ static int register_trace_probe(struct trace_probe *tp)
goto end;
}

+ tp->flags = TP_FLAG_TRACE;
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -987,23 +992,24 @@ static int probe_event_enable(struct ftrace_event_call *call)
{
struct trace_probe *tp = (struct trace_probe *)call->data;

- if (probe_is_return(tp)) {
- tp->rp.handler = kretprobe_trace_func;
+ tp->flags |= TP_FLAG_TRACE;
+ if (probe_is_return(tp))
return enable_kretprobe(&tp->rp);
- } else {
- tp->rp.kp.pre_handler = kprobe_trace_func;
+ else
return enable_kprobe(&tp->rp.kp);
- }
}

static void probe_event_disable(struct ftrace_event_call *call)
{
struct trace_probe *tp = (struct trace_probe *)call->data;

- if (probe_is_return(tp))
- disable_kretprobe(&tp->rp);
- else
- disable_kprobe(&tp->rp.kp);
+ tp->flags &= ~TP_FLAG_TRACE;
+ if (!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE))) {
+ if (probe_is_return(tp))
+ disable_kretprobe(&tp->rp);
+ else
+ disable_kprobe(&tp->rp.kp);
+ }
}

static int probe_event_raw_init(struct ftrace_event_call *event_call)
@@ -1212,22 +1218,57 @@ static int probe_profile_enable(struct ftrace_event_call *call)
if (atomic_inc_return(&call->profile_count))
return 0;

- if (probe_is_return(tp)) {
- tp->rp.handler = kretprobe_profile_func;
+ tp->flags |= TP_FLAG_PROFILE;
+ if (probe_is_return(tp))
return enable_kretprobe(&tp->rp);
- } else {
- tp->rp.kp.pre_handler = kprobe_profile_func;
+ else
return enable_kprobe(&tp->rp.kp);
- }
}

static void probe_profile_disable(struct ftrace_event_call *call)
{
+ struct trace_probe *tp = (struct trace_probe *)call->data;
+
if (atomic_add_negative(-1, &call->profile_count))
- probe_event_disable(call);
+ tp->flags &= ~TP_FLAG_PROFILE;
+
+ if (!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE))) {
+ if (probe_is_return(tp))
+ disable_kretprobe(&tp->rp);
+ else
+ disable_kprobe(&tp->rp.kp);
+ }
}
+#endif /* CONFIG_EVENT_PROFILE */
+
+
+static __kprobes
+int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
+{
+ struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);

+ if (tp->flags & TP_FLAG_TRACE)
+ kprobe_trace_func(kp, regs);
+#ifdef CONFIG_EVENT_PROFILE
+ if (tp->flags & TP_FLAG_PROFILE)
+ kprobe_profile_func(kp, regs);
#endif /* CONFIG_EVENT_PROFILE */
+ return 0; /* We don't tweek kernel, so just return 0 */
+}
+
+static __kprobes
+int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
+
+ if (tp->flags & TP_FLAG_TRACE)
+ kretprobe_trace_func(ri, regs);
+#ifdef CONFIG_EVENT_PROFILE
+ if (tp->flags & TP_FLAG_PROFILE)
+ kretprobe_profile_func(ri, regs);
+#endif /* CONFIG_EVENT_PROFILE */
+ return 0; /* We don't tweek kernel, so just return 0 */
+}

static int register_probe_event(struct trace_probe *tp)
{


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-14 20:48:06

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 5/6] tracing/kprobes: Fix profiling alignment for perf_counter buffer

Fix *probe_profile_func() to align buffer size, since pref_counter requires.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_kprobe.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 70b632c..d8db935 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1149,18 +1149,23 @@ static __kprobes int kprobe_profile_func(struct kprobe *kp,
struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
struct ftrace_event_call *call = &tp->call;
struct kprobe_trace_entry *entry;
- int size, i, pc;
+ int size, __size, i, pc;
unsigned long irq_flags;

local_save_flags(irq_flags);
pc = preempt_count();

- size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+ __size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+ size = ALIGN(__size + sizeof(u32), sizeof(u64));
+ size -= sizeof(u32);

do {
char raw_data[size];
struct trace_entry *ent;
-
+ /*
+ * Zero dead bytes from alignment to avoid stack leak
+ * to userspace
+ */
*(u64 *)(&raw_data[size - sizeof(u64)]) = 0ULL;
entry = (struct kprobe_trace_entry *)raw_data;
ent = &entry->ent;
@@ -1183,13 +1188,15 @@ static __kprobes int kretprobe_profile_func(struct kretprobe_instance *ri,
struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
struct ftrace_event_call *call = &tp->call;
struct kretprobe_trace_entry *entry;
- int size, i, pc;
+ int size, __size, i, pc;
unsigned long irq_flags;

local_save_flags(irq_flags);
pc = preempt_count();

- size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+ __size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+ size = ALIGN(__size + sizeof(u32), sizeof(u64));
+ size -= sizeof(u32);

do {
char raw_data[size];


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-14 20:48:15

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 6/6] tracing/kprobes: Disable kprobe events by default

Disable newly created kprobe events by default, not to disturb
another user using ftrace. "Disturb" means when someone using
ftrace and another user tries to use perf-tools, (in near
future) if he defines new kprobe event via perf-tools, then new
events will mess up the frace buffer. I think that's ugly.


Signed-off-by: Masami Hiramatsu <[email protected]>
c: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

Documentation/trace/kprobetrace.txt | 11 +++++++++--
kernel/trace/trace_kprobe.c | 4 ++--
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 6521681..9b8f7c6 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -122,8 +122,15 @@ print fmt: "(%lx) dfd=%lx filename=%lx flags=%lx mode=%lx", REC->ip, REC->dfd, R

echo > /sys/kernel/debug/tracing/kprobe_events

- This clears all probe points. and you can see the traced information via
-/sys/kernel/debug/tracing/trace.
+ This clears all probe points.
+
+ Right after definition, each event is disabled by default. For tracing these
+events, you need to enable it.
+
+ echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable
+ echo 1 > /sys/kernel/debug/tracing/events/kprobes/myretprobe/enable
+
+ And you can see the traced information via /sys/kernel/debug/tracing/trace.

cat /sys/kernel/debug/tracing/trace
# tracer: nop
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d8db935..f6821f1 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -383,7 +383,7 @@ static int register_trace_probe(struct trace_probe *tp)
goto end;
}

- tp->flags = TP_FLAG_TRACE;
+ tp->rp.kp.flags |= KPROBE_FLAG_DISABLED;
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -1298,7 +1298,7 @@ static int register_probe_event(struct trace_probe *tp)
call->id = register_ftrace_event(&tp->event);
if (!call->id)
return -ENODEV;
- call->enabled = 1;
+ call->enabled = 0;
call->regfunc = probe_event_enable;
call->unregfunc = probe_event_disable;



--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-15 23:51:24

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list

On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:
> Initialize event_call.list and handle failuer path in trace_add_event_call()
> for fixing below bug which occurred when I tried to add invalid event twice.
>
> Could not create debugfs 'kmalloc' directory
> Failed to register kprobe event: kmalloc
> Faild to register probe event(-1)
> ------------[ cut here ]------------
> WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
> __list_add+0x27/0x5c()
> Hardware name:
> list_add corruption. next->prev should be prev (c07d78cc), but was
> 00001000. (next=d854236c).
> Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
> i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
> scsi_wait_scan]
> Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
> Call Trace:
> [<c0438424>] warn_slowpath_common+0x65/0x7c
> [<c05371b3>] ? __list_add+0x27/0x5c
> [<c043846f>] warn_slowpath_fmt+0x24/0x27
> [<c05371b3>] __list_add+0x27/0x5c
> [<c047f050>] list_add+0xa/0xc
> [<c047f8f5>] trace_add_event_call+0x60/0x97
> [<c0483133>] command_trace_probe+0x42c/0x51b
> [<c044a1b3>] ? remove_wait_queue+0x22/0x27
> [<c042a9c0>] ? __wake_up+0x32/0x3b
> [<c04832f6>] probes_write+0xd4/0x10a
> [<c0483222>] ? probes_write+0x0/0x10a
> [<c04b27a9>] vfs_write+0x80/0xdf
> [<c04b289c>] sys_write+0x3b/0x5d
> [<c0670d41>] syscall_call+0x7/0xb
> ---[ end trace 2b962b5dc1fdc07d ]---
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Jim Keniston <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Frank Ch. Eigler <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Jason Baron <[email protected]>
> Cc: K.Prasad <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
> Cc: Li Zefan <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Srikar Dronamraju <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Tom Zanussi <[email protected]>
> ---
>
> kernel/trace/trace_events.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index ba34920..38e82a5 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
> if (!d_events)
> return -ENOENT;
>
> + INIT_LIST_HEAD(&call->list);

The INIT_LIST_HEAD is not needed here. The list_add will assign it.


> list_add(&call->list, &ftrace_events);
> - return event_create_dir(call, d_events, &ftrace_event_id_fops,
> + ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
> &ftrace_enable_fops, &ftrace_event_filter_fops,
> &ftrace_event_format_fops);
> + if (ret < 0)
> + list_del(&call->list);

But this I can see is needed ;-)

-- Steve

> + return ret;
> }
>
> /* Add an additional event_call dynamically */
>
>

2009-09-16 03:17:36

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:

>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 38e82a5..a3d6bad 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1055,6 +1055,9 @@ static void remove_subsystem_dir(const char *name)
> }
> }
>
> +/*
> + * Must be called under locking both of event_mutex and trace_event_mutex.
> + */
> static void __trace_remove_event_call(struct ftrace_event_call *call)
> {
> ftrace_event_enable_disable(call, 0);
> @@ -1071,7 +1074,9 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
> void trace_remove_event_call(struct ftrace_event_call *call)

Is this from a previous patch set, because I can't find this in either
my tree or tip/master.

-- Steve

> {
> mutex_lock(&event_mutex);
> + down_write(&trace_event_mutex);
> __trace_remove_event_call(call);
> + up_write(&trace_event_mutex);
> mutex_unlock(&event_mutex);
> }
>
>
>

2009-09-16 04:05:18

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

On Tue, Sep 15, 2009 at 11:17:30PM -0400, Steven Rostedt wrote:
> On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:
>
> >
> > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> > index 38e82a5..a3d6bad 100644
> > --- a/kernel/trace/trace_events.c
> > +++ b/kernel/trace/trace_events.c
> > @@ -1055,6 +1055,9 @@ static void remove_subsystem_dir(const char *name)
> > }
> > }
> >
> > +/*
> > + * Must be called under locking both of event_mutex and trace_event_mutex.
> > + */
> > static void __trace_remove_event_call(struct ftrace_event_call *call)
> > {
> > ftrace_event_enable_disable(call, 0);
> > @@ -1071,7 +1074,9 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
> > void trace_remove_event_call(struct ftrace_event_call *call)
>
> Is this from a previous patch set, because I can't find this in either
> my tree or tip/master.
>
> -- Steve


It's in tip:/tracing/kprobes, which is not merged into tip/master.
I have some pending updates in my tracing/kprobes tree:

Frederic Weisbecker (1):
Merge commit 'tracing/core' into tracing/kprobes

Masami Hiramatsu (16):
kprobes/x86: Call BUG() when reentering probe into KPROBES_HIT_SS
kprobes/x86-64: Allow to reenter probe on post_handler
kprobes/x86: Fix to add __kprobes to in-kernel fault handing functions
kprobes: Fix to add __kprobes to notify_die
kprobes/x86-64: Fix to move common_interrupt to .kprobes.text
kprobes: Prohibit to probe native_get_debugreg
x86: Allow x86-32 instruction decoder selftest on x86-64
x86: Remove unused config macros from instruction decoder selftest
x86: Add MMX support for instruction decoder
kprobes/x86-32: Move irq-exit functions to kprobes section
x86/ptrace: Fix regs_get_argument_nth() to add correct offset
tracing/kprobes: Fix probe offset to be unsigned
tracing/kprobes: Cleanup kprobe tracer code.
tracing/kprobes: Add event profiling support
tracing/kprobes: Add argument name support
tracing/kprobes: Show event name in trace output

Plus the current set that I'm about to apply, if no problem arises.
I should also merge one more time tip:tracing/core into it because
it has developed another conflicts against it since then.

Ingo, if you are fine with it, I'll send you a pull request
soon.

Thanks,
Frederic.

Subject: Re: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

On Wed, Sep 16, 2009 at 06:05:12AM +0200, Frederic Weisbecker wrote:
> On Tue, Sep 15, 2009 at 11:17:30PM -0400, Steven Rostedt wrote:
> > On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:

...

> It's in tip:/tracing/kprobes, which is not merged into tip/master.
> I have some pending updates in my tracing/kprobes tree:
>
> Frederic Weisbecker (1):
> Merge commit 'tracing/core' into tracing/kprobes
>
> Masami Hiramatsu (16):
> kprobes/x86: Call BUG() when reentering probe into KPROBES_HIT_SS
> kprobes/x86-64: Allow to reenter probe on post_handler
> kprobes/x86: Fix to add __kprobes to in-kernel fault handing functions
> kprobes: Fix to add __kprobes to notify_die
> kprobes/x86-64: Fix to move common_interrupt to .kprobes.text
> kprobes: Prohibit to probe native_get_debugreg
> x86: Allow x86-32 instruction decoder selftest on x86-64
> x86: Remove unused config macros from instruction decoder selftest
> x86: Add MMX support for instruction decoder
> kprobes/x86-32: Move irq-exit functions to kprobes section
> x86/ptrace: Fix regs_get_argument_nth() to add correct offset
> tracing/kprobes: Fix probe offset to be unsigned
> tracing/kprobes: Cleanup kprobe tracer code.
> tracing/kprobes: Add event profiling support
> tracing/kprobes: Add argument name support
> tracing/kprobes: Show event name in trace output
>
> Plus the current set that I'm about to apply, if no problem arises.
> I should also merge one more time tip:tracing/core into it because
> it has developed another conflicts against it since then.
>
> Ingo, if you are fine with it, I'll send you a pull request
> soon.

Frederic,

Could you please add http://lkml.org/lkml/2009/9/15/13 too?

Ananth

2009-09-16 05:33:35

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

On Wed, Sep 16, 2009 at 10:24:20AM +0530, Ananth N Mavinakayanahalli wrote:
> On Wed, Sep 16, 2009 at 06:05:12AM +0200, Frederic Weisbecker wrote:
> > On Tue, Sep 15, 2009 at 11:17:30PM -0400, Steven Rostedt wrote:
> > > On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:
>
> ...
>
> > It's in tip:/tracing/kprobes, which is not merged into tip/master.
> > I have some pending updates in my tracing/kprobes tree:
> >
> > Frederic Weisbecker (1):
> > Merge commit 'tracing/core' into tracing/kprobes
> >
> > Masami Hiramatsu (16):
> > kprobes/x86: Call BUG() when reentering probe into KPROBES_HIT_SS
> > kprobes/x86-64: Allow to reenter probe on post_handler
> > kprobes/x86: Fix to add __kprobes to in-kernel fault handing functions
> > kprobes: Fix to add __kprobes to notify_die
> > kprobes/x86-64: Fix to move common_interrupt to .kprobes.text
> > kprobes: Prohibit to probe native_get_debugreg
> > x86: Allow x86-32 instruction decoder selftest on x86-64
> > x86: Remove unused config macros from instruction decoder selftest
> > x86: Add MMX support for instruction decoder
> > kprobes/x86-32: Move irq-exit functions to kprobes section
> > x86/ptrace: Fix regs_get_argument_nth() to add correct offset
> > tracing/kprobes: Fix probe offset to be unsigned
> > tracing/kprobes: Cleanup kprobe tracer code.
> > tracing/kprobes: Add event profiling support
> > tracing/kprobes: Add argument name support
> > tracing/kprobes: Show event name in trace output
> >
> > Plus the current set that I'm about to apply, if no problem arises.
> > I should also merge one more time tip:tracing/core into it because
> > it has developed another conflicts against it since then.
> >
> > Ingo, if you are fine with it, I'll send you a pull request
> > soon.
>
> Frederic,
>
> Could you please add http://lkml.org/lkml/2009/9/15/13 too?
>
> Ananth


Sure, I put it in the queue, thanks!

2009-09-16 14:16:02

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list

Steven Rostedt wrote:
> On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:
>> Initialize event_call.list and handle failuer path in trace_add_event_call()
>> for fixing below bug which occurred when I tried to add invalid event twice.
>>
>> Could not create debugfs 'kmalloc' directory
>> Failed to register kprobe event: kmalloc
>> Faild to register probe event(-1)
>> ------------[ cut here ]------------
>> WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
>> __list_add+0x27/0x5c()
>> Hardware name:
>> list_add corruption. next->prev should be prev (c07d78cc), but was
>> 00001000. (next=d854236c).
>> Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
>> i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
>> scsi_wait_scan]
>> Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
>> Call Trace:
>> [<c0438424>] warn_slowpath_common+0x65/0x7c
>> [<c05371b3>] ? __list_add+0x27/0x5c
>> [<c043846f>] warn_slowpath_fmt+0x24/0x27
>> [<c05371b3>] __list_add+0x27/0x5c
>> [<c047f050>] list_add+0xa/0xc
>> [<c047f8f5>] trace_add_event_call+0x60/0x97
>> [<c0483133>] command_trace_probe+0x42c/0x51b
>> [<c044a1b3>] ? remove_wait_queue+0x22/0x27
>> [<c042a9c0>] ? __wake_up+0x32/0x3b
>> [<c04832f6>] probes_write+0xd4/0x10a
>> [<c0483222>] ? probes_write+0x0/0x10a
>> [<c04b27a9>] vfs_write+0x80/0xdf
>> [<c04b289c>] sys_write+0x3b/0x5d
>> [<c0670d41>] syscall_call+0x7/0xb
>> ---[ end trace 2b962b5dc1fdc07d ]---
>>
>> Signed-off-by: Masami Hiramatsu <[email protected]>
>> Cc: Jim Keniston <[email protected]>
>> Cc: Ananth N Mavinakayanahalli <[email protected]>
>> Cc: Andi Kleen <[email protected]>
>> Cc: Christoph Hellwig <[email protected]>
>> Cc: Frank Ch. Eigler <[email protected]>
>> Cc: Frederic Weisbecker <[email protected]>
>> Cc: H. Peter Anvin <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Jason Baron <[email protected]>
>> Cc: K.Prasad <[email protected]>
>> Cc: Lai Jiangshan <[email protected]>
>> Cc: Li Zefan <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Srikar Dronamraju <[email protected]>
>> Cc: Steven Rostedt <[email protected]>
>> Cc: Tom Zanussi <[email protected]>
>> ---
>>
>> kernel/trace/trace_events.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>> index ba34920..38e82a5 100644
>> --- a/kernel/trace/trace_events.c
>> +++ b/kernel/trace/trace_events.c
>> @@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
>> if (!d_events)
>> return -ENOENT;
>>
>> + INIT_LIST_HEAD(&call->list);
>
> The INIT_LIST_HEAD is not needed here. The list_add will assign it.

Without initializing it, list debugging code warns always :-)
Please see, __list_add()@lib/list_debug.c

Thank you,


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-16 14:29:32

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list

On Wed, 2009-09-16 at 10:17 -0400, Masami Hiramatsu wrote:
> Steven Rostedt wrote:

> >> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> >> index ba34920..38e82a5 100644
> >> --- a/kernel/trace/trace_events.c
> >> +++ b/kernel/trace/trace_events.c
> >> @@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
> >> if (!d_events)
> >> return -ENOENT;
> >>
> >> + INIT_LIST_HEAD(&call->list);
> >
> > The INIT_LIST_HEAD is not needed here. The list_add will assign it.
>
> Without initializing it, list debugging code warns always :-)
> Please see, __list_add()@lib/list_debug.c

/me looks

from: include/linux/list.h

static inline void list_add(struct list_head *new, struct list_head *head)
{
__list_add(new, head, head->next);
}

from: lib/list_debug.c

void __list_add(struct list_head *new,
struct list_head *prev,
struct list_head *next)
{
WARN(next->prev != prev,
"list_add corruption. next->prev should be "
"prev (%p), but was %p. (next=%p).\n",
prev, next->prev, next);
WARN(prev->next != next,
"list_add corruption. prev->next should be "
"next (%p), but was %p. (prev=%p).\n",
next, prev->next, prev);
next->prev = new;
new->next = next;
new->prev = prev;
prev->next = new;
}

What you pass in is:

list_add(&call->list, &ftrace_events);

new = &call->list;
prev = &ftrace_events->prev;
next = &ftrace_events->next;

The above code never tests "new". The INIT_LIST_HEAD is useless.

-- Steve

2009-09-16 14:30:04

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 3/6] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

Steven Rostedt wrote:
> On Mon, 2009-09-14 at 16:49 -0400, Masami Hiramatsu wrote:
>
>>
>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>> index 38e82a5..a3d6bad 100644
>> --- a/kernel/trace/trace_events.c
>> +++ b/kernel/trace/trace_events.c
>> @@ -1055,6 +1055,9 @@ static void remove_subsystem_dir(const char *name)
>> }
>> }
>>
>> +/*
>> + * Must be called under locking both of event_mutex and trace_event_mutex.
>> + */
>> static void __trace_remove_event_call(struct ftrace_event_call *call)
>> {
>> ftrace_event_enable_disable(call, 0);
>> @@ -1071,7 +1074,9 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
>> void trace_remove_event_call(struct ftrace_event_call *call)
>
> Is this from a previous patch set, because I can't find this in either
> my tree or tip/master.

Right, this bug is from my patch:
tracing: Ftrace dynamic ftrace_event_call support

Thanks,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-16 15:39:53

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list take 2

Steven Rostedt wrote:
> On Wed, 2009-09-16 at 10:17 -0400, Masami Hiramatsu wrote:
>> Steven Rostedt wrote:
>
>>>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>>>> index ba34920..38e82a5 100644
>>>> --- a/kernel/trace/trace_events.c
>>>> +++ b/kernel/trace/trace_events.c
>>>> @@ -1009,10 +1009,14 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
>>>> if (!d_events)
>>>> return -ENOENT;
>>>>
>>>> + INIT_LIST_HEAD(&call->list);
>>>
>>> The INIT_LIST_HEAD is not needed here. The list_add will assign it.
>>
>> Without initializing it, list debugging code warns always :-)
>> Please see, __list_add()@lib/list_debug.c
>
> /me looks
>
> from: include/linux/list.h
>
> static inline void list_add(struct list_head *new, struct list_head *head)
> {
> __list_add(new, head, head->next);
> }
>
> from: lib/list_debug.c
>
> void __list_add(struct list_head *new,
> struct list_head *prev,
> struct list_head *next)
> {
> WARN(next->prev != prev,
> "list_add corruption. next->prev should be "
> "prev (%p), but was %p. (next=%p).\n",
> prev, next->prev, next);
> WARN(prev->next != next,
> "list_add corruption. prev->next should be "
> "next (%p), but was %p. (prev=%p).\n",
> next, prev->next, prev);
> next->prev = new;
> new->next = next;
> new->prev = prev;
> prev->next = new;
> }
>
> What you pass in is:
>
> list_add(&call->list,&ftrace_events);
>
> new =&call->list;
> prev =&ftrace_events->prev;
> next =&ftrace_events->next;
>
> The above code never tests "new". The INIT_LIST_HEAD is useless.

Oh, I see, I misunderstood. Thank you for pointing it out. :)
Here, I updated the patch.

---
ftrace: Fix trace_add_event_call() to initialize list

From: Masami Hiramatsu <[email protected]>

Handle failuer path in trace_add_event_call() for fixing below bug
which occurred when I tried to add invalid event twice.

Could not create debugfs 'kmalloc' directory
Failed to register kprobe event: kmalloc
Faild to register probe event(-1)
------------[ cut here ]------------
WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
__list_add+0x27/0x5c()
Hardware name:
list_add corruption. next->prev should be prev (c07d78cc), but was
00001000. (next=d854236c).
Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
scsi_wait_scan]
Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
Call Trace:
[<c0438424>] warn_slowpath_common+0x65/0x7c
[<c05371b3>] ? __list_add+0x27/0x5c
[<c043846f>] warn_slowpath_fmt+0x24/0x27
[<c05371b3>] __list_add+0x27/0x5c
[<c047f050>] list_add+0xa/0xc
[<c047f8f5>] trace_add_event_call+0x60/0x97
[<c0483133>] command_trace_probe+0x42c/0x51b
[<c044a1b3>] ? remove_wait_queue+0x22/0x27
[<c042a9c0>] ? __wake_up+0x32/0x3b
[<c04832f6>] probes_write+0xd4/0x10a
[<c0483222>] ? probes_write+0x0/0x10a
[<c04b27a9>] vfs_write+0x80/0xdf
[<c04b289c>] sys_write+0x3b/0x5d
[<c0670d41>] syscall_call+0x7/0xb
---[ end trace 2b962b5dc1fdc07d ]---

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Tom Zanussi <[email protected]>
---

kernel/trace/trace_events.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)


diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index ba34920..83cc2c0 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1010,9 +1010,12 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
return -ENOENT;

list_add(&call->list, &ftrace_events);
- return event_create_dir(call, d_events, &ftrace_event_id_fops,
+ ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
&ftrace_enable_fops, &ftrace_event_filter_fops,
&ftrace_event_format_fops);
+ if (ret < 0)
+ list_del(&call->list);
+ return ret;
}

/* Add an additional event_call dynamically */


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-09-16 15:43:30

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 2/6] ftrace: Fix trace_add_event_call() to initialize list take 2

On Wed, 2009-09-16 at 11:42 -0400, Masami Hiramatsu wrote:

> Oh, I see, I misunderstood. Thank you for pointing it out. :)
> Here, I updated the patch.

Thanks!

Acked-by: Steven Rostedt <[email protected]> for the whole series.

-- Steve

>
> ---
> ftrace: Fix trace_add_event_call() to initialize list
>
> From: Masami Hiramatsu <[email protected]>
>
> Handle failuer path in trace_add_event_call() for fixing below bug
> which occurred when I tried to add invalid event twice.
>
> Could not create debugfs 'kmalloc' directory
> Failed to register kprobe event: kmalloc
> Faild to register probe event(-1)
> ------------[ cut here ]------------
> WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
> __list_add+0x27/0x5c()
> Hardware name:
> list_add corruption. next->prev should be prev (c07d78cc), but was
> 00001000. (next=d854236c).
> Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
> i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
> scsi_wait_scan]
> Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
> Call Trace:
> [<c0438424>] warn_slowpath_common+0x65/0x7c
> [<c05371b3>] ? __list_add+0x27/0x5c
> [<c043846f>] warn_slowpath_fmt+0x24/0x27
> [<c05371b3>] __list_add+0x27/0x5c
> [<c047f050>] list_add+0xa/0xc
> [<c047f8f5>] trace_add_event_call+0x60/0x97
> [<c0483133>] command_trace_probe+0x42c/0x51b
> [<c044a1b3>] ? remove_wait_queue+0x22/0x27
> [<c042a9c0>] ? __wake_up+0x32/0x3b
> [<c04832f6>] probes_write+0xd4/0x10a
> [<c0483222>] ? probes_write+0x0/0x10a
> [<c04b27a9>] vfs_write+0x80/0xdf
> [<c04b289c>] sys_write+0x3b/0x5d
> [<c0670d41>] syscall_call+0x7/0xb
> ---[ end trace 2b962b5dc1fdc07d ]---
>
> Signed-off-by: Masami Hiramatsu <[email protected]>

2009-09-17 02:39:19

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 0/6] tracing/kprobes: kprobe-based event tracer update and perf support

On Mon, Sep 14, 2009 at 04:48:47PM -0400, Masami Hiramatsu wrote:
> Hi,
>
> Here are bugfixes for kprobe-based event tracer. Thank you for review
> and reporting bugs! I fixed it. And also I decided to disable
> kprobe event when defining, since the events outputs may suddenly
> mess up the ftrace ring_buffer.
>
> Frederic, could you see the patch 4/6? which is what I said on the
> previous threads.
>
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (6):
> tracing/kprobes: Disable kprobe events by default
> tracing/kprobes: Fix profiling alignment for perf_counter buffer
> tracing/kprobes: Add probe handler dispatcher for support perf and ftrace
> ftrace: Fix trace_remove_event_call() to lock trace_event_mutex
> ftrace: Fix trace_add_event_call() to initialize list
> tracing/kprobes: Fix trace_probe registration order



Applied in

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
tracing/kprobes

I've included Ananth fix too.

This time it looks like I haven't lost the last patch.
But I should double-check each morning as I suspect I become
a sleepwalker who does random git-reset in midnight...

2009-09-17 17:29:48

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH tracing/kprobes 0/6] tracing/kprobes: kprobe-based event tracer update and perf support

Frederic Weisbecker wrote:
> On Mon, Sep 14, 2009 at 04:48:47PM -0400, Masami Hiramatsu wrote:
>> Hi,
>>
>> Here are bugfixes for kprobe-based event tracer. Thank you for review
>> and reporting bugs! I fixed it. And also I decided to disable
>> kprobe event when defining, since the events outputs may suddenly
>> mess up the ftrace ring_buffer.
>>
>> Frederic, could you see the patch 4/6? which is what I said on the
>> previous threads.
>>
>>
>> Thank you,
>>
>> ---
>>
>> Masami Hiramatsu (6):
>> tracing/kprobes: Disable kprobe events by default
>> tracing/kprobes: Fix profiling alignment for perf_counter buffer
>> tracing/kprobes: Add probe handler dispatcher for support perf and ftrace
>> ftrace: Fix trace_remove_event_call() to lock trace_event_mutex
>> ftrace: Fix trace_add_event_call() to initialize list
>> tracing/kprobes: Fix trace_probe registration order
>
>
>
> Applied in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> tracing/kprobes
>
> I've included Ananth fix too.
>
> This time it looks like I haven't lost the last patch.
> But I should double-check each morning as I suspect I become
> a sleepwalker who does random git-reset in midnight...

Thanks!
I've double checked. Please sleep in peace :-)


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-10-17 10:01:29

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Fix trace_probe registration order

Commit-ID: 2d5e067edc4635ff7515bfa9ab3edb38bc344cab
Gitweb: http://git.kernel.org/tip/2d5e067edc4635ff7515bfa9ab3edb38bc344cab
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 14 Sep 2009 16:48:56 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:03:40 +0200

tracing/kprobes: Fix trace_probe registration order

Fix trace_probe registration order. ftrace_event_call and ftrace_event
must be registered before kprobe/kretprobe, because tracing/profiling
handlers dereference the event-id.

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_kprobe.c | 42 +++++++++++++++++++-----------------------
1 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cbc0870..ea0db8e 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -347,20 +347,15 @@ static struct trace_probe *find_probe_event(const char *event)
return NULL;
}

-static void __unregister_trace_probe(struct trace_probe *tp)
+/* Unregister a trace_probe and probe_event: call with locking probe_lock */
+static void unregister_trace_probe(struct trace_probe *tp)
{
if (probe_is_return(tp))
unregister_kretprobe(&tp->rp);
else
unregister_kprobe(&tp->rp.kp);
-}
-
-/* Unregister a trace_probe and probe_event: call with locking probe_lock */
-static void unregister_trace_probe(struct trace_probe *tp)
-{
- unregister_probe_event(tp);
- __unregister_trace_probe(tp);
list_del(&tp->list);
+ unregister_probe_event(tp);
}

/* Register a trace_probe and probe_event */
@@ -371,6 +366,19 @@ static int register_trace_probe(struct trace_probe *tp)

mutex_lock(&probe_lock);

+ /* register as an event */
+ old_tp = find_probe_event(tp->call.name);
+ if (old_tp) {
+ /* delete old event */
+ unregister_trace_probe(old_tp);
+ free_trace_probe(old_tp);
+ }
+ ret = register_probe_event(tp);
+ if (ret) {
+ pr_warning("Faild to register probe event(%d)\n", ret);
+ goto end;
+ }
+
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -384,21 +392,9 @@ static int register_trace_probe(struct trace_probe *tp)
tp->rp.kp.addr);
ret = -EINVAL;
}
- goto end;
- }
- /* register as an event */
- old_tp = find_probe_event(tp->call.name);
- if (old_tp) {
- /* delete old event */
- unregister_trace_probe(old_tp);
- free_trace_probe(old_tp);
- }
- ret = register_probe_event(tp);
- if (ret) {
- pr_warning("Faild to register probe event(%d)\n", ret);
- __unregister_trace_probe(tp);
- }
- list_add_tail(&tp->list, &probe_list);
+ unregister_probe_event(tp);
+ } else
+ list_add_tail(&tp->list, &probe_list);
end:
mutex_unlock(&probe_lock);
return ret;

2009-10-17 10:01:50

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] ftrace: Fix trace_add_event_call() to initialize list

Commit-ID: 588bebb74fe87270f94c2810652bd683d63c4b54
Gitweb: http://git.kernel.org/tip/588bebb74fe87270f94c2810652bd683d63c4b54
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 16 Sep 2009 11:42:55 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:03:46 +0200

ftrace: Fix trace_add_event_call() to initialize list

Handle failure path in trace_add_event_call() to fix the below bug
which occurred when I tried to add invalid event twice.

Could not create debugfs 'kmalloc' directory
Failed to register kprobe event: kmalloc
Faild to register probe event(-1)
------------[ cut here ]------------
WARNING: at /home/mhiramat/ksrc/random-tracing/lib/list_debug.c:26
__list_add+0x27/0x5c()
Hardware name:
list_add corruption. next->prev should be prev (c07d78cc), but was
00001000. (next=d854236c).
Modules linked in: sunrpc uinput virtio_net virtio_balloon i2c_piix4 pcspkr
i2c_core virtio_blk virtio_pci virtio_ring virtio [last unloaded:
scsi_wait_scan]
Pid: 1394, comm: tee Not tainted 2.6.31-rc9 #51
Call Trace:
[<c0438424>] warn_slowpath_common+0x65/0x7c
[<c05371b3>] ? __list_add+0x27/0x5c
[<c043846f>] warn_slowpath_fmt+0x24/0x27
[<c05371b3>] __list_add+0x27/0x5c
[<c047f050>] list_add+0xa/0xc
[<c047f8f5>] trace_add_event_call+0x60/0x97
[<c0483133>] command_trace_probe+0x42c/0x51b
[<c044a1b3>] ? remove_wait_queue+0x22/0x27
[<c042a9c0>] ? __wake_up+0x32/0x3b
[<c04832f6>] probes_write+0xd4/0x10a
[<c0483222>] ? probes_write+0x0/0x10a
[<c04b27a9>] vfs_write+0x80/0xdf
[<c04b289c>] sys_write+0x3b/0x5d
[<c0670d41>] syscall_call+0x7/0xb
---[ end trace 2b962b5dc1fdc07d ]---

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index ba34920..83cc2c0 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1010,9 +1010,12 @@ static int __trace_add_event_call(struct ftrace_event_call *call)
return -ENOENT;

list_add(&call->list, &ftrace_events);
- return event_create_dir(call, d_events, &ftrace_event_id_fops,
+ ret = event_create_dir(call, d_events, &ftrace_event_id_fops,
&ftrace_enable_fops, &ftrace_event_filter_fops,
&ftrace_event_format_fops);
+ if (ret < 0)
+ list_del(&call->list);
+ return ret;
}

/* Add an additional event_call dynamically */

2009-10-17 10:02:16

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

Commit-ID: 4fead8e46fded93cc0d432ced774d9a3a8d21bad
Gitweb: http://git.kernel.org/tip/4fead8e46fded93cc0d432ced774d9a3a8d21bad
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 14 Sep 2009 16:49:12 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:03:50 +0200

ftrace: Fix trace_remove_event_call() to lock trace_event_mutex

Lock not only event_mutex but also trace_event_mutex in
trace_remove_event_call() to protect __unregister_ftrace_event().

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_events.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 83cc2c0..f85b0f1 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1054,6 +1054,9 @@ static void remove_subsystem_dir(const char *name)
}
}

+/*
+ * Must be called under locking both of event_mutex and trace_event_mutex.
+ */
static void __trace_remove_event_call(struct ftrace_event_call *call)
{
ftrace_event_enable_disable(call, 0);
@@ -1070,7 +1073,9 @@ static void __trace_remove_event_call(struct ftrace_event_call *call)
void trace_remove_event_call(struct ftrace_event_call *call)
{
mutex_lock(&event_mutex);
+ down_write(&trace_event_mutex);
__trace_remove_event_call(call);
+ up_write(&trace_event_mutex);
mutex_unlock(&event_mutex);
}

2009-10-17 10:02:10

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Add probe handler dispatcher to support perf and ftrace concurrent use

Commit-ID: 50d780560785b068c358675c5f0bf6c83b5c373e
Gitweb: http://git.kernel.org/tip/50d780560785b068c358675c5f0bf6c83b5c373e
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 14 Sep 2009 16:49:20 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:03:54 +0200

tracing/kprobes: Add probe handler dispatcher to support perf and ftrace concurrent use

Add kprobe_dispatcher and kretprobe_dispatcher to dispatch event
in both profile and tracing handlers.

This allows simultaneous kprobe uses by ftrace and perf.

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_kprobe.c | 85 ++++++++++++++++++++++++++++++++-----------
1 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index ea0db8e..70b632c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -185,10 +185,15 @@ struct probe_arg {
const char *name;
};

+/* Flags for trace_probe */
+#define TP_FLAG_TRACE 1
+#define TP_FLAG_PROFILE 2
+
struct trace_probe {
struct list_head list;
struct kretprobe rp; /* Use rp.kp for kprobe use */
unsigned long nhit;
+ unsigned int flags; /* For TP_FLAG_* */
const char *symbol; /* symbol name */
struct ftrace_event_call call;
struct trace_event event;
@@ -200,10 +205,6 @@ struct trace_probe {
(offsetof(struct trace_probe, args) + \
(sizeof(struct probe_arg) * (n)))

-static int kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs);
-static int kretprobe_trace_func(struct kretprobe_instance *ri,
- struct pt_regs *regs);
-
static __kprobes int probe_is_return(struct trace_probe *tp)
{
return tp->rp.handler != NULL;
@@ -263,6 +264,10 @@ static void unregister_probe_event(struct trace_probe *tp);
static DEFINE_MUTEX(probe_lock);
static LIST_HEAD(probe_list);

+static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
+static int kretprobe_dispatcher(struct kretprobe_instance *ri,
+ struct pt_regs *regs);
+
/*
* Allocate new trace_probe and initialize it (including kprobes).
*/
@@ -288,11 +293,10 @@ static struct trace_probe *alloc_trace_probe(const char *group,
} else
tp->rp.kp.addr = addr;

- /* Set handler here for checking whether this probe is return or not. */
if (is_return)
- tp->rp.handler = kretprobe_trace_func;
+ tp->rp.handler = kretprobe_dispatcher;
else
- tp->rp.kp.pre_handler = kprobe_trace_func;
+ tp->rp.kp.pre_handler = kprobe_dispatcher;

if (!event)
goto error;
@@ -379,6 +383,7 @@ static int register_trace_probe(struct trace_probe *tp)
goto end;
}

+ tp->flags = TP_FLAG_TRACE;
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -987,23 +992,24 @@ static int probe_event_enable(struct ftrace_event_call *call)
{
struct trace_probe *tp = (struct trace_probe *)call->data;

- if (probe_is_return(tp)) {
- tp->rp.handler = kretprobe_trace_func;
+ tp->flags |= TP_FLAG_TRACE;
+ if (probe_is_return(tp))
return enable_kretprobe(&tp->rp);
- } else {
- tp->rp.kp.pre_handler = kprobe_trace_func;
+ else
return enable_kprobe(&tp->rp.kp);
- }
}

static void probe_event_disable(struct ftrace_event_call *call)
{
struct trace_probe *tp = (struct trace_probe *)call->data;

- if (probe_is_return(tp))
- disable_kretprobe(&tp->rp);
- else
- disable_kprobe(&tp->rp.kp);
+ tp->flags &= ~TP_FLAG_TRACE;
+ if (!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE))) {
+ if (probe_is_return(tp))
+ disable_kretprobe(&tp->rp);
+ else
+ disable_kprobe(&tp->rp.kp);
+ }
}

static int probe_event_raw_init(struct ftrace_event_call *event_call)
@@ -1212,22 +1218,57 @@ static int probe_profile_enable(struct ftrace_event_call *call)
if (atomic_inc_return(&call->profile_count))
return 0;

- if (probe_is_return(tp)) {
- tp->rp.handler = kretprobe_profile_func;
+ tp->flags |= TP_FLAG_PROFILE;
+ if (probe_is_return(tp))
return enable_kretprobe(&tp->rp);
- } else {
- tp->rp.kp.pre_handler = kprobe_profile_func;
+ else
return enable_kprobe(&tp->rp.kp);
- }
}

static void probe_profile_disable(struct ftrace_event_call *call)
{
+ struct trace_probe *tp = (struct trace_probe *)call->data;
+
if (atomic_add_negative(-1, &call->profile_count))
- probe_event_disable(call);
+ tp->flags &= ~TP_FLAG_PROFILE;
+
+ if (!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE))) {
+ if (probe_is_return(tp))
+ disable_kretprobe(&tp->rp);
+ else
+ disable_kprobe(&tp->rp.kp);
+ }
}
+#endif /* CONFIG_EVENT_PROFILE */
+
+
+static __kprobes
+int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
+{
+ struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);

+ if (tp->flags & TP_FLAG_TRACE)
+ kprobe_trace_func(kp, regs);
+#ifdef CONFIG_EVENT_PROFILE
+ if (tp->flags & TP_FLAG_PROFILE)
+ kprobe_profile_func(kp, regs);
#endif /* CONFIG_EVENT_PROFILE */
+ return 0; /* We don't tweek kernel, so just return 0 */
+}
+
+static __kprobes
+int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
+
+ if (tp->flags & TP_FLAG_TRACE)
+ kretprobe_trace_func(ri, regs);
+#ifdef CONFIG_EVENT_PROFILE
+ if (tp->flags & TP_FLAG_PROFILE)
+ kretprobe_profile_func(ri, regs);
+#endif /* CONFIG_EVENT_PROFILE */
+ return 0; /* We don't tweek kernel, so just return 0 */
+}

static int register_probe_event(struct trace_probe *tp)
{

2009-10-17 10:02:29

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Fix profiling alignment for perf_counter buffer

Commit-ID: 74ebb63e7cd25f6fb02a45fc2ea7735bce1217c9
Gitweb: http://git.kernel.org/tip/74ebb63e7cd25f6fb02a45fc2ea7735bce1217c9
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 14 Sep 2009 16:49:28 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:03:57 +0200

tracing/kprobes: Fix profiling alignment for perf_counter buffer

Fix *probe_profile_func() to align buffer size, since perf_counter
requires its buffer entries to be 8 bytes aligned.

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/trace/trace_kprobe.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 70b632c..d8db935 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1149,18 +1149,23 @@ static __kprobes int kprobe_profile_func(struct kprobe *kp,
struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
struct ftrace_event_call *call = &tp->call;
struct kprobe_trace_entry *entry;
- int size, i, pc;
+ int size, __size, i, pc;
unsigned long irq_flags;

local_save_flags(irq_flags);
pc = preempt_count();

- size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+ __size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+ size = ALIGN(__size + sizeof(u32), sizeof(u64));
+ size -= sizeof(u32);

do {
char raw_data[size];
struct trace_entry *ent;
-
+ /*
+ * Zero dead bytes from alignment to avoid stack leak
+ * to userspace
+ */
*(u64 *)(&raw_data[size - sizeof(u64)]) = 0ULL;
entry = (struct kprobe_trace_entry *)raw_data;
ent = &entry->ent;
@@ -1183,13 +1188,15 @@ static __kprobes int kretprobe_profile_func(struct kretprobe_instance *ri,
struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
struct ftrace_event_call *call = &tp->call;
struct kretprobe_trace_entry *entry;
- int size, i, pc;
+ int size, __size, i, pc;
unsigned long irq_flags;

local_save_flags(irq_flags);
pc = preempt_count();

- size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+ __size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+ size = ALIGN(__size + sizeof(u32), sizeof(u64));
+ size -= sizeof(u32);

do {
char raw_data[size];

2009-10-17 10:02:56

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Disable kprobe events by default after creation

Commit-ID: 5a0d9050db4d1147722b42afef9011251b2651ee
Gitweb: http://git.kernel.org/tip/5a0d9050db4d1147722b42afef9011251b2651ee
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 14 Sep 2009 16:49:37 -0400
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Thu, 17 Sep 2009 04:04:01 +0200

tracing/kprobes: Disable kprobe events by default after creation

Disable newly created kprobe events by default, not to disturb
another user using ftrace. "Disturb" means when someone is using
ftrace and another user tries to use perf-tools, (in near
future) if he defines new kprobe event via perf-tools, then new
events will mess up the frace buffer. Fix this to allow proper
and transparent kprobes events concurrent usage between ftrace
users and perf users.

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
Documentation/trace/kprobetrace.txt | 11 +++++++++--
kernel/trace/trace_kprobe.c | 4 ++--
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 6521681..9b8f7c6 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -122,8 +122,15 @@ print fmt: "(%lx) dfd=%lx filename=%lx flags=%lx mode=%lx", REC->ip, REC->dfd, R

echo > /sys/kernel/debug/tracing/kprobe_events

- This clears all probe points. and you can see the traced information via
-/sys/kernel/debug/tracing/trace.
+ This clears all probe points.
+
+ Right after definition, each event is disabled by default. For tracing these
+events, you need to enable it.
+
+ echo 1 > /sys/kernel/debug/tracing/events/kprobes/myprobe/enable
+ echo 1 > /sys/kernel/debug/tracing/events/kprobes/myretprobe/enable
+
+ And you can see the traced information via /sys/kernel/debug/tracing/trace.

cat /sys/kernel/debug/tracing/trace
# tracer: nop
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d8db935..f6821f1 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -383,7 +383,7 @@ static int register_trace_probe(struct trace_probe *tp)
goto end;
}

- tp->flags = TP_FLAG_TRACE;
+ tp->rp.kp.flags |= KPROBE_FLAG_DISABLED;
if (probe_is_return(tp))
ret = register_kretprobe(&tp->rp);
else
@@ -1298,7 +1298,7 @@ static int register_probe_event(struct trace_probe *tp)
call->id = register_ftrace_event(&tp->event);
if (!call->id)
return -ENODEV;
- call->enabled = 1;
+ call->enabled = 0;
call->regfunc = probe_event_enable;
call->unregfunc = probe_event_disable;