code review 6842081: runtime: implement SysUnused for netbsd

runtime: implement SysUnused for netbsd

[jsing, 2012-11-24 16:58:22]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2012-11-24 17:27:38]

LGTM
 On Nov 24, 2012 8:58 AM, <jsing@google.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> runtime: implement SysUnused for netbsd
>
> Please review this at
http://codereview.appspot.com/**6842081/<http://codereview.appspot.com/6842081/>
>
> Affected files:
>   M src/pkg/runtime/defs_netbsd.go
>   M src/pkg/runtime/defs_netbsd_**386.h
>   M src/pkg/runtime/defs_netbsd_**amd64.h
>   M src/pkg/runtime/mem_netbsd.c
>   M src/pkg/runtime/sys_netbsd_**386.s
>   M src/pkg/runtime/sys_netbsd_**amd64.s
>
>
> Index: src/pkg/runtime/defs_netbsd.go
> ==============================**==============================**=======
> --- a/src/pkg/runtime/defs_netbsd.**go
> +++ b/src/pkg/runtime/defs_netbsd.**go
> @@ -37,6 +37,9 @@
>         MAP_PRIVATE = C.MAP_PRIVATE
>         MAP_FIXED   = C.MAP_FIXED
>
> +       MADV_DONTNEED = C.MADV_DONTNEED
> +       MADV_FREE     = C.MADV_FREE
> +
>         SA_SIGINFO = C.SA_SIGINFO
>         SA_RESTART = C.SA_RESTART
>         SA_ONSTACK = C.SA_ONSTACK
> Index: src/pkg/runtime/defs_netbsd_**386.h
> ==============================**==============================**=======
> --- a/src/pkg/runtime/defs_netbsd_**386.h
> +++ b/src/pkg/runtime/defs_netbsd_**386.h
> @@ -12,6 +12,9 @@
>         MAP_PRIVATE     = 0x2,
>         MAP_FIXED       = 0x10,
>
> +       MADV_DONTNEED   = 0x4,
> +       MADV_FREE       = 0x6,
> +
>         SA_SIGINFO      = 0x40,
>         SA_RESTART      = 0x2,
>         SA_ONSTACK      = 0x1,
> Index: src/pkg/runtime/defs_netbsd_**amd64.h
> ==============================**==============================**=======
> --- a/src/pkg/runtime/defs_netbsd_**amd64.h
> +++ b/src/pkg/runtime/defs_netbsd_**amd64.h
> @@ -12,6 +12,9 @@
>         MAP_PRIVATE     = 0x2,
>         MAP_FIXED       = 0x10,
>
> +       MADV_DONTNEED   = 0x4,
> +       MADV_FREE       = 0x6,
> +
>         SA_SIGINFO      = 0x40,
>         SA_RESTART      = 0x2,
>         SA_ONSTACK      = 0x1,
> Index: src/pkg/runtime/mem_netbsd.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/mem_netbsd.c
> +++ b/src/pkg/runtime/mem_netbsd.c
> @@ -28,9 +28,7 @@
>  void
>  runtime·SysUnused(void *v, uintptr n)
>  {
> -       USED(v);
> -       USED(n);
> -       // TODO(rsc): call madvise MADV_DONTNEED
> +       runtime·madvise(v, n, MADV_FREE);
>  }
>
>  void
> Index: src/pkg/runtime/sys_netbsd_**386.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_netbsd_**386.s
> +++ b/src/pkg/runtime/sys_netbsd_**386.s
> @@ -83,6 +83,13 @@
>         MOVL    $0xf1, 0xf1             // crash
>         RET
>
> +TEXT runtime·madvise(SB),7,$-4
> +       MOVL    $75, AX                 // sys_madvise
> +       INT     $0x80
> +       JAE     2(PC)
> +       MOVL    $0xf1, 0xf1             // crash
> +       RET
> +
>  TEXT runtime·setitimer(SB),7,$-4
>         MOVL    $425, AX                // sys_setitimer
>         INT     $0x80
> @@ -180,7 +187,7 @@
>         CMPL    BX, $0
>         JNE     5(PC)
>         MOVL    signo+0(FP), BX
> -       MOVL    BX, 0(SP)
> +       MOVL    BX, 0(SP)
>         CALL    runtime·badsignal(SB)
>         RET
>
> Index: src/pkg/runtime/sys_netbsd_**amd64.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/sys_netbsd_**amd64.s
> +++ b/src/pkg/runtime/sys_netbsd_**amd64.s
> @@ -10,7 +10,6 @@
>
>  // int32 lwp_create(void *context, uintptr flags, void *lwpid)
>  TEXT runtime·lwp_create(SB),7,$0
> -
>         MOVQ    context+0(FP), DI
>         MOVQ    flags+8(FP), SI
>         MOVQ    lwpid+16(FP), DX
> @@ -248,6 +247,18 @@
>         MOVL    $0xf1, 0xf1             // crash
>         RET
>
> +
> +TEXT runtime·madvise(SB),7,$0
> +       MOVQ    addr+0(FP), DI          // arg 1 - addr
> +       MOVQ    len+8(FP), SI           // arg 2 - len
> +       MOVQ    behav+16(FP), DX        // arg 3 - behav
> +       MOVQ    $75, AX                 // sys_madvise
> +       SYSCALL
> +       CMPQ    AX, $0xfffffffffffff001
> +       JLS     2(PC)
> +       MOVL    $0xf1, 0xf1             // crash
> +       RET
> +
>  TEXT runtime·sigaltstack(SB),7,$-8
>         MOVQ    new+8(SP), DI           // arg 1 - nss
>         MOVQ    old+16(SP), SI          // arg 2 - oss
>
>
>

[minux1, 2012-11-24 17:59:12]

https://codereview.appspot.com/6842081/diff/7001/src/pkg/runtime/sys_netbsd_a...
File src/pkg/runtime/sys_netbsd_amd64.s (right):

https://codereview.appspot.com/6842081/diff/7001/src/pkg/runtime/sys_netbsd_a...
src/pkg/runtime/sys_netbsd_amd64.s:257: CMPQ	AX, $0xfffffffffffff001
this is wrong.
you should use JCC 2(PC)

for one thing, madvise returns an int, not long.

[jsing, 2012-11-25 11:55:52]

On 2012/11/24 17:59:12, minux wrote:
>
https://codereview.appspot.com/6842081/diff/7001/src/pkg/runtime/sys_netbsd_a...
> File src/pkg/runtime/sys_netbsd_amd64.s (right):
> 
>
https://codereview.appspot.com/6842081/diff/7001/src/pkg/runtime/sys_netbsd_a...
> src/pkg/runtime/sys_netbsd_amd64.s:257: CMPQ	AX, $0xfffffffffffff001
> this is wrong.
> you should use JCC 2(PC)
> 
> for one thing, madvise returns an int, not long.

Indeed. Fixed.

PTAL.

[dave_cheney.net, 2012-11-25 12:01:40]

Thanks for fixing this, the asm for madvise should closely follow munmap.

[minux1, 2012-11-25 12:22:40]

LGTM.

[jsing, 2012-11-26 11:34:08]

*** Submitted as http://code.google.com/p/go/source/detail?r=d202aa686554 ***

runtime: implement SysUnused for netbsd

R=golang-dev, bradfitz, minux.ma, dave
CC=golang-dev
http://codereview.appspot.com/6842081