Re: [PATCH v14 04/19] function_graph: Replace fgraph_ret_regs with ftrace_regs

Masami Hiramatsu
2024-10-02T14:31:13Z

On Tue, 1 Oct 2024 19:32:34 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 2 Oct 2024 08:10:37 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > >
> > > I may add some compiler hacks to enforce this. Something like:
> > >
> > > struct ftrace_regs {
> > > void *nothing_to_see_here;
> > > };
> >
> > Yeah, OK. But sizeof(fregs) may be changed. (Shouldn't we do too?)
>
> Honestly, I don't think anything should be doing a sizeof(struct ftrace_regs)
>
> Heck, perhaps we should make it totally zero!
>
> struct ftrace_regs {
> long nothing_here[];
> };
>
> If someone needs to allocate, then we could provide a:
>
> ftrace_regs_size()
>
> helper function.

Ah, Indeed.

>
> >
> > >
> > > And then change the arch code to be something like:
> > >
> > > // in arch/arm64/include/asm/ftrace.h:
> > >
> > > struct arch_ftrace_regs {
> > > /* x0 - x8 */
> > > unsigned long regs[9];
> > >
> > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > unsigned long direct_tramp;
> > > #else
> > > unsigned long __unused;
> > > #endif
> > >
> > > unsigned long fp;
> > > unsigned long lr;
> > >
> > > unsigned long sp;
> > > unsigned long pc;
> > > };
> >
> > And if it is pt_regs compatible,
> >
> > #define arch_ftrace_regs pt_regs
> >
> > ?
> >
>
> Only if it is fully pt_regs compatible.

Yeah, OK, this is good idea.

Thank you,

>
> -- Steve

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>