On Fri, Jun 26, 2020 at 01:29:31PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 25, 2020 at 03:40:42PM -0700, Sami Tolvanen wrote:
> > Anyway, since objtool is run before recordmcount, I just left this unchanged
> > for testing and ignored the recordmcount warnings about __mcount_loc already
> > existing. Something is a bit off still though, I see this at boot:
> >
> > ------------[ ftrace bug ]------------
> > ftrace failed to modify
> > [<ffffffff81000660>] __tracepoint_iter_initcall_level+0x0/0x40
> > actual: 0f:1f:44:00:00
> > Initializing ftrace call sites
> > ftrace record flags: 0
> > (0)
> > expected tramp: ffffffff81056500
> > ------------[ cut here ]------------
> >
> > Otherwise, this looks pretty good.
>
> Ha! it is trying to convert the "CALL __fentry__" into a NOP and not
> finding the CALL -- because objtool already made it a NOP...
>
> Weird, I thought recordmcount would also write NOPs, it certainly has
> code for that. I suppose we can use CC_USING_NOP_MCOUNT to avoid those,
> but I'd rather Steve explain this before I wreck things further.
Something like so would ignore whatever text is there and rewrite it
with ideal_nop.
---
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index c84d28e90a58..98a6a93d7615 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -109,9 +109,11 @@ static int __ref
ftrace_modify_code_direct(unsigned long ip, const char *old_code,
const char *new_code)
{
- int ret = ftrace_verify_code(ip, old_code);
- if (ret)
- return ret;
+ if (old_code) {
+ int ret = ftrace_verify_code(ip, old_code);
+ if (ret)
+ return ret;
+ }
/* replace the text with the new text */
if (ftrace_poke_late)
@@ -124,9 +126,8 @@ ftrace_modify_code_direct(unsigned long ip, const char *old_code,
int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr)
{
unsigned long ip = rec->ip;
- const char *new, *old;
+ const char *new;
- old = ftrace_call_replace(ip, addr);
new = ftrace_nop_replace();
/*
@@ -138,7 +139,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long ad
* just modify the code directly.
*/
if (addr == MCOUNT_ADDR)
- return ftrace_modify_code_direct(ip, old, new);
+ return ftrace_modify_code_direct(ip, NULL, new);
/*
* x86 overrides ftrace_replace_code -- this function will never be used
