code review 10848043: runtime: disable preemption

runtime: disable preemption

There are various problems, and both Dmitriy and I
will be away for the next week. Make the runtime a bit
more stable while we're gone.

[rsc, 2013-07-01 21:51:08]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-07-01 21:52:34]

LGTM but did you mean to have all those runtime printf in this CL?



On Mon, Jul 1, 2013 at 2:51 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: disable preemption
>
> There are various problems, and both Dmitriy and I
> will be away for the next week. Make the runtime a bit
> more stable while we're gone.
>
> Please review this at
https://codereview.appspot.**com/10848043/<https://codereview.appspot.com/108...
>
> Affected files:
>   M src/pkg/runtime/proc.c
>
>
> Index: src/pkg/runtime/proc.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/proc.c
> +++ b/src/pkg/runtime/proc.c
> @@ -359,33 +359,40 @@
>         int32 i;
>         uint32 s;
>         P *p;
> -       bool wait;
> +       int32 wait;
>
>         runtime·lock(&runtime·sched);
>         runtime·sched.stopwait = runtime·gomaxprocs;
>         runtime·atomicstore((uint32*)&**runtime·gcwaiting, 1);
> +runtime·printf("stoptheworld %d\n", runtime·gomaxprocs);
>         preemptall();
>         // stop current P
>         m->p->status = Pgcstop;
>         runtime·sched.stopwait--;
> +runtime·printf("1 current\n");
>         // try to retake all P's in Psyscall status
>         for(i = 0; i < runtime·gomaxprocs; i++) {
>                 p = runtime·allp[i];
>                 s = p->status;
> -               if(s == Psyscall && runtime·cas(&p->status, s, Pgcstop))
> +               if(s == Psyscall && runtime·cas(&p->status, s, Pgcstop)) {
> +runtime·printf("#%d in syscall\n", i);
>                         runtime·sched.stopwait--;
> +               }
>         }
>         // stop idle P's
>         while(p = pidleget()) {
>                 p->status = Pgcstop;
>                 runtime·sched.stopwait--;
> +runtime·printf("1 idle\n");
>         }
> -       wait = runtime·sched.stopwait > 0;
> +       wait = runtime·sched.stopwait;
> +runtime·printf("%d left\n", runtime·sched.stopwait);
>         runtime·unlock(&runtime·sched)**;
>
>         // wait for remaining P's to stop voluntarily
>         if(wait) {
>                 for(;;) {
> +runtime·printf("wait for %d %d of %d\n", wait, runtime·sched.stopwait,
> runtime·gomaxprocs);
>                         // wait for 100us, then try to re-preempt in case
> of any races
>                         if(runtime·notetsleep(&**runtime·sched.stopnote,
> 100*1000)) {
>                                 runtime·noteclear(&runtime·**
> sched.stopnote);
> @@ -876,6 +883,7 @@
>                 runtime·throw("startm: m has p");
>         mp->spinning = spinning;
>         mp->nextp = p;
> +runtime·printf("wakem %p\n", mp);
>         runtime·notewakeup(&mp->park);
>  }
>
> @@ -1954,6 +1962,7 @@
>                 pidleput(p);
>         }
>         runtime·singleproc = new == 1;
> +runtime·printf("gomaxprocs=%**d\n", new);
>         runtime·atomicstore((uint32*)&**runtime·gomaxprocs, new);
>  }
>
> @@ -2149,6 +2158,10 @@
>         M *mp;
>         G *gp;
>
> +// Preemption requires more robust traceback routines.
> +// For now, disable.
> +return;
> +
>         mp = p->m;
>         if(mp == nil || mp == m)
>                 return;
> @@ -2231,6 +2244,7 @@
>  static void
>  pidleput(P *p)
>  {
> +runtime·printf("pidleput\n");
>         p->link = runtime·sched.pidle;
>         runtime·sched.pidle = p;
>         runtime·xadd(&runtime·sched.**npidle, 1);  // TODO: fast atomic
> @@ -2245,6 +2259,7 @@
>
>         p = runtime·sched.pidle;
>         if(p) {
> +runtime·printf("pidleget\n");
>                 runtime·sched.pidle = p->link;
>                 runtime·xadd(&runtime·sched.**npidle, -1);  // TODO: fast
> atomic
>         }
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[rsc, 2013-07-01 21:56:55]

no, fixed.

g% hg diff @10848043
diff -r 0f6938710930 src/pkg/runtime/proc.c
--- a/src/pkg/runtime/proc.c Mon Jul 01 17:36:08 2013 -0400
+++ b/src/pkg/runtime/proc.c Mon Jul 01 17:56:40 2013 -0400
@@ -2149,6 +2149,12 @@
  M *mp;
  G *gp;

+// Preemption requires more robust traceback routines.
+// For now, disable.
+// The if(1) silences a compiler warning about the rest of the
+// function being unreachable.
+if(1) return;
+
  mp = p->m;
  if(mp == nil || mp == m)
  return;
g%

[rsc, 2013-07-01 21:57:13]

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

runtime: disable preemption

There are various problems, and both Dmitriy and I
will be away for the next week. Make the runtime a bit
more stable while we're gone.

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/10848043