Re: Re: Re: [RFC PATCH -tip 1/9] ftrace: Add pt_regs acceptable trace callback

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2012/06/04 23:25), Steven Rostedt wrote:
> On Mon, 2012-06-04 at 22:58 +0900, Masami Hiramatsu wrote:
> 
>> Hmm, how about initializing in __init function ?
>> Or we can make func and regs_func in different members,
>> instead of using a union. (in that case, we can remove
>> FTRACE_OPS_FL_SAVE_REGS.)
>> I just consider passing uninitialized argument to user
>> function can cause unexpected behavior...
> 
> Easy solution:
> 
> As I want all functions to pass the ftrace_ops anyway, we can implement
> the ftrace_ops and the regs passing together. The arch will need to
> update both at the same time.

Hmm, is that ftrace_ops for recursion check? :)

> 
> But for archs that do not support ftrace_ops (and thus also not regs),
> we can do (and I will do this):
> 
> static inline void
>  __global_list_func(unsigned long ip, unsigned long parent_ip,
> 			struct ftrace_ops *ops, struct pt_regs *regs)
> {
> 	[do the loop, ignoring ops anyway, but passing in regs]
> }
> 
> #ifndef ARCH_SUPPORTS_FTRACE_OPS
> static void
> noregs_global_list_func(unsigned long ip, unsigned long parent_ip)
> {
> 	__global_list_func(ip, parent_ip, NULL, NULL);
> }
> #define global_list_func (ftrace_func_t)noregs_global_list_func
> #else
> static void global_list_func(unsigned long ip, unsigned long parent_ip,
> 			struct ftrace_ops *ops, struct pt_regs *regs)
> {
> 	__global_list_func(ip, parent_ip, ops, regs);
> }
> #endif
> 
> 
> 
> Nothing will be passed uninitialized. If an arch does not support
> passing ftrace ops and regs, then it will be calling the
> noregs_global_list_func() (or whatever I name it), which only expects
> the ip and parent_ip as parameters. Then that will be calling the actual
> loop function with NULLs in the parameters.

Yeah, that's safe, but I think dyn_ftrace can't call handler
directly from ftrace_call on such archs, can it?
# It depends on performance degradation.

> When an arch supports passing of ftrace_ops, then it should also support
> passing in the regs (as that should be the trivial part).

Preparing pt_regs by software is hard on some archs, e.g. IA64.
But yeah, that's an obsolete arch. We'd better focus on x86 and
ARM variants.

> Note, all funcs will get regs, but it may not get the full regs. That
> requires the ftrace_ops setting the special flag. The regs are saved for
> the mcount already. But only a partial set. Those will be sent to all
> function callbacks. If the function call back requires a full set (like
> kprobes does), then it must set the flag before registering.

Just out of curiously, would you mean that you will allocate full
pt_regs frame on stack always?

> 
> Hows this sound?

Sounds better to me, at far as there are non-initialized parameters
passed to user handler. :)

BTW, would you like to update ftrace part? I'd like to fix to remove
notrace from my previous patchset and resend tomorrow.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com