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

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

[Masami Hiramatsu]

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: mhiramat@redhat.com

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  2d5e067edc4635ff7515bfa9ab3edb38bc344cab
Gitweb:     http://git.kernel.org/tip/2d5e067edc4635ff7515bfa9ab3edb38bc344cab
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Mon, 14 Sep 2009 16:48:56 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <20090914204856.18779.52961.stgit@dhcp-100-2-132.bos.redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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;

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[Steven Rostedt]

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 <mhiramat@redhat.com>
> Cc: Jim Keniston <jkenisto@us.ibm.com>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Frank Ch. Eigler <fche@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: K.Prasad <prasad@linux.vnet.ibm.com>
> Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
> Cc: Li Zefan <lizf@cn.fujitsu.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Tom Zanussi <tzanussi@gmail.com>
> ---
> 
>  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 */
> 
> 

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>
>> Cc: Jim Keniston <jkenisto@us.ibm.com>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: Frank Ch. Eigler <fche@redhat.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: H. Peter Anvin <hpa@zytor.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Jason Baron <jbaron@redhat.com>
>> Cc: K.Prasad <prasad@linux.vnet.ibm.com>
>> Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
>> Cc: Li Zefan <lizf@cn.fujitsu.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Tom Zanussi <tzanussi@gmail.com>
>> ---
>>
>>  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: mhiramat@redhat.com

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

[Steven Rostedt]

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

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>

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 <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[Steven Rostedt]

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 <rostedt@goodmis.org> for the whole series.

-- Steve

> 
> ---
> ftrace: Fix trace_add_event_call() to initialize list
> 
> From: Masami Hiramatsu <mhiramat@redhat.com>
> 
> 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 <mhiramat@redhat.com>

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  588bebb74fe87270f94c2810652bd683d63c4b54
Gitweb:     http://git.kernel.org/tip/588bebb74fe87270f94c2810652bd683d63c4b54
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Wed, 16 Sep 2009 11:42:55 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <4AB1077F.6020107@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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 */

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[Steven Rostedt]

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);
>  }
>  
> 
> 

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

[Frederic Weisbecker]

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.

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

[Ananth N Mavinakayanahalli]

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

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

[Frederic Weisbecker]

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!

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

[Masami Hiramatsu]

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: mhiramat@redhat.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  4fead8e46fded93cc0d432ced774d9a3a8d21bad
Gitweb:     http://git.kernel.org/tip/4fead8e46fded93cc0d432ced774d9a3a8d21bad
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Mon, 14 Sep 2009 16:49:12 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <20090914204912.18779.68734.stgit@dhcp-100-2-132.bos.redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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);
 }
 

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  50d780560785b068c358675c5f0bf6c83b5c373e
Gitweb:     http://git.kernel.org/tip/50d780560785b068c358675c5f0bf6c83b5c373e
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Mon, 14 Sep 2009 16:49:20 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <20090914204920.18779.57555.stgit@dhcp-100-2-132.bos.redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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)
 {

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

[Masami Hiramatsu]

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

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  74ebb63e7cd25f6fb02a45fc2ea7735bce1217c9
Gitweb:     http://git.kernel.org/tip/74ebb63e7cd25f6fb02a45fc2ea7735bce1217c9
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Mon, 14 Sep 2009 16:49:28 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <20090914204928.18779.60029.stgit@dhcp-100-2-132.bos.redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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];

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

[Masami Hiramatsu]

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 <mhiramat@redhat.com>
c: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
---

 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: mhiramat@redhat.com

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

[tip-bot for Masami Hiramatsu]

Commit-ID:  5a0d9050db4d1147722b42afef9011251b2651ee
Gitweb:     http://git.kernel.org/tip/5a0d9050db4d1147722b42afef9011251b2651ee
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Mon, 14 Sep 2009 16:49:37 -0400
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
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 <mhiramat@redhat.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jason Baron <jbaron@redhat.com>
Cc: K.Prasad <prasad@linux.vnet.ibm.com>
Cc: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <20090914204937.18779.59422.stgit@dhcp-100-2-132.bos.redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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;
 

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

[Frederic Weisbecker]

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

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

[Masami Hiramatsu]

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: mhiramat@redhat.com