Messages in this thread | | | Date | Wed, 23 Feb 2022 20:30:01 -0500 | From | Steven Rostedt <> | Subject | Re: [External] Re: [PATCH] arm64/ftrace: Make function graph use ftrace directly |
| |
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.
-- Steve
> + unsigned long *parent = (unsigned long *)&procedure_link_pointer(regs); > + > + prepare_ftrace_return(ip, parent, frame_pointer(regs)); > +}
-- Steve
| |