Re: [PATCH] LTT for SH4

Hi Giuseppe,

This is great :-)

A few comments:

(I am not a sh expert, I refer to this document for the ABI:
http://lars.nocrew.org/computers/processors/SuperH/sh5_abi.pdf)

* Giuseppe Cavallaro ([email protected]) wrote:
> Hi All,
> I'm working on LTT for SH4 architecture (*).
> In attachment you will find:
> - lttng-sh_timestamp.patch
>        It reviews the include/asm-sh/ltt.h file (adding timestamp mechanism
> via TMU).
> - lttng-0.9.5-sh_ptrace.patch:
>        It marks the do_syscall_trace function within ptrace.c file.
> - lttng-0.9.5-probe-kernel_arch_sh.patch
> 
> I cannot actually test these patches on the latest kernels.
> In any case, this code, added in the 2.6.17 tree, seems to start working.
> 
> Hoping that could be useful.
> Cheers,
>   Giuseppe
> 
> (*) using a kernel 2.6.17 plus LTTng-0.9.5 patches.

If you can send your patches inline with your email, it will make the
review easier.


> This marks the do_syscall_trace function within the ptrace.c file
> 
> --- linux-2.6.21/arch/sh/kernel/ptrace.c.orig   2007-07-03 15:49:05.000000000
> ++0200
> +++ linux-2.6.21/arch/sh/kernel/ptrace.c        2007-07-03 15:50:14.000000000
> ++0200
> @@ -264,10 +264,23 @@ long arch_ptrace(struct task_struct *chi
>         return ret;
>  }
> 
> +#ifdef CONFIG_LTT
> +asmlinkage void do_syscall_trace(struct pt_regs *regs, int traceid)
> +#else

Can you do like i386 and use int entryexit variable instead of traceid ?

...

timestamping with TMU1 :

(there seem to be leftover whitespaces in your code around your
indentation tabs)

Do you really have to use the timer interrupt at all ? Does this timer
stop once it reaches the underflow ?

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.

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.

Does it conflict with arch/sh/kernel/timers/timer-tmu.c ?

See:
static cycle_t tmu_timer_read(void)
{
        return ~ctrl_inl(TMU1_TCNT);
}

and

static void tmu1_clk_init(struct clk *clk)
{
        u8 divisor = TMU_TCR_INIT & 0x7;
        ctrl_outw(divisor, TMU1_TCR);
        clk->rate = clk->parent->rate / (4 << (divisor << 1));
}


static void tmu1_clk_recalc(struct clk *clk)
{
        u8 divisor = ctrl_inw(TMU1_TCR) & 0x7;
        clk->rate = clk->parent->rate / (4 << (divisor << 1));
}

Could we use this infrastructure or do we have to disable theirs when
LTT is compiled in ?

I guess you'll also need to send the patch that puts the trace_mark
markers into the sh code at some point.

Thanks for your work :)

Mathieu

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