Re: [RFC][PATCH 06/12] ftrace/x86: Add save_regs for i386 functioncalls

[Masami Hiramatsu]

(2012/06/06 23:37), Steven Rostedt wrote:
> On Tue, 2012-06-05 at 23:51 -0400, Steven Rostedt wrote:
> 
>> +ENTRY(ftrace_regs_caller)
>> +	pushf	/* push flags before compare */
>> +	cmpl $0, function_trace_stop
>> +	jne ftrace_exit
>> +
>> +
> 
> Masami,
> 
> Do we really need to push before the compare? As the compare flags are
> really meaningless with calling functions, and here we are only trying
> to hide what the cmpl did. If something else was tracing without regs,
> and we put a probe just after the nop, then it would include the cmpl
> changes. My version of the patch doesn't restore the flags, so two
> probes would have different values. But again, do we care? What would
> need to know the value of cmp flags when calling into a function when
> they are not going to be restored anyway.

Yes, it needs to be saved and restored too, for transparency.
If we don't guarantee it, users must check whether a probe
can do what they are doing, before they put the probe. And
if not, they must find another appropriate place. It's not
compatible with previous kprobes.

Actually, I think we can push flags after compare if we
add a note on kprobes document, so that user can expect
compare flags will be not correct if KPROBE_FLAG_FTRACE
is set. (Of course, it is better to provide an API
(ftrace_location is enough?) for giving him a hint how he
can get a correct flags with kprobes)

But if you'd like to introduce -mfentry, I hope ftrace to
restore flags, which will be useful for debugging/investigation
by tweaking flags while executing a function.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/