Re: [PATCH] LTT for SH4

Hi Giuseppe,

* Giuseppe Cavallaro ([email protected]) wrote:
> Hi Mathieu,
> 
> On 03/07/07, Mathieu Desnoyers <[email protected]> wrote:
> >
> >
> >(I am not a sh expert, I refer to this document for the ABI:
> >http://lars.nocrew.org/computers/processors/SuperH/sh5_abi.pdf)
> 
> 
> You ought to look at the SH-4 ABI documentation.
> SH5 is a CPU Core 64 bit (sh64 within the kernel sources).
> I know it because, some years ago, I worked on STLinux for SH5.
> 

Ok, thanks for the info.

> Can you do like i386 and use int entryexit variable instead of traceid ?
> 
> 
> yes I can.
> 
> ...
> >
> >timestamping with TMU1 :
> >
> >(there seem to be leftover whitespaces in your code around your
> >indentation tabs)
> 
> 
> I'm sorry, I'll fix that!
> 
> Does this timer stop once it reaches the underflow ?
> 
> 
> Yes it does.

Hrm, here you seems to say that the timer stops, while right after you
say that it stops... so, what happens ? :)

> 
> Oh, I see.. you use the underflow count do detects underflows on your
> >counter.. but isn't it pointless to detect 32 bits underflows in the
> >input counter when you are keeping a 32 bits counter? All this underflow
> >detection could go away happily. However, to as MIPS does: require the
> >"Use Synthetic TSC" in ltt/Kconfig option to detect 32 bits counter
> >overflows to extend it to a 64 bits counter.
> 
> 
> Probably I'm a bit confused about this question.
> With my current implementation based on the interrupt, the get_timestamp32
> function returns the "absolute" time elapsed from the start counting.
> Indeed, loading the maximum value (0xffffffff) into the TCNT (TMU1 counter
> register), with an external frequency of 16 MHz, I get an underflow
> interrupt every ~ 4'.
> If we want to get an absolute timestamp value we need to treat the
> underflow.
> Welcome advice!

Yep, we need to deal with the underflow.

The idea of ltt/ltt-heartbeat.ko is that it spawns a kernel timer
handler, launched periodically by the kernel timer, to check wether or
not an overflow of the counter has occured. I also wrap the counter
reads in a function that also detects if an overflow occured since the
last heartbeat and deals with it.

So, it is all transparent, you really just have to feed it a
monotonically increasing 32 bits timestamp counter.

I would like not to depend on the interrupt generated by it to detect
the underflow/overflow because it will leave a race period where
interrupts with higher priority than the TMU1 interrupt might read a
time that goes way backward (count would have been reset, but the number
of overflows would not have been incremented yet).

> So I'll happy to review my code and send you the new patches.
> 
> Since the timer reloads the initial value after underflow and continues
> >to run, we only need to do timer reads and could even disable the
> >interrupt.
> 
> 
> A brief note about the SH4-TMU device:
> when the TCNT counter underflows while counting down, the default value (in
> the TCOR register) is set in that TCNT, which continues counting down from
> the set value.
> 
> Does it conflict with arch/sh/kernel/timers/timer-tmu.c ?
> 
> 
> Perhaps, you have, in your tree, some other kernel patches (i.e. Paul
> Mundt's patch "sh: clockevent/clocksource/hrtimers/nohz TMU support" ... ).
> Unfortunately, I cannot apply this patches in my development tree (2.6.17).
> 
> Could we use this infrastructure or do we have to disable theirs when
> >LTT is compiled in ?
> 
> 
> I do suggest not to use the infrastructure included in the timer-tmu.c file
> at this moment.
> If I have the new kernel tree, I'll use and test these functions within
> LTTng.
> Moreover, this approach, in my opinion, is clear and safe enough!
> 

Hrm, I guess we'll have to figure which kernel config option it clashes
with in order to make sure it is never selected by users.
Probably that CONFIG_SH_TMU is a good canditate.

Well, ideally, since we don't really need its interrupt, we could
probably share this register if they use it as a simple counter, just
like we should.


> I guess you'll also need to send the patch that puts the trace_mark
> >markers into the sh code at some point.
> 
> 
> The "lttng-instrumentation-sh.patch" file already puts the trace_mark(s)
> into the sh architecture.
> Maybe, we only need to adds trace_mark in the do_syscall_trace.
> 

Oh great, I did not remember that :)

> Thanks for your work :)
> >
> Mathieu
> 

Thanks for your help :)

Mathieu

> 
> You are welcome! I like LTTng ;-)))
> Giuseppe

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68