From 1e112cd59f560129f4dca5e9af7c3cbc445850b6 Mon Sep 17 00:00:00 2001 From: Dmitriy Vyukov Date: Fri, 28 Jun 2013 17:52:17 +0400 Subject: [PATCH] runtime: preempt goroutines for GC The last patch for preemptive scheduler, with this change stoptheworld issues preemption requests every 100us. Update #543. R=golang-dev, daniel.morsing, rsc CC=golang-dev https://golang.org/cl/10264044 --- src/pkg/runtime/proc.c | 20 +++++++++++++------- src/pkg/runtime/proc_test.go | 30 ++++++++++++++++++++++++++++++ src/pkg/runtime/runtime.h | 1 + src/pkg/runtime/stack.c | 24 +++++++++++++++++++++--- src/pkg/runtime/stack.h | 11 ++++++----- 5 files changed, 71 insertions(+), 15 deletions(-) diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c index 12ca09849c..44892e8540 100644 --- a/src/pkg/runtime/proc.c +++ b/src/pkg/runtime/proc.c @@ -99,7 +99,6 @@ static void inclocked(int32); static void checkdead(void); static void exitsyscall0(G*); static void park0(G*); -static void gosched0(G*); static void goexit0(G*); static void gfput(P*, G*); static G* gfget(P*); @@ -364,6 +363,7 @@ runtime·stoptheworld(void) runtime·lock(&runtime·sched); runtime·sched.stopwait = runtime·gomaxprocs; runtime·atomicstore((uint32*)&runtime·gcwaiting, 1); + preemptall(); // stop current P m->p->status = Pgcstop; runtime·sched.stopwait--; @@ -382,10 +382,16 @@ runtime·stoptheworld(void) wait = runtime·sched.stopwait > 0; runtime·unlock(&runtime·sched); - // wait for remaining P's to stop voluntary + // wait for remaining P's to stop voluntarily if(wait) { - runtime·notesleep(&runtime·sched.stopnote); - runtime·noteclear(&runtime·sched.stopnote); + for(;;) { + // 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); + break; + } + preemptall(); + } } if(runtime·sched.stopwait) runtime·throw("stoptheworld: not stopped"); @@ -1240,12 +1246,12 @@ park0(G *gp) void runtime·gosched(void) { - runtime·mcall(gosched0); + runtime·mcall(runtime·gosched0); } // runtime·gosched continuation on g0. -static void -gosched0(G *gp) +void +runtime·gosched0(G *gp) { gp->status = Grunnable; gp->m = nil; diff --git a/src/pkg/runtime/proc_test.go b/src/pkg/runtime/proc_test.go index 83368e0c33..605f747cbe 100644 --- a/src/pkg/runtime/proc_test.go +++ b/src/pkg/runtime/proc_test.go @@ -157,6 +157,36 @@ func TestTimerFairness2(t *testing.T) { <-done } +// The function is used to test preemption at split stack checks. +// Declaring a var avoids inlining at the call site. +var preempt = func() int { + var a [128]int + sum := 0 + for _, v := range a { + sum += v + } + return sum +} + +func TestPreemptionGC(t *testing.T) { + // Test that pending GC preempts running goroutines. + const P = 5 + defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(P + 1)) + var stop uint32 + for i := 0; i < P; i++ { + go func() { + for atomic.LoadUint32(&stop) == 0 { + preempt() + } + }() + } + for i := 0; i < 10; i++ { + runtime.Gosched() + runtime.GC() + } + atomic.StoreUint32(&stop, 1) +} + func stackGrowthRecursive(i int) { var pad [128]uint64 if i != 0 && pad[0] == 0 { diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h index 4fb022c39c..8b3f10f945 100644 --- a/src/pkg/runtime/runtime.h +++ b/src/pkg/runtime/runtime.h @@ -809,6 +809,7 @@ void runtime·newextram(void); void runtime·exit(int32); void runtime·breakpoint(void); void runtime·gosched(void); +void runtime·gosched0(G*); void runtime·park(void(*)(Lock*), Lock*, int8*); void runtime·tsleep(int64, int8*); M* runtime·newm(void); diff --git a/src/pkg/runtime/stack.c b/src/pkg/runtime/stack.c index 16dfa041a0..2150d5ec1f 100644 --- a/src/pkg/runtime/stack.c +++ b/src/pkg/runtime/stack.c @@ -211,7 +211,10 @@ runtime·newstack(void) gp->status = Gwaiting; gp->waitreason = "stack split"; reflectcall = framesize==1; + if(reflectcall) + framesize = 0; + // For reflectcall the context already points to beginning of reflect·call. if(!reflectcall) runtime·rewindmorestack(&gp->sched); @@ -238,9 +241,24 @@ runtime·newstack(void) runtime·throw("runtime: stack split argsize"); } - reflectcall = framesize==1; - if(reflectcall) - framesize = 0; + if(gp->stackguard0 == StackPreempt) { + if(gp == m->g0) + runtime·throw("runtime: preempt g0"); + if(oldstatus == Grunning && (m->p == nil || m->p->status != Prunning)) + runtime·throw("runtime: g is running but p is not"); + // Be conservative about where we preempt. + // We are interested in preempting user Go code, not runtime code. + if(oldstatus != Grunning || m->locks || m->mallocing || m->gcing) { + // Let the goroutine keep running for now. + // TODO(dvyukov): remember but delay the preemption. + gp->stackguard0 = gp->stackguard; + gp->status = oldstatus; + runtime·gogo(&gp->sched); // never return + } + // Act like goroutine called runtime.Gosched. + gp->status = oldstatus; + runtime·gosched0(gp); // never return + } if(reflectcall && m->morebuf.sp - sizeof(Stktop) - argsize - 32 > gp->stackguard) { // special case: called from reflect.call (framesize==1) diff --git a/src/pkg/runtime/stack.h b/src/pkg/runtime/stack.h index b6924c198e..f56d4a7263 100644 --- a/src/pkg/runtime/stack.h +++ b/src/pkg/runtime/stack.h @@ -103,9 +103,10 @@ enum { // The actual size can be smaller than this but cannot be larger. // Checked in proc.c's runtime.malg. StackTop = 96, - - // Goroutine preemption request. - // Stored into g->stackguard0 to cause split stack check failure. - // Must be greater than any real sp. - StackPreempt = (uintptr)(intptr)0xfffffade, }; + +// Goroutine preemption request. +// Stored into g->stackguard0 to cause split stack check failure. +// Must be greater than any real sp. +// 0xfffffade in hex. +#define StackPreempt ((uintptr)-1314) -- 2.48.1