Re: Anyone working on ftrace function graph support on ARM?

Uwe Kleine-König
2009-03-25T12:10:41Z

Hi Russell,

> > > Hmm, and it looks like the ftrace code is rather crap:
> > >
> > > ENTRY(mcount)
> > > stmdb sp!, {r0-r3, lr}
> > > ldr r0, =ftrace_trace_function
> > > ldr r2, [r0]
> > > adr r0, ftrace_stub
> > > cmp r0, r2
> > > bne trace
> > > ldr lr, [fp, #-4] @ restore lr
> > > ldmia sp!, {r0-r3, pc}
> > >
> > > trace:
> > > ldr r1, [fp, #-4] @ lr of instrumented routine
> > > mov r0, lr
> > > sub r0, r0, #MCOUNT_INSN_SIZE
> > > mov lr, pc
> > > mov pc, r2
> > > XXX calling a C function results in r0-r3,ip,lr being clobbered XXX
> > >
> > > mov lr, r1 @ restore lr
> > > XXX not necessarily, r1 might be some other random value
> > >
> > > ldmia sp!, {r0-r3, pc}
> > >
> > > In fact, to me the above code looks totally crap, because it's checking
> > > whether the caller is 'ftrace_stub'. It can never be 'ftrace_stub'
> > > because that is an assembly function:
> > >
> > > .globl ftrace_stub
> > > ftrace_stub:
> > > mov pc, lr
> > >
> > > and therefore gcc has no hand in adding a mcount call to it.
> > Hhhm. Isn't the equivalent C-Code ~ as follows:
> >
> > if (ftrace_trace_function != ftrace_stub)
> > trace(some, args);
> > return;
> > ? ftrace_trace_function is initialised to ftrace_stub at compile time
> > and is changed when a tracerfunction is registered.
>
> Correct. But my point is that there's no way for ftrace_stub to ever call
> mcount. So the check there is pointless.
Which check? ftrace_trace_function isn't the caller of mcount but a
function pointer defined in kernel/trace/ftrace.c. And if this pointer
changes, the if-condition becomes true.

> Ok - it would be nice if there was a comment to explain that.
Some comments would be nice, that's right.

> Is someone going to fix the existing ftrace before trying to build stuff
> on top of it?
Mmh, I think you misunderstood the code, so I don't see something to fix
here (at least for non-dynamic ftrace). If the next few mails show,
that it's me who didn't understand the code, I will fix the problems, of
course.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |