code review 72760043: runtime: make use of THREAD_SHARE userpsace mutex on freebsd

runtime: make use of THREAD_SHARE userspace mutex on freebsd

For now Note, futexsleep and futexwakeup are designed for threads,
not for processes. The explicit use of UMTX_OP_WAIT_UINT_PRIVATE and
UMTX_OP_WAKE_PRIVATE can avoid unnecessary traversals of VM objects,
to hit undiscovered bugs related to VM system on SMP/SMT/NUMA
environment.

Update issue 7496

[mikio, 2014-03-08 11:00:31]

Hello golang-codereviews@googlegroups.com (cc:
golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[gobot, 2014-03-11 23:38:20]

R=iant@golang.org (assigned by mikioh.mikioh@gmail.com)

[bradfitz, 2014-03-11 23:46:09]

Typos:

userpsace
explict
syetem

And I'd probably write "traversals" instead.



On Sat, Mar 8, 2014 at 3:00 AM, <mikioh.mikioh@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> runtime: make use of THREAD_SHARE userpsace mutex on freebsd
>
> Note, futexsleep and futexwakeup are designed for threads, not for
> processes. The explict use of UMTX_OP_WAIT_UINT_PRIVATE and
> UMTX_OP_WAKE_PRIVATE can avoid unnecessary traverses of VM objects,
> to hit undiscovered bugs related to VM syetem on SMP/SMT/NUMA
> environment.
>
> Update issue 7496
>
> Please review this at https://codereview.appspot.com/72760043/
>
> Affected files (+19, -11 lines):
>   M src/pkg/runtime/defs_freebsd.go
>   M src/pkg/runtime/defs_freebsd_386.h
>   M src/pkg/runtime/defs_freebsd_amd64.h
>   M src/pkg/runtime/defs_freebsd_arm.h
>   M src/pkg/runtime/os_freebsd.c
>
>
> Index: src/pkg/runtime/defs_freebsd.go
> ===================================================================
> --- a/src/pkg/runtime/defs_freebsd.go
> +++ b/src/pkg/runtime/defs_freebsd.go
> @@ -49,8 +49,10 @@
>         SA_RESTART = C.SA_RESTART
>         SA_ONSTACK = C.SA_ONSTACK
>
> -       UMTX_OP_WAIT_UINT = C.UMTX_OP_WAIT_UINT
> -       UMTX_OP_WAKE      = C.UMTX_OP_WAKE
> +       UMTX_OP_WAIT_UINT         = C.UMTX_OP_WAIT_UINT
> +       UMTX_OP_WAIT_UINT_PRIVATE = C.UMTX_OP_WAIT_UINT_PRIVATE
> +       UMTX_OP_WAKE              = C.UMTX_OP_WAKE
> +       UMTX_OP_WAKE_PRIVATE      = C.UMTX_OP_WAKE_PRIVATE
>
>         SIGHUP    = C.SIGHUP
>         SIGINT    = C.SIGINT
> Index: src/pkg/runtime/defs_freebsd_386.h
> ===================================================================
> --- a/src/pkg/runtime/defs_freebsd_386.h
> +++ b/src/pkg/runtime/defs_freebsd_386.h
> @@ -21,8 +21,10 @@
>         SA_RESTART      = 0x2,
>         SA_ONSTACK      = 0x1,
>
> -       UMTX_OP_WAIT_UINT       = 0xb,
> -       UMTX_OP_WAKE            = 0x3,
> +       UMTX_OP_WAIT_UINT               = 0xb,
> +       UMTX_OP_WAIT_UINT_PRIVATE       = 0xf,
> +       UMTX_OP_WAKE                    = 0x3,
> +       UMTX_OP_WAKE_PRIVATE            = 0x10,
>
>         SIGHUP          = 0x1,
>         SIGINT          = 0x2,
> Index: src/pkg/runtime/defs_freebsd_amd64.h
> ===================================================================
> --- a/src/pkg/runtime/defs_freebsd_amd64.h
> +++ b/src/pkg/runtime/defs_freebsd_amd64.h
> @@ -21,8 +21,10 @@
>         SA_RESTART      = 0x2,
>         SA_ONSTACK      = 0x1,
>
> -       UMTX_OP_WAIT_UINT       = 0xb,
> -       UMTX_OP_WAKE            = 0x3,
> +       UMTX_OP_WAIT_UINT               = 0xb,
> +       UMTX_OP_WAIT_UINT_PRIVATE       = 0xf,
> +       UMTX_OP_WAKE                    = 0x3,
> +       UMTX_OP_WAKE_PRIVATE            = 0x10,
>
>         SIGHUP          = 0x1,
>         SIGINT          = 0x2,
> Index: src/pkg/runtime/defs_freebsd_arm.h
> ===================================================================
> --- a/src/pkg/runtime/defs_freebsd_arm.h
> +++ b/src/pkg/runtime/defs_freebsd_arm.h
> @@ -21,8 +21,10 @@
>         SA_RESTART      = 0x2,
>         SA_ONSTACK      = 0x1,
>
> -       UMTX_OP_WAIT_UINT       = 0xb,
> -       UMTX_OP_WAKE            = 0x3,
> +       UMTX_OP_WAIT_UINT               = 0xb,
> +       UMTX_OP_WAIT_UINT_PRIVATE       = 0xf,
> +       UMTX_OP_WAKE                    = 0x3,
> +       UMTX_OP_WAKE_PRIVATE            = 0x10,
>
>         SIGHUP          = 0x1,
>         SIGINT          = 0x2,
> Index: src/pkg/runtime/os_freebsd.c
> ===================================================================
> --- a/src/pkg/runtime/os_freebsd.c
> +++ b/src/pkg/runtime/os_freebsd.c
> @@ -50,7 +50,7 @@
>         Timespec ts;
>
>         if(ns < 0) {
> -               ret = runtime·sys_umtx_op(addr, UMTX_OP_WAIT_UINT, val,
> nil, nil);
> +               ret = runtime·sys_umtx_op(addr, UMTX_OP_WAIT_UINT_PRIVATE,
> val, nil, nil);
>                 if(ret >= 0 || ret == -EINTR)
>                         return;
>                 goto fail;
> @@ -58,7 +58,7 @@
>         // NOTE: tv_nsec is int64 on amd64, so this assumes a
> little-endian system.
>         ts.tv_nsec = 0;
>         ts.tv_sec = runtime·timediv(ns, 1000000000, (int32*)&ts.tv_nsec);
> -       ret = runtime·sys_umtx_op(addr, UMTX_OP_WAIT_UINT, val, nil, &ts);
> +       ret = runtime·sys_umtx_op(addr, UMTX_OP_WAIT_UINT_PRIVATE, val,
> nil, &ts);
>         if(ret >= 0 || ret == -EINTR)
>                 return;
>
> @@ -78,7 +78,7 @@
>  {
>         int32 ret;
>
> -       ret = runtime·sys_umtx_op(addr, UMTX_OP_WAKE, cnt, nil, nil);
> +       ret = runtime·sys_umtx_op(addr, UMTX_OP_WAKE_PRIVATE, cnt, nil,
> nil);
>         if(ret >= 0)
>                 return;
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[mikio, 2014-03-12 00:00:21]

On Wed, Mar 12, 2014 at 8:46 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> Typos:
>
> userpsace
> explict
> syetem
>
> And I'd probably write "traversals" instead.

Fixed, thanks.

[iant, 2014-03-12 01:17:00]

LGTM

[mikio, 2014-03-12 01:33:15]

*** Submitted as https://code.google.com/p/go/source/detail?r=19fe312278ac ***

runtime: make use of THREAD_SHARE userspace mutex on freebsd

For now Note, futexsleep and futexwakeup are designed for threads,
not for processes. The explicit use of UMTX_OP_WAIT_UINT_PRIVATE and
UMTX_OP_WAKE_PRIVATE can avoid unnecessary traversals of VM objects,
to hit undiscovered bugs related to VM system on SMP/SMT/NUMA
environment.

Update issue 7496

LGTM=iant
R=golang-codereviews, gobot, iant, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/72760043

[gobot, 2014-03-12 02:37:13]

This CL appears to have broken the linux-amd64-race builder.