[Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Jan Kiszka]

Hi Philippe,

I saw this for 2.6.30, but it obviously also made it into 2.6.29:

http://git.denx.de/?p=ipipe-2.6.git;a=commitdiff;h=d302f7638b9b52ea90e3c042edd940449cedb7a1

What precise problem does it try to solve? Is it a 2.6.30-only problem?
The point is that, last time I checked, the dynamic ftrace approach was
by far not I-pipe compatible. So I suspect we rather need a fix for the
core issue, but keep DYNAMIC_FTRACE off.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Philippe Gerum]

On Mon, 2009-06-29 at 17:11 +0200, Jan Kiszka wrote:
> Hi Philippe,
> 
> I saw this for 2.6.30, but it obviously also made it into 2.6.29:
> 
> http://git.denx.de/?p=ipipe-2.6.git;a=commitdiff;h=d302f7638b9b52ea90e3c042edd940449cedb7a1
> 
> What precise problem does it try to solve? Is it a 2.6.30-only problem?
> The point is that, last time I checked, the dynamic ftrace approach was
> by far not I-pipe compatible. So I suspect we rather need a fix for the
> core issue, but keep DYNAMIC_FTRACE off.

There are compatible for basic usage at least, but the way the tracer
forced activation of the ftrace layer in 2.6.29+ was wrong in the first
place, and this basically wrecked the ppc64 boot sequence. But now that
it is right (at least non-x86s are happy with this now), we need
DYNAMIC_FTRACE to enable the tracepoints; otherwise we would only get a
few xenomai symbols in the trace log (seen on x86*/ppc*). As it is, we
do get meaningful traces via /proc/ipipe/trace, but I would not assume
that we could not break the kernel when fiddling with ftrace's debugfs
interface.

Quite frankly, moving the tracer over ftrace brought quite a lot of
issues to the non-x86 ports unfortunately (including significant
overhead issues on ppc64); the fact that ftrace is a moving target did
not help either. This integration probably requires more work.

> 
> Jan
> 
-- 
Philippe.

Re: [Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Jan Kiszka]

Philippe Gerum wrote:
> On Mon, 2009-06-29 at 17:11 +0200, Jan Kiszka wrote:
>> Hi Philippe,
>>
>> I saw this for 2.6.30, but it obviously also made it into 2.6.29:
>>
>> http://git.denx.de/?p=ipipe-2.6.git;a=commitdiff;h=d302f7638b9b52ea90e3c042edd940449cedb7a1
>>
>> What precise problem does it try to solve? Is it a 2.6.30-only problem?
>> The point is that, last time I checked, the dynamic ftrace approach was
>> by far not I-pipe compatible. So I suspect we rather need a fix for the
>> core issue, but keep DYNAMIC_FTRACE off.
> 
> There are compatible for basic usage at least, but the way the tracer
> forced activation of the ftrace layer in 2.6.29+ was wrong in the first
> place, and this basically wrecked the ppc64 boot sequence.

That was a different issue, unrelated to this one.

> But now that
> it is right (at least non-x86s are happy with this now), we need
> DYNAMIC_FTRACE to enable the tracepoints; otherwise we would only get a
> few xenomai symbols in the trace log (seen on x86*/ppc*). As it is, we
> do get meaningful traces via /proc/ipipe/trace, but I would not assume
> that we could not break the kernel when fiddling with ftrace's debugfs
> interface.

Look at x86's entry_64.S: Enabling DYNAMIC_FTRACE cannot be correct as
it disables direct invocation of a callback handler in favor of runtime
patching (around ftrace_stub). And runtime patching goes through int3
IIRC. That will break sooner or later.

ppc64 looks similar on first sight.

> 
> Quite frankly, moving the tracer over ftrace brought quite a lot of
> issues to the non-x86 ports unfortunately (including significant
> overhead issues on ppc64); the fact that ftrace is a moving target did
> not help either. This integration probably requires more work.

Yes, that's unfortunate. It was actually aiming at reducing the
maintenance, reducing it to the no-arch part. I'll try to find some time
to look into the details again. Maybe we need to rethink the approach,
maybe patch ftrace instead (hopefully in the generic part).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Philippe Gerum]

On Mon, 2009-06-29 at 17:53 +0200, Jan Kiszka wrote: 
> Philippe Gerum wrote:
> > On Mon, 2009-06-29 at 17:11 +0200, Jan Kiszka wrote:
> >> Hi Philippe,
> >>
> >> I saw this for 2.6.30, but it obviously also made it into 2.6.29:
> >>
> >> http://git.denx.de/?p=ipipe-2.6.git;a=commitdiff;h=d302f7638b9b52ea90e3c042edd940449cedb7a1
> >>
> >> What precise problem does it try to solve? Is it a 2.6.30-only problem?
> >> The point is that, last time I checked, the dynamic ftrace approach was
> >> by far not I-pipe compatible. So I suspect we rather need a fix for the
> >> core issue, but keep DYNAMIC_FTRACE off.
> > 
> > There are compatible for basic usage at least, but the way the tracer
> > forced activation of the ftrace layer in 2.6.29+ was wrong in the first
> > place, and this basically wrecked the ppc64 boot sequence.
> 
> That was a different issue, unrelated to this one.

What I meant is that 2.6.29 had an init problem wrt activating ftrace
too early, somehow hiding the next issue regarding the actual lack of
tracepoints. Then first attempts on 2.6.30 booted properly, but had no
tracepoints. The two issues got in the way at the same time, causing
only partial fixes to be applied.

> 
> > But now that
> > it is right (at least non-x86s are happy with this now), we need
> > DYNAMIC_FTRACE to enable the tracepoints; otherwise we would only get a
> > few xenomai symbols in the trace log (seen on x86*/ppc*). As it is, we
> > do get meaningful traces via /proc/ipipe/trace, but I would not assume
> > that we could not break the kernel when fiddling with ftrace's debugfs
> > interface.
> 
> Look at x86's entry_64.S: Enabling DYNAMIC_FTRACE cannot be correct as
> it disables direct invocation of a callback handler in favor of runtime
> patching (around ftrace_stub). And runtime patching goes through int3
> IIRC. That will break sooner or later.

>From what I see in this code, dyn traces are injected on the fly under
stop_machine() callback protection, by emitting a jmp (e8) to the proper
target (either to skip MCOUNT_SIZE insns, or to branch to ftrace_caller,
depending on the disabled/enabled status).

The main issue I can see at first glance concerns how "atomic"
stop_machine() can be without any I-pipe awareness. Maybe we could reuse
the NMI protection hack available.

Admittedly, enabling DYNAMIC_FTRACE was a cheap way to get the
tracepoints back; however, I would rather try to iron the kstop path in
order to enable dynamic patching (and maybe fix a few other issues),
rather than dismissing dyn ftrace totally, which would be really helpful
in pre-production setups.

> ppc64 looks similar on first sight.
> 

Same remarks than above, we also have a branch to target pattern, no
exception path; with the added issue of dealing with ppc64 TOC, but that
is not involved in our problem anyway.

> > 
> > Quite frankly, moving the tracer over ftrace brought quite a lot of
> > issues to the non-x86 ports unfortunately (including significant
> > overhead issues on ppc64); the fact that ftrace is a moving target did
> > not help either. This integration probably requires more work.
> 
> Yes, that's unfortunate. It was actually aiming at reducing the
> maintenance, reducing it to the no-arch part. I'll try to find some time
> to look into the details again. Maybe we need to rethink the approach,
> maybe patch ftrace instead (hopefully in the generic part).
> 

Probably, yes. At the very least, from a design POV, the ipipe tracer
should appear as a regular ftrace tracer, not as some sideway.

> 
> Jan
> 
-- 
Philippe.

Re: [Adeos-main] DYNAMIC_FTRACE vs. I-pipe tracer

[Jan Kiszka]

Philippe Gerum wrote:
> On Mon, 2009-06-29 at 17:53 +0200, Jan Kiszka wrote: 
>> Philippe Gerum wrote:
>>> But now that
>>> it is right (at least non-x86s are happy with this now), we need
>>> DYNAMIC_FTRACE to enable the tracepoints; otherwise we would only get a
>>> few xenomai symbols in the trace log (seen on x86*/ppc*). As it is, we
>>> do get meaningful traces via /proc/ipipe/trace, but I would not assume
>>> that we could not break the kernel when fiddling with ftrace's debugfs
>>> interface.
>> Look at x86's entry_64.S: Enabling DYNAMIC_FTRACE cannot be correct as
>> it disables direct invocation of a callback handler in favor of runtime
>> patching (around ftrace_stub). And runtime patching goes through int3
>> IIRC. That will break sooner or later.
> 
>>From what I see in this code, dyn traces are injected on the fly under
> stop_machine() callback protection, by emitting a jmp (e8) to the proper
> target (either to skip MCOUNT_SIZE insns, or to branch to ftrace_caller,
> depending on the disabled/enabled status).

Yes, my information was outdated, based on their first approach. They
now no longer patch on demand, but atomically on enable/disable. That
should be safe.

> 
> The main issue I can see at first glance concerns how "atomic"
> stop_machine() can be without any I-pipe awareness. Maybe we could reuse
> the NMI protection hack available.

I would rather go for replacing stop_machine with
ipipe_critical_enter/exit here. Clean cut, no side effects due to other
users of stop_machine (module loading e.g.). NMIs is another issue, but
ftrace's protection mechanism looks OK for us.

> 
> Admittedly, enabling DYNAMIC_FTRACE was a cheap way to get the
> tracepoints back; however, I would rather try to iron the kstop path in
> order to enable dynamic patching (and maybe fix a few other issues),
> rather than dismissing dyn ftrace totally, which would be really helpful
> in pre-production setups.

If we get it ironed so that you can switch on the tracer without risking
to blow up your box, I'm all for it (and would actually start to ship
our kernels with CONFIG_IPIPE_TRACE_MCOUNT).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux