On Mon, 17 Feb 2014 16:22:49 +0100
Petr Mladek <pmla...@suse.cz> wrote:
> Ftrace modifies function calls using Int3 breakpoints on x86. It patches
> all functions in parallel to reduce the number of sync() calls. There is
> a code that removes pending Int3 breakpoints when something goes wrong.
>
> The recovery does not work on x86_64. I simulated an error in
> ftrace_replace_code() and the system got rebooted.
Thanks for the report, I just did the same and it caused a reboot too.
>
> This patch set fixes the recovery code, improves debugging of this
> type of problem, and does some clean up.
>
> BTW: It is an echo from the patch set that tried to use text_poke_bp()
> instead of the ftrace-specific Int3 based patching code. Ftrace has
> some specific restrictions. I did not find a way how to make the
> universal text_poke_bp effective, safe, and clean to be usable
> there.
>
> The patch set is against linux/tip. Last commit is a5b3cca53c43c3ba7
> (Merge tag 'v3.14-rc3')
>
> Petr Mladek (4):
> x86: Clean up remove_breakpoint() in ftrace code
> x86: Fix ftrace patching recovery code to work on x86_64
> x86: BUG when ftrace patching recovery fails
> x86: Fix order of warning messages when ftrace modifies code
>
> arch/x86/kernel/ftrace.c | 67
> ++++++++++++++++++++++++------------------------
> 1 file changed, 34 insertions(+), 33 deletions(-)
>
That's quite a bit of changes. I did a quick debug analysis of it, and
found two bugs.
One, I shouldn't be using direct access to the ip with
probe_kernel_write(), I need to do the within() magic (also have a
ftrace_write() wrapper now).
The second bug is that I need to do another run_sync() after the fix
up. Can you see if this patch fixes the issue?
-- Steve
ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index e625319..6b566c8 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -455,7 +455,7 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
}
update:
- return probe_kernel_write((void *)ip, &nop[0], 1);
+ return ftrace_write(ip, nop, 1);
}
static int add_update_code(unsigned long ip, unsigned const char *new)
@@ -634,6 +634,7 @@ void ftrace_replace_code(int enable)
rec = ftrace_rec_iter_record(iter);
remove_breakpoint(rec);
}
+ run_sync();
}
static int
@@ -664,7 +665,7 @@ ftrace_modify_code(unsigned long ip, unsigned const
char *old_code, return ret;
fail_update:
- probe_kernel_write((void *)ip, &old_code[0], 1);
+ ftrace_write(ip, old_code, 1);
goto out;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
