Messages in this thread | | | Date | Tue, 12 Nov 2019 17:48:16 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH -v5 05/17] x86/ftrace: Use text_poke() |
| |
On Tue, 12 Nov 2019 23:24:13 +0100 Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Nov 12, 2019 at 01:25:36PM -0500, Steven Rostedt wrote: > > On Mon, 11 Nov 2019 14:12:57 +0100 > > Peter Zijlstra <peterz@infradead.org> wrote: > > > > int ftrace_arch_code_modify_post_process(void) > > > __releases(&text_mutex) > > > { > > > - set_all_modules_text_ro(); > > > - set_kernel_text_ro(); > > > + text_poke_finish(); > > > > Why is the text_poke_finish() needed here? Can we add a comment about > > why? > > I think this is because of the text_poke_queue() in > ftrace_modify_code_direct(). I seem to have forgotten the code-flow > between the core and arch parts of ftrace again.
Hmm, I don't think there's a case where ftrace_make_nop() or ftrace_make_call() ever use the queued function. I added this:
static int 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; /* replace the text with the new text */ - if (ftrace_poke_late) + if (ftrace_poke_late) { + printk("POKE LATE!\n"); text_poke_queue((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL); - else + } else text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE); return 0; }
And that printk() never printed, even after running the ftracetests.
> > But sure, I can try and dig that out again and write a comment. > > > > > + text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL); // BATCH > > > > What do you mean by "BATCH" ? > > Ah, that was a question, can/should we use text_poke_queue() there?
Probably not, as that's just a single location or two. It modifies the static trampolines if only a single user is registered to ftrace.
-- Steve
| |