On 07/02/2019 14:51, Steven Rostedt wrote: > On Thu, 7 Feb 2019 10:33:50 +0000 > Julien Thierry <julien.thierry@xxxxxxx> wrote: > >> I don't see really much documentation on that function. As far as I can >> tell it is only called once for each site (and if it didn't, we'd always >> be placing the same instruction, but I agree it wouldn't be nice). It >> could depend on how far you can expand the notion of "adjusting" :) . >> >> Steven, do you have an opinion on whether it would be acceptable to >> modify function entry code in ftrace_call_adjust() ? > > Just to make sure I'm on the same page as you are. You want to modify > the function entry code at the time of the ftrace_call_adjust()? > Yes, that was the intention, the reason being that we want to have an instruction that is just patched once on each entry for initialization. > I would update the rec->ip to the offset you want at > ftrace_call_adjust() but not do any modifications. It really isn't safe > to do it there. But right after that is called, you will have the arch > specific call of ftrace_make_nop() called with MCOUNT_ADDR as the > second parameter to let you know that this is the first time the call > is made at this address. This is where you can do that initial > modifications. Yes, this is was the current version of this patch is doing, I was just wondering whether we could clean things up a little (i.e. have ftrace_make_nop() not generate a non-nop instruction :) ). But we're not going to hack through the API if this is definitely not what it's intended for. We'll keep it as is and just update the rec->ip with ftrace_call_adjust(). If we really want to clean things up, we could look into patching this at instruction at build time. But if it's not trivial I guess we can keep that for a later time. Thanks, -- Julien Thierry
