Messages in this thread | | | Date | Thu, 24 Feb 2022 10:03:13 +0800 | Subject | Re: [External] Re: [PATCH] arm64/ftrace: Make function graph use ftrace directly | From | Chengming Zhou <> |
| |
On 2022/2/24 9:30 上午, Steven Rostedt wrote: > On Wed, 23 Feb 2022 16:00:27 +0800 > Chengming Zhou <zhouchengming@bytedance.com> wrote: > >> Yes, it would be better to implement DYNAMIC_FTRACE_WITH_ARGS on arm64 too, >> and this patch just use DYNAMIC_FTRACE_WITH_REGS to install return_hooker >> for graph tracer, so it's a code cleanup, no performance optimization. > > I'm worried that a clean up is either breaking the design or hurting > performance. > > You have: > >> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, >> + struct ftrace_ops *op, struct ftrace_regs *fregs) >> +{ >> + struct pt_regs *regs = arch_ftrace_get_regs(fregs); > > Now, technically, arch_ftrace_get_regs() is to return NULL if the > ftrace_ops was not registered with ops->flags |= FTRACE_OPS_FL_SAVE_REGS. > > Which function graph does not do. > > But this is in arch specific code so you have more control of this > "undefined behavior". But you really should have a comment saying that > this needs to be fixed when DYNAMIC_FTRACE_WITH_ARGS is implemented. >
Ok, I will add a comment noting that.
Thanks.
> -- Steve > > >> + unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs); >> + >> + prepare_ftrace_return(ip, parent, frame_pointer(regs)); >> +} > > -- Steve
| |