* [PATCH] ftrace: warn on failure to disable mcount callers
@ 2008-08-20 16:38 Steven Rostedt
2008-08-21 8:44 ` Ingo Molnar
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2008-08-20 16:38 UTC (permalink / raw)
To: LKML
Cc: Ingo Molnar, Thomas Gleixner, Peter Zijlstra, Andrew Morton,
Linus Torvalds
With the recent updates to ftrace, there should not be any failures when
modifying the code. If there is, then we need to warn about it.
This patch has a cleaned up version of the code that I used to discover
that the weak symbols were causing failures.
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
kernel/trace/ftrace.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
Index: linux-tip.git/kernel/trace/ftrace.c
===================================================================
--- linux-tip.git.orig/kernel/trace/ftrace.c 2008-08-20 11:56:12.000000000 -0400
+++ linux-tip.git/kernel/trace/ftrace.c 2008-08-20 12:20:45.000000000 -0400
@@ -574,6 +574,16 @@ static void ftrace_shutdown_replenish(vo
ftrace_pages->next = (void *)get_zeroed_page(GFP_KERNEL);
}
+static void print_ip_ins(const char *fmt, unsigned char *p)
+{
+ int i;
+
+ printk(KERN_CONT "%s", fmt);
+
+ for (i = 0; i < MCOUNT_INSN_SIZE; i++)
+ printk(KERN_CONT "%s%02x", i ? ":" : "", p[i]);
+}
+
static int
ftrace_code_disable(struct dyn_ftrace *rec)
{
@@ -588,6 +598,21 @@ ftrace_code_disable(struct dyn_ftrace *r
failed = ftrace_modify_code(ip, call, nop);
if (failed) {
+ switch (failed) {
+ case 1:
+ pr_info("WARNING: ftrace faulted on modifying ");
+ print_ip_sym(ip);
+ break;
+ case 2:
+ pr_info("WARNING: ftrace failed to modify ");
+ print_ip_sym(ip);
+ print_ip_ins(" expected: ", call);
+ print_ip_ins(" actual: ", (unsigned char *)ip);
+ print_ip_ins(" replace: ", nop);
+ printk(KERN_CONT "\n");
+ break;
+ }
+
rec->flags |= FTRACE_FL_FAILED;
return 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ftrace: warn on failure to disable mcount callers
2008-08-20 16:38 [PATCH] ftrace: warn on failure to disable mcount callers Steven Rostedt
@ 2008-08-21 8:44 ` Ingo Molnar
2008-08-21 14:31 ` [PATCH v2] " Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2008-08-21 8:44 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Linus Torvalds
* Steven Rostedt <rostedt@goodmis.org> wrote:
> + pr_info("WARNING: ftrace faulted on modifying ");
> + print_ip_sym(ip);
please use the WARN() API, so that it gets picked up by automation.
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] ftrace: warn on failure to disable mcount callers
2008-08-21 8:44 ` Ingo Molnar
@ 2008-08-21 14:31 ` Steven Rostedt
2008-08-22 6:46 ` Ingo Molnar
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2008-08-21 14:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Linus Torvalds
On Thu, 21 Aug 2008, Ingo Molnar wrote:
>
> * Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > + pr_info("WARNING: ftrace faulted on modifying ");
> > + print_ip_sym(ip);
>
> please use the WARN() API, so that it gets picked up by automation.
>
> Ingo
>
[
Added WARN_ON as requested above.
]
With the recent updates to ftrace, there should not be any failures when
modifying the code. If there is, then we need to warn about it.
This patch has a cleaned up version of the code that I used to discover
that the weak symbols were causing failures.
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
kernel/trace/ftrace.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
Index: linux-tip.git/kernel/trace/ftrace.c
===================================================================
--- linux-tip.git.orig/kernel/trace/ftrace.c 2008-08-20 21:58:19.000000000 -0400
+++ linux-tip.git/kernel/trace/ftrace.c 2008-08-21 10:19:09.000000000 -0400
@@ -574,6 +574,16 @@ static void ftrace_shutdown_replenish(vo
ftrace_pages->next = (void *)get_zeroed_page(GFP_KERNEL);
}
+static void print_ip_ins(const char *fmt, unsigned char *p)
+{
+ int i;
+
+ printk(KERN_CONT "%s", fmt);
+
+ for (i = 0; i < MCOUNT_INSN_SIZE; i++)
+ printk(KERN_CONT "%s%02x", i ? ":" : "", p[i]);
+}
+
static int
ftrace_code_disable(struct dyn_ftrace *rec)
{
@@ -588,6 +598,23 @@ ftrace_code_disable(struct dyn_ftrace *r
failed = ftrace_modify_code(ip, call, nop);
if (failed) {
+ switch (failed) {
+ case 1:
+ WARN_ON_ONCE(1);
+ pr_info("ftrace faulted on modifying ");
+ print_ip_sym(ip);
+ break;
+ case 2:
+ WARN_ON_ONCE(1);
+ pr_info("ftrace failed to modify ");
+ print_ip_sym(ip);
+ print_ip_ins(" expected: ", call);
+ print_ip_ins(" actual: ", (unsigned char *)ip);
+ print_ip_ins(" replace: ", nop);
+ printk(KERN_CONT "\n");
+ break;
+ }
+
rec->flags |= FTRACE_FL_FAILED;
return 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ftrace: warn on failure to disable mcount callers
2008-08-21 14:31 ` [PATCH v2] " Steven Rostedt
@ 2008-08-22 6:46 ` Ingo Molnar
2008-08-22 13:00 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2008-08-22 6:46 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Linus Torvalds
* Steven Rostedt <rostedt@goodmis.org> wrote:
> + WARN_ON_ONCE(1);
> + pr_info("ftrace faulted on modifying ");
> + print_ip_sym(ip);
> + break;
> + case 2:
> + WARN_ON_ONCE(1);
> + pr_info("ftrace failed to modify ");
> + print_ip_sym(ip);
> + print_ip_ins(" expected: ", call);
> + print_ip_ins(" actual: ", (unsigned char *)ip);
> + print_ip_ins(" replace: ", nop);
> + printk(KERN_CONT "\n");
> + break;
hm, i think it makes little sense to only print out the stacktrace once,
but to print out the rest all the time.
If there's such a failure then ftrace should warn once, with stacktrace
and everthing else, and turn itself off permanently. That makes it sure
that we 1) get the report 2) dont spam the user 3) keep the system
working 4) turn off the malfunctioning component.
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ftrace: warn on failure to disable mcount callers
2008-08-22 6:46 ` Ingo Molnar
@ 2008-08-22 13:00 ` Steven Rostedt
0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2008-08-22 13:00 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Thomas Gleixner, Peter Zijlstra, Andrew Morton, Linus Torvalds
On Fri, 22 Aug 2008, Ingo Molnar wrote:
>
> * Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > + WARN_ON_ONCE(1);
> > + pr_info("ftrace faulted on modifying ");
> > + print_ip_sym(ip);
> > + break;
> > + case 2:
> > + WARN_ON_ONCE(1);
> > + pr_info("ftrace failed to modify ");
> > + print_ip_sym(ip);
> > + print_ip_ins(" expected: ", call);
> > + print_ip_ins(" actual: ", (unsigned char *)ip);
> > + print_ip_ins(" replace: ", nop);
> > + printk(KERN_CONT "\n");
> > + break;
>
> hm, i think it makes little sense to only print out the stacktrace once,
> but to print out the rest all the time.
Actually, the more functions that are printed the easier it is to find
where things went wrong. The backtrace is only to trigger automated tools,
and helps very little in diagnosing the problem.
I had to examine several functions to figure out the issue with the weak
symbols.
>
> If there's such a failure then ftrace should warn once, with stacktrace
> and everthing else, and turn itself off permanently. That makes it sure
> that we 1) get the report 2) dont spam the user 3) keep the system
> working 4) turn off the malfunctioning component.
The backtrace should give us 1 without a problem. In fact if we do 2,
they are more likely to complain ;-)
I think 3 and 4 are related, and I agree, if this is detected, we turn
ftrace off.
Note, this only prints out on boot up and module loading. The spamming
should not be that bad.
-- Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-08-22 13:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-20 16:38 [PATCH] ftrace: warn on failure to disable mcount callers Steven Rostedt
2008-08-21 8:44 ` Ingo Molnar
2008-08-21 14:31 ` [PATCH v2] " Steven Rostedt
2008-08-22 6:46 ` Ingo Molnar
2008-08-22 13:00 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).