Re: Porting DTrace to ARM

[Ryota Ozaki <ozaki-r%iij.ad.jp@localhost>]

Hi Matt,

Thank you very much for your kind reviewing.

(2014/01/24 23:03), Matt Thomas wrote:
> On Jan 24, 2014, at 2:34 AM, Ryota Ozaki <ozaki-r%iij.ad.jp@localhost> wrote:
>
> > Updated as I planned, there is no curious update though.
>
> Use of curcpu() or cpu_number() without having done kpreempt_disable() is 
> unsafe.

All the functions that I added and calls cpu_number() are
called from an interrupt handler, xc_unicast(XC_HIGHPRI, ...)
or dtrace_probe() that calls interrupt_disable() prior to
calling the functions.

In these cases, we don't need kpreempt_disable(), right?

> dtrace_asm.S doesn't "assym.h"

Do you mean dtrace_asm.S doesn't need "assym.h"?

> assembly routines should have a END(name) as well.
> Use of =foo is frowned upon.
>
> ENTRY(dtrace_invop_init)
>         #ldr    r0, dtrace_invop_start
>         ldr     r1, .Ldtrace_invop
>         ldr     r2, .Ldtrace_invop_jump_addr
>         str     r1, [r2]
>         RET
>         .align  0
> .Ldtrace_invop:
>         .word   dtrace_invop
> .Ldtrace_invop_jump_addr
>         .word   dtrace_invop_jump_addr
> END(dtrace_invop_init)
>
> ENTRY(dtrace_invop_uninit)
>         mov     r0, #0
>         ldr     r1, .Ldtrace_invop_jump_addr
>         str     r0, [r1]
>         RET
> END(dtrace_invop_uninit)

Thanks, it works. The code is what I wanted to
do initially.

> external/cddl/osnet/dev/fbt/fbt.c:
>
> Don't use u_int{8,16,32}_t

OK. Will fix.

> Makefiles:
>
> Use include <bsd.own.mk>, then use
>
> .if exists(${.CURDIR}/${MACHINE})
> ARCHDIR=${MACHINE}
> .elif exists(${.CURDIR}/${MACHINE_CPU})
> ARCHDIR=${MACHINE_CPU}
> .else
> .error ${MACHINE} or ${MACHINE_CPU} not supported.
> .endif

Followed you suggestion, I changed Makefile like this:

OSNETDIR=${NETBSDSRCDIR}/external/cddl/osnet

.if exists(${OSNETDIR}/dev/cyclic/${MACHINE:S/amd64/i386/})
ARCH=${MACHINE:S/amd64/i386/}
.elif exists(${OSNETDIR}/dev/cyclic/${MACHINE_CPU})
ARCH=${MACHINE_CPU}
.else
.error ${MACHINE} or ${MACHINE_CPU} not supported.
.endif

Two notes; .CURDIR is changed to the corresponding directory
and ARCHDIR is changed to ARCH because for arm ARCHDIR
is already defined in share/mk/bsd.kmodule.mk.

Thanks,
  ozaki-r