From dbcfed93e75b91819bd01eb228996073b18c8196 Mon Sep 17 00:00:00 2001 From: Dmitriy Vyukov Date: Wed, 12 Jun 2013 18:46:35 +0400 Subject: [PATCH] runtime: fix scheduler race condition In starttheworld() we assume that P's with local work are situated in the beginning of idle P list. However, once we start the first M, it can execute all local G's and steal G's from other P's. That breaks the assumption above. Thus starttheworld() will fail to start some P's with local work. It seems that it can not lead to very bad things, but still it's wrong and breaks other assumtions (e.g. we can have a spinning M with local work). The fix is to collect all P's with local work first, and only then start them. R=golang-dev, rsc CC=golang-dev https://golang.org/cl/10051045 --- src/pkg/runtime/proc.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c index 432298a9ca..5b3dbab7e0 100644 --- a/src/pkg/runtime/proc.c +++ b/src/pkg/runtime/proc.c @@ -420,16 +420,9 @@ runtime·starttheworld(void) pidleput(p); break; } - mp = mget(); - if(mp == nil) { - p->link = p1; - p1 = p; - continue; - } - if(mp->nextp) - runtime·throw("starttheworld: inconsistent mp->nextp"); - mp->nextp = p; - runtime·notewakeup(&mp->park); + p->m = mget(); + p->link = p1; + p1 = p; } if(runtime·sched.sysmonwait) { runtime·sched.sysmonwait = false; @@ -440,8 +433,18 @@ runtime·starttheworld(void) while(p1) { p = p1; p1 = p1->link; - add = false; - newm(nil, p); + if(p->m) { + mp = p->m; + p->m = nil; + if(mp->nextp) + runtime·throw("starttheworld: inconsistent mp->nextp"); + mp->nextp = p; + runtime·notewakeup(&mp->park); + } else { + // Start M to run P. Do not start another M below. + newm(nil, p); + add = false; + } } if(add) { @@ -1154,6 +1157,8 @@ top: } gp = runqget(m->p); + if(gp && m->spinning) + runtime·throw("schedule: spinning with local work"); if(gp == nil) gp = findrunnable(); -- 2.48.1