]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: always run semacquire on the G stack
authorKeith Randall <khr@golang.org>
Wed, 17 Sep 2014 00:26:16 +0000 (17:26 -0700)
committerKeith Randall <khr@golang.org>
Wed, 17 Sep 2014 00:26:16 +0000 (17:26 -0700)
semacquire might need to park the currently running G.  It can
only park if called from the G stack (because it has no way of
saving the M stack state).  So all calls to semacquire must come
from the G stack.

The three violators are GOMAXPROCS, ReadMemStats, and WriteHeapDump.
This change moves the semacquire call earlier, out of their C code
and into their Go code.

This seldom caused bugs because semacquire seldom actually had
to park the caller.  But it did happen intermittently.

Fixes #8749

LGTM=dvyukov
R=golang-codereviews, dvyukov, bradfitz
CC=golang-codereviews
https://golang.org/cl/144940043

src/runtime/debug.go
src/runtime/heapdump.c
src/runtime/mem.go
src/runtime/mgc0.c
src/runtime/proc.c
src/runtime/runtime.h
src/runtime/sema.go
src/runtime/stubs.go
src/runtime/thunk.s

index bb4bd60ed4dad3facd0f4f5da6799f010e7a76cb..4414dd55d2380341efb147daca7de4dff4da9dfe 100644 (file)
@@ -24,15 +24,29 @@ func UnlockOSThread()
 // The number of logical CPUs on the local machine can be queried with NumCPU.
 // This call will go away when the scheduler improves.
 func GOMAXPROCS(n int) int {
-       g := getg()
-       g.m.scalararg[0] = uintptr(n)
-       onM(gomaxprocs_m)
-       n = int(g.m.scalararg[0])
-       g.m.scalararg[0] = 0
-       return n
-}
+       if n > _MaxGomaxprocs {
+               n = _MaxGomaxprocs
+       }
+       lock(&sched.lock)
+       ret := int(gomaxprocs)
+       unlock(&sched.lock)
+       if n <= 0 || n == ret {
+               return ret
+       }
 
-func gomaxprocs_m() // proc.c
+       semacquire(&worldsema, false)
+       gp := getg()
+       gp.m.gcing = 1
+       onM(stoptheworld)
+
+       // newprocs will be processed by starttheworld
+       newprocs = int32(n)
+
+       gp.m.gcing = 0
+       semrelease(&worldsema)
+       onM(starttheworld)
+       return ret
+}
 
 // NumCPU returns the number of logical CPUs on the local machine.
 func NumCPU() int {
index 8bbc7d8a5677771a46d035500e255e90cf65b06b..75897c3d353272be5488760b2efb4a5d82e6810c 100644 (file)
@@ -737,33 +737,16 @@ mdump(void)
        flush();
 }
 
-static void writeheapdump_m(void);
-
-#pragma textflag NOSPLIT
 void
-runtime∕debug·WriteHeapDump(uintptr fd)
-{
-       void (*fn)(void);
-       
-       g->m->scalararg[0] = fd;
-       fn = writeheapdump_m;
-       runtime·onM(&fn);
-}
-
-static void
-writeheapdump_m(void)
+runtime·writeheapdump_m(void)
 {
        uintptr fd;
        
        fd = g->m->scalararg[0];
        g->m->scalararg[0] = 0;
 
-       // Stop the world.
        runtime·casgstatus(g->m->curg, Grunning, Gwaiting);
        g->waitreason = runtime·gostringnocopy((byte*)"dumping heap");
-       runtime·semacquire(&runtime·worldsema, false);
-       g->m->gcing = 1;
-       runtime·stoptheworld();
 
        // Update stats so we can dump them.
        // As a side effect, flushes all the MCaches so the MSpan.freelist
@@ -784,13 +767,7 @@ writeheapdump_m(void)
                tmpbufsize = 0;
        }
 
-       // Start up the world again.
-       g->m->gcing = 0;
-       g->m->locks++;
-       runtime·semrelease(&runtime·worldsema);
-       runtime·starttheworld();
        runtime·casgstatus(g->m->curg, Gwaiting, Grunning);
-       g->m->locks--;
 }
 
 // dumpint() the kind & offset of each field in an object.
index 34391b2eb28ec6e3fb8a8df59a471645e15ac884..99bb9285115db8295c403eaa68764fbd1b23d35a 100644 (file)
@@ -69,4 +69,39 @@ func init() {
 }
 
 // ReadMemStats populates m with memory allocator statistics.
-func ReadMemStats(m *MemStats)
+func ReadMemStats(m *MemStats) {
+       // Have to acquire worldsema to stop the world,
+       // because stoptheworld can only be used by
+       // one goroutine at a time, and there might be
+       // a pending garbage collection already calling it.
+       semacquire(&worldsema, false)
+       gp := getg()
+       gp.m.gcing = 1
+       onM(stoptheworld)
+
+       gp.m.ptrarg[0] = noescape(unsafe.Pointer(m))
+       onM(readmemstats_m)
+
+       gp.m.gcing = 0
+       gp.m.locks++
+       semrelease(&worldsema)
+       onM(starttheworld)
+       gp.m.locks--
+}
+
+// Implementation of runtime/debug.WriteHeapDump
+func writeHeapDump(fd uintptr) {
+       semacquire(&worldsema, false)
+       gp := getg()
+       gp.m.gcing = 1
+       onM(stoptheworld)
+
+       gp.m.scalararg[0] = fd
+       onM(writeheapdump_m)
+
+       gp.m.gcing = 0
+       gp.m.locks++
+       semrelease(&worldsema)
+       onM(starttheworld)
+       gp.m.locks--
+}
index fda3efcc18edeff589c0c32412321203966ae246..35aed78a53ffc95cd24ed5fee12d1f3a3aef0cad 100644 (file)
@@ -1451,32 +1451,14 @@ extern uintptr runtime·sizeof_C_MStats;
 
 static void readmemstats_m(void);
 
-#pragma textflag NOSPLIT
 void
-runtime·ReadMemStats(MStats *stats)
-{
-       void (*fn)(void);
-       
-       g->m->ptrarg[0] = stats;
-       fn = readmemstats_m;
-       runtime·onM(&fn);
-}
-
-static void
-readmemstats_m(void)
+runtime·readmemstats_m(void)
 {
        MStats *stats;
        
        stats = g->m->ptrarg[0];
        g->m->ptrarg[0] = nil;
 
-       // Have to acquire worldsema to stop the world,
-       // because stoptheworld can only be used by
-       // one goroutine at a time, and there might be
-       // a pending garbage collection already calling it.
-       runtime·semacquire(&runtime·worldsema, false);
-       g->m->gcing = 1;
-       runtime·stoptheworld();
        runtime·updatememstats(nil);
        // Size of the trailing by_size array differs between Go and C,
        // NumSizeClasses was changed, but we can not change Go struct because of backward compatibility.
@@ -1486,12 +1468,6 @@ readmemstats_m(void)
        stats->stacks_sys = stats->stacks_inuse;
        stats->heap_inuse -= stats->stacks_inuse;
        stats->heap_sys -= stats->stacks_inuse;
-       
-       g->m->gcing = 0;
-       g->m->locks++;
-       runtime·semrelease(&runtime·worldsema);
-       runtime·starttheworld();
-       g->m->locks--;
 }
 
 static void readgcstats_m(void);
index 25f916640314ba4360e70528fcd5c3fe546365db..0e677a9d28a8f35ce377fcf3a15a3afdc5ce7ca4 100644 (file)
 //
 // Design doc at http://golang.org/s/go11sched.
 
-typedef struct Sched Sched;
-struct Sched {
-       Mutex   lock;
-
-       uint64  goidgen;
-
-       M*      midle;   // idle m's waiting for work
-       int32   nmidle;  // number of idle m's waiting for work
-       int32   nmidlelocked; // number of locked m's waiting for work
-       int32   mcount;  // number of m's that have been created
-       int32   maxmcount;      // maximum number of m's allowed (or die)
-
-       P*      pidle;  // idle P's
-       uint32  npidle;
-       uint32  nmspinning;
-
-       // Global runnable queue.
-       G*      runqhead;
-       G*      runqtail;
-       int32   runqsize;
-
-       // Global cache of dead G's.
-       Mutex   gflock;
-       G*      gfree;
-       int32   ngfree;
-
-       uint32  gcwaiting;      // gc is waiting to run
-       int32   stopwait;
-       Note    stopnote;
-       uint32  sysmonwait;
-       Note    sysmonnote;
-       uint64  lastpoll;
-
-       int32   profilehz;      // cpu profiling rate
-};
-
 enum
 {
        // Number of goroutine ids to grab from runtime·sched.goidgen to local per-P cache at once.
@@ -67,7 +31,7 @@ enum
        GoidCacheBatch = 16,
 };
 
-Sched  runtime·sched;
+SchedType      runtime·sched;
 int32  runtime·gomaxprocs;
 uint32 runtime·needextram;
 bool   runtime·iscgo;
@@ -79,7 +43,7 @@ M*    runtime·extram;
 P*     runtime·allp[MaxGomaxprocs+1];
 int8*  runtime·goos;
 int32  runtime·ncpu;
-static int32   newprocs;
+int32  runtime·newprocs;
 
 Mutex runtime·allglock;       // the following vars are protected by this lock or by stoptheworld
 G**    runtime·allg;
@@ -763,9 +727,9 @@ runtime·starttheworld(void)
        injectglist(gp);
        add = needaddgcproc();
        runtime·lock(&runtime·sched.lock);
-       if(newprocs) {
-               procresize(newprocs);
-               newprocs = 0;
+       if(runtime·newprocs) {
+               procresize(runtime·newprocs);
+               runtime·newprocs = 0;
        } else
                procresize(runtime·gomaxprocs);
        runtime·sched.gcwaiting = 0;
@@ -2364,39 +2328,6 @@ runtime·Breakpoint(void)
        runtime·breakpoint();
 }
 
-// Implementation of runtime.GOMAXPROCS.
-// delete when scheduler is even stronger
-void
-runtime·gomaxprocs_m(void)
-{
-       int32 n, ret;
-       
-       n = g->m->scalararg[0];
-       g->m->scalararg[0] = 0;
-
-       if(n > MaxGomaxprocs)
-               n = MaxGomaxprocs;
-       runtime·lock(&runtime·sched.lock);
-       ret = runtime·gomaxprocs;
-       if(n <= 0 || n == ret) {
-               runtime·unlock(&runtime·sched.lock);
-               g->m->scalararg[0] = ret;
-               return;
-       }
-       runtime·unlock(&runtime·sched.lock);
-
-       runtime·semacquire(&runtime·worldsema, false);
-       g->m->gcing = 1;
-       runtime·stoptheworld();
-       newprocs = n;
-       g->m->gcing = 0;
-       runtime·semrelease(&runtime·worldsema);
-       runtime·starttheworld();
-
-       g->m->scalararg[0] = ret;
-       return;
-}
-
 // lockOSThread is called by runtime.LockOSThread and runtime.lockOSThread below
 // after they modify m->locked. Do not allow preemption during this call,
 // or else the m might be different in this function than in the caller.
index adc74cf417f848eb09205db926001ef91203978e..c034f3aa9770957f81623640c6dceed397aa9a2c 100644 (file)
@@ -60,6 +60,7 @@ typedef       struct  SudoG           SudoG;
 typedef        struct  Mutex           Mutex;
 typedef        struct  M               M;
 typedef        struct  P               P;
+typedef struct SchedType       SchedType;
 typedef        struct  Note            Note;
 typedef        struct  Slice           Slice;
 typedef        struct  String          String;
@@ -433,6 +434,42 @@ enum {
        MaxGomaxprocs = 1<<8,
 };
 
+struct SchedType
+{
+       Mutex   lock;
+
+       uint64  goidgen;
+
+       M*      midle;   // idle m's waiting for work
+       int32   nmidle;  // number of idle m's waiting for work
+       int32   nmidlelocked; // number of locked m's waiting for work
+       int32   mcount;  // number of m's that have been created
+       int32   maxmcount;      // maximum number of m's allowed (or die)
+
+       P*      pidle;  // idle P's
+       uint32  npidle;
+       uint32  nmspinning;
+
+       // Global runnable queue.
+       G*      runqhead;
+       G*      runqtail;
+       int32   runqsize;
+
+       // Global cache of dead G's.
+       Mutex   gflock;
+       G*      gfree;
+       int32   ngfree;
+
+       uint32  gcwaiting;      // gc is waiting to run
+       int32   stopwait;
+       Note    stopnote;
+       uint32  sysmonwait;
+       Note    sysmonnote;
+       uint64  lastpoll;
+
+       int32   profilehz;      // cpu profiling rate
+};
+
 // The m->locked word holds two pieces of state counting active calls to LockOSThread/lockOSThread.
 // The low bit (LockExternal) is a boolean reporting whether any LockOSThread call is active.
 // External locks are not recursive; a second lock is silently ignored.
@@ -716,6 +753,8 @@ extern      DebugVars       runtime·debug;
 extern uintptr runtime·maxstacksize;
 extern Note    runtime·signote;
 extern ForceGCState    runtime·forcegc;
+extern SchedType       runtime·sched;
+extern int32           runtime·newprocs;
 
 /*
  * common functions and data
index beacd671621b066b053609707abe961943a2b79e..504462de330f4114031285dccae210f118c5bcbd 100644 (file)
@@ -49,6 +49,11 @@ func asyncsemrelease(addr *uint32) {
 
 // Called from runtime.
 func semacquire(addr *uint32, profile bool) {
+       gp := getg()
+       if gp != gp.m.curg {
+               gothrow("semacquire not on the G stack")
+       }
+
        // Easy case.
        if cansemacquire(addr) {
                return
index ff443c4cd4b2c85fe148ae6748655b79e69416ba..2e6aadca7a562ae70a7ceac9f9ac335c6a26bb2a 100644 (file)
@@ -119,6 +119,8 @@ func deferproc_m()
 func goexit_m()
 func startpanic_m()
 func dopanic_m()
+func readmemstats_m()
+func writeheapdump_m()
 
 // memclr clears n bytes starting at ptr.
 // in memclr_*.s
index 3b66cf47d31c6b29c477108bfc50ce286ee3687b..3dd86e99195068f27766a406a69440ebbc48e5e5 100644 (file)
@@ -80,6 +80,9 @@ TEXT reflect·memmove(SB), NOSPLIT, $0-0
 TEXT runtime∕debug·freeOSMemory(SB), NOSPLIT, $0-0
        JMP     runtime·freeOSMemory(SB)
 
+TEXT runtime∕debug·WriteHeapDump(SB), NOSPLIT, $0-0
+       JMP     runtime·writeHeapDump(SB)
+
 TEXT net·runtime_pollServerInit(SB),NOSPLIT,$0-0
        JMP     runtime·netpollServerInit(SB)