[PATCH 0/3 v2] [GIT PULL][v3.0] tracing: fixes

[PATCH 0/3 v2] [GIT PULL][v3.0] tracing: fixes

[Steven Rostedt]

Ingo,

Please pull the latest tip/perf/urgent-2 tree, which can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/urgent-2


GuoWen Li (1):
      ftrace: Fix possible undefined return code

Steven Rostedt (2):
      kprobes/trace: Fix kprobe selftest for gcc 4.6
      ftrace: Revert 8ab2b7efd ftrace: Remove unnecessary disabling of irqs

----
 kernel/trace/ftrace.c       |    9 ++++++++-
 kernel/trace/trace_kprobe.c |    8 ++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

[PATCH 1/3 v2] ftrace: Fix possible undefined return code

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-Fix-possible-undefined-return-code.patch --]
[-- Type: text/plain, Size: 847 bytes --]

From: GuoWen Li <guowen.li.linux@gmail.com>

kernel/trace/ftrace.c: In function 'ftrace_regex_write.clone.15':
kernel/trace/ftrace.c:2743:6: warning: 'ret' may be used uninitialized in this
function

Signed-off-by: GuoWen Li <guowen.li.linux@gmail.com>
Link: http://lkml.kernel.org/r/201106011918.47939.guowen.li.linux@gmail.com
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 1ee417f..204b3eb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2740,7 +2740,7 @@ static int ftrace_process_regex(struct ftrace_hash *hash,
 {
 	char *func, *command, *next = buff;
 	struct ftrace_func_command *p;
-	int ret;
+	int ret = -EINVAL;
 
 	func = strsep(&next, ":");
 
-- 
1.7.4.4

[PATCH 2/3 v2] kprobes/trace: Fix kprobe selftest for gcc 4.6

[Steven Rostedt]

[-- Attachment #1: 0002-kprobes-trace-Fix-kprobe-selftest-for-gcc-4.6.patch --]
[-- Type: text/plain, Size: 1263 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

With gcc 4.6, the self test kprobe function:

 kprobe_trace_selftest_target()

is optimized such that kallsyms does not list it. The kprobes
test uses this function to insert a probe and test it. But
it will fail the test if the function is not listed in kallsyms.

Adding a __used annotation keeps the symbol in the kallsyms table.

Suggested-by: David Daney <ddaney@caviumnetworks.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_kprobe.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index f925c45..27d13b3 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1870,8 +1870,12 @@ fs_initcall(init_kprobe_trace);
 
 #ifdef CONFIG_FTRACE_STARTUP_TEST
 
-static int kprobe_trace_selftest_target(int a1, int a2, int a3,
-					int a4, int a5, int a6)
+/*
+ * The "__used" keeps gcc from removing the function symbol
+ * from the kallsyms table.
+ */
+static __used int kprobe_trace_selftest_target(int a1, int a2, int a3,
+					       int a4, int a5, int a6)
 {
 	return a1 + a2 + a3 + a4 + a5 + a6;
 }
-- 
1.7.4.4

Re: [PATCH 2/3 v2] kprobes/trace: Fix kprobe selftest for gcc 4.6

[Masami Hiramatsu]

(2011/06/08 6:29), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> With gcc 4.6, the self test kprobe function:
> 
>  kprobe_trace_selftest_target()
> 
> is optimized such that kallsyms does not list it. The kprobes
> test uses this function to insert a probe and test it. But
> it will fail the test if the function is not listed in kallsyms.
> 
> Adding a __used annotation keeps the symbol in the kallsyms table.

Thanks Steven and David,

That's good to me!

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

> 
> Suggested-by: David Daney <ddaney@caviumnetworks.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_kprobe.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index f925c45..27d13b3 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1870,8 +1870,12 @@ fs_initcall(init_kprobe_trace);
>  
>  #ifdef CONFIG_FTRACE_STARTUP_TEST
>  
> -static int kprobe_trace_selftest_target(int a1, int a2, int a3,
> -					int a4, int a5, int a6)
> +/*
> + * The "__used" keeps gcc from removing the function symbol
> + * from the kallsyms table.
> + */
> +static __used int kprobe_trace_selftest_target(int a1, int a2, int a3,
> +					       int a4, int a5, int a6)
>  {
>  	return a1 + a2 + a3 + a4 + a5 + a6;
>  }


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH 3/3 v2] ftrace: Revert 8ab2b7efd ftrace: Remove unnecessary disabling of

[Steven Rostedt]

[-- Attachment #1: 0003-ftrace-Revert-8ab2b7efd-ftrace-Remove-unnecessary-di.patch irqs --]
[-- Type: text/plain, Size: 1537 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Revert the commit that removed the disabling of interrupts around
the initial modifying of mcount callers to nops, and update the comment.

The original comment was outdated and stated that the interrupts were
being disabled to prevent kstop machine, which was required with the
old ftrace daemon, but was no longer the case.

What the comment failed to mention was that interrupts needed to be
disabled to keep interrupts from preempting the modifying of the code
and then executing the code that was partially modified.

Revert the commit and update the comment.

Reported-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 204b3eb..908038f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3330,6 +3330,7 @@ static int ftrace_process_locs(struct module *mod,
 {
 	unsigned long *p;
 	unsigned long addr;
+	unsigned long flags;
 
 	mutex_lock(&ftrace_lock);
 	p = start;
@@ -3346,7 +3347,13 @@ static int ftrace_process_locs(struct module *mod,
 		ftrace_record_ip(addr);
 	}
 
+	/*
+	 * Disable interrupts to prevent interrupts from executing
+	 * code that is being modified.
+	 */
+	local_irq_save(flags);
 	ftrace_update_code(mod);
+	local_irq_restore(flags);
 	mutex_unlock(&ftrace_lock);
 
 	return 0;
-- 
1.7.4.4

Re: [PATCH 3/3 v2] ftrace: Revert 8ab2b7efd ftrace: Remove unnecessary disabling of

[Richard W.M. Jones]

On Tue, Jun 07, 2011 at 05:29:40PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Revert the commit that removed the disabling of interrupts around
> the initial modifying of mcount callers to nops, and update the comment.
> 
> The original comment was outdated and stated that the interrupts were
> being disabled to prevent kstop machine, which was required with the
> old ftrace daemon, but was no longer the case.
> 
> What the comment failed to mention was that interrupts needed to be
> disabled to keep interrupts from preempting the modifying of the code
> and then executing the code that was partially modified.
> 
> Revert the commit and update the comment.
> 
> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> Tested-by: Richard W.M. Jones <rjones@redhat.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  kernel/trace/ftrace.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 204b3eb..908038f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3330,6 +3330,7 @@ static int ftrace_process_locs(struct module *mod,
>  {
>  	unsigned long *p;
>  	unsigned long addr;
> +	unsigned long flags;
>  
>  	mutex_lock(&ftrace_lock);
>  	p = start;
> @@ -3346,7 +3347,13 @@ static int ftrace_process_locs(struct module *mod,
>  		ftrace_record_ip(addr);
>  	}
>  
> +	/*
> +	 * Disable interrupts to prevent interrupts from executing
> +	 * code that is being modified.
> +	 */
> +	local_irq_save(flags);
>  	ftrace_update_code(mod);
> +	local_irq_restore(flags);
>  	mutex_unlock(&ftrace_lock);
>  
>  	return 0;
> -- 
> 1.7.4.4

ACK as before.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

Re: [PATCH 3/3 v2] ftrace: Revert 8ab2b7efd ftrace: Remove unnecessary disabling of

[Steven Rostedt]

On Tue, 2011-06-07 at 22:38 +0100, Richard W.M. Jones wrote:
> On Tue, Jun 07, 2011 at 05:29:40PM -0400, Steven Rostedt wrote:

> > 1.7.4.4
> 
> ACK as before.

I added your tested by, but never added your Acked-by. Sorry :-p

-- Steve

Re: [PATCH 0/3 v2] [GIT PULL][v3.0] tracing: fixes

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> Please pull the latest tip/perf/urgent-2 tree, which can be found at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/perf/urgent-2
> 
> 
> GuoWen Li (1):
>       ftrace: Fix possible undefined return code
> 
> Steven Rostedt (2):
>       kprobes/trace: Fix kprobe selftest for gcc 4.6
>       ftrace: Revert 8ab2b7efd ftrace: Remove unnecessary disabling of irqs
> 
> ----
>  kernel/trace/ftrace.c       |    9 ++++++++-
>  kernel/trace/trace_kprobe.c |    8 ++++++--
>  2 files changed, 14 insertions(+), 3 deletions(-)

Pulled, thanks a lot Steve!

Thanks,

	Ingo