Messages in this thread | | | Subject | Re: [RFC][PATCH 03/13 v2] ftrace: Return pt_regs to function trace callback | From | Steven Rostedt <> | Date | Thu, 14 Jun 2012 23:17:35 -0400 |
| |
On Fri, 2012-06-15 at 12:02 +0900, Masami Hiramatsu wrote: > (2012/06/13 7:43), Steven Rostedt wrote: > > From: Steven Rostedt <srostedt@redhat.com> > > > > Return as the 4th paramater to the function tracer callback the pt_regs. > > > > Currently x86_64 just passes NULL as the regs arguement. Later patches > > that implement regs passing will require having the ftrace_ops set the > > SAVE_REGS flag, which will tell the arch to take the time to pass a > > full set of pt_regs to the ftrace_ops callback function. If the arch > > does not support it then it should pass NULL. > > Hmm, I think the x86-64 part of this patch would be better to be separated > with x86-64 part of [5/13], so that I can easily review the change... > Another reason is that this patch doesn't define ARCH_SUPPORTS_FTRACE_SAVE_REGS > on x86_64 too...
I agree. This was more left over from the 'PARTIAL_REGS' saving, which I scrapped. I'll just fold the x86 bits of this patch into patch 5 as you suggested, and then just keep this patch as the 'added API' change.
> > > > A ftrace_ops call back can either check if the macro ARCH_SUPPORTS_FTRACE_SAVE_REGS > > is defined, or it can check if regs is NULL. As it will be NULL if > > it is not supported by the arch even if the SAVE_REGS flag is set. > > > > If an arch can pass full regs, then it should define: > > ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1 > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > [...] > > diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S > > index 2b4f94c..83d8ae0 100644 > > --- a/arch/x86/kernel/entry_64.S > > +++ b/arch/x86/kernel/entry_64.S > > @@ -80,7 +80,11 @@ ENTRY(ftrace_caller) > > MCOUNT_SAVE_FRAME > > > > leaq function_trace_op, %rdx > > - movq 0x38(%rsp), %rdi > > + > > + /* regs go into 4th parameter (but make it NULL) */ > > + movq $0, %rcx > > There is no ARCH_SUPPORTS_FTRACE_SAVE_REGS, so I think you > don't need to clear rcx at least this time.
Agreed, I'll fold this in then.
> > Another generic ftrace part of this patch is good to me :) > > Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thanks for the review, but I'll modify 3 and 5 and hopefully, you can review it again. I'll do this tomorrow as it's bed time for me now ;-)
-- Steve
| |