diff options
author | Russ Cox <rsc@golang.org> | 2011-02-23 15:51:20 -0500 |
---|---|---|
committer | Russ Cox <rsc@golang.org> | 2011-02-23 15:51:20 -0500 |
commit | b5dfac45ba29f12cbc86925ab7c7cd018f87f4fe (patch) | |
tree | 84da18dce1a9135bbf30f3bee0abcd86a582c6fb | |
parent | 59ce067da8d9e7d968e51f31fb532ed2369f205a (diff) | |
download | go-b5dfac45ba29f12cbc86925ab7c7cd018f87f4fe.tar.gz go-b5dfac45ba29f12cbc86925ab7c7cd018f87f4fe.zip |
runtime: always run stackalloc on scheduler stack
Avoids deadlocks like the one below, in which a stack split happened
in order to call lock(&stacks), but then the stack unsplit cannot run
because stacks is now locked.
The only code calling stackalloc that wasn't on a scheduler
stack already was malg, which creates a new goroutine.
runtime.futex+0x23 /home/rsc/g/go/src/pkg/runtime/linux/amd64/sys.s:139
runtime.futex()
futexsleep+0x50 /home/rsc/g/go/src/pkg/runtime/linux/thread.c:51
futexsleep(0x5b0188, 0x300000003, 0x100020000, 0x4159e2)
futexlock+0x85 /home/rsc/g/go/src/pkg/runtime/linux/thread.c:119
futexlock(0x5b0188, 0x5b0188)
runtime.lock+0x56 /home/rsc/g/go/src/pkg/runtime/linux/thread.c:158
runtime.lock(0x5b0188, 0x7f0d27b4a000)
runtime.stackfree+0x4d /home/rsc/g/go/src/pkg/runtime/malloc.goc:336
runtime.stackfree(0x7f0d27b4a000, 0x1000, 0x8, 0x7fff37e1e218)
runtime.oldstack+0xa6 /home/rsc/g/go/src/pkg/runtime/proc.c:705
runtime.oldstack()
runtime.lessstack+0x22 /home/rsc/g/go/src/pkg/runtime/amd64/asm.s:224
runtime.lessstack()
----- lessstack called from goroutine 2 -----
runtime.lock+0x56 /home/rsc/g/go/src/pkg/runtime/linux/thread.c:158
runtime.lock(0x5b0188, 0x40a5e2)
runtime.stackalloc+0x55 /home/rsc/g/go/src/pkg/runtime/malloc.c:316
runtime.stackalloc(0x1000, 0x4055b0)
runtime.malg+0x3d /home/rsc/g/go/src/pkg/runtime/proc.c:803
runtime.malg(0x1000, 0x40add9)
runtime.newproc1+0x12b /home/rsc/g/go/src/pkg/runtime/proc.c:854
runtime.newproc1(0xf840027440, 0x7f0d27b49230, 0x0, 0x49f238, 0x40, ...)
runtime.newproc+0x2f /home/rsc/g/go/src/pkg/runtime/proc.c:831
runtime.newproc(0x0, 0xf840027440, 0xf800000010, 0x44b059)
...
R=r, r2
CC=golang-dev
https://golang.org/cl/4216045
-rw-r--r-- | src/pkg/runtime/malloc.goc | 6 | ||||
-rw-r--r-- | src/pkg/runtime/mgc0.c | 1 | ||||
-rw-r--r-- | src/pkg/runtime/proc.c | 46 | ||||
-rw-r--r-- | src/pkg/runtime/runtime.h | 1 |
4 files changed, 45 insertions, 9 deletions
diff --git a/src/pkg/runtime/malloc.goc b/src/pkg/runtime/malloc.goc index abbf63b931..41060682eb 100644 --- a/src/pkg/runtime/malloc.goc +++ b/src/pkg/runtime/malloc.goc @@ -394,6 +394,12 @@ runtime·stackalloc(uint32 n) { void *v; + // Stackalloc must be called on scheduler stack, so that we + // never try to grow the stack during the code that stackalloc runs. + // Doing so would cause a deadlock (issue 1547). + if(g != m->g0) + runtime·throw("stackalloc not on scheduler stack"); + if(m->mallocing || m->gcing || n == FixedStack) { runtime·lock(&stacks); if(stacks.size == 0) diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c index c471fff5e8..1d382580fa 100644 --- a/src/pkg/runtime/mgc0.c +++ b/src/pkg/runtime/mgc0.c @@ -380,6 +380,7 @@ mark(void) break; case Grunning: case Grecovery: + case Gstackalloc: if(gp != g) runtime·throw("mark - world not stopped"); scanstack(gp); diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c index 1bbca63177..455a39e22b 100644 --- a/src/pkg/runtime/proc.c +++ b/src/pkg/runtime/proc.c @@ -253,8 +253,10 @@ readylocked(G *g) } // Mark runnable. - if(g->status == Grunnable || g->status == Grunning || g->status == Grecovery) + if(g->status == Grunnable || g->status == Grunning || g->status == Grecovery || g->status == Gstackalloc) { + runtime·printf("goroutine %d has status %d\n", g->goid, g->status); runtime·throw("bad g->status in ready"); + } g->status = Grunnable; gput(g); @@ -492,6 +494,13 @@ scheduler(void) runtime·free(d); runtime·gogo(&gp->sched, 1); } + + if(gp->status == Gstackalloc) { + // switched to scheduler stack to call stackalloc. + gp->param = runtime·stackalloc((uintptr)gp->param); + gp->status = Grunning; + runtime·gogo(&gp->sched, 1); + } // Jumped here via runtime·gosave/gogo, so didn't // execute lock(&runtime·sched) above. @@ -509,6 +518,8 @@ scheduler(void) switch(gp->status){ case Grunnable: case Gdead: + case Grecovery: + case Gstackalloc: // Shouldn't have been running! runtime·throw("bad gp->status in sched"); case Grunning: @@ -795,18 +806,35 @@ runtime·newstack(void) G* runtime·malg(int32 stacksize) { - G *g; + G *newg; byte *stk; + int32 oldstatus; - g = runtime·malloc(sizeof(G)); + newg = runtime·malloc(sizeof(G)); if(stacksize >= 0) { - stk = runtime·stackalloc(StackSystem + stacksize); - g->stack0 = stk; - g->stackguard = stk + StackSystem + StackGuard; - g->stackbase = stk + StackSystem + stacksize - sizeof(Stktop); - runtime·memclr(g->stackbase, sizeof(Stktop)); + if(g == m->g0) { + // running on scheduler stack already. + stk = runtime·stackalloc(StackSystem + stacksize); + } else { + // have to call stackalloc on scheduler stack. + oldstatus = g->status; + g->param = (void*)(StackSystem + stacksize); + g->status = Gstackalloc; + // next two lines are runtime·gosched without the check + // of m->locks. we're almost certainly holding a lock, + // but this is not a real rescheduling so it's okay. + if(runtime·gosave(&g->sched) == 0) + runtime·gogo(&m->sched, 1); + stk = g->param; + g->param = nil; + g->status = oldstatus; + } + newg->stack0 = stk; + newg->stackguard = stk + StackSystem + StackGuard; + newg->stackbase = stk + StackSystem + stacksize - sizeof(Stktop); + runtime·memclr(newg->stackbase, sizeof(Stktop)); } - return g; + return newg; } /* diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h index ac992a2f1b..4456e9b8d4 100644 --- a/src/pkg/runtime/runtime.h +++ b/src/pkg/runtime/runtime.h @@ -104,6 +104,7 @@ enum Gmoribund, Gdead, Grecovery, + Gstackalloc, }; enum { |