Skip to content

Commit 5887f14

Browse files
dvyukovrsc
authored andcommitted
runtime: more reliable preemption
Currently preemption signal g->stackguard0==StackPreempt can be lost if it is received when preemption is disabled (e.g. m->lock!=0). This change duplicates the preemption signal in g->preempt and restores g->stackguard0 when preemption is enabled. Update #543. R=golang-dev, rsc CC=golang-dev https://golang.org/cl/10792043
1 parent a837485 commit 5887f14

File tree

7 files changed

+28
-2
lines changed

7 files changed

+28
-2
lines changed

src/pkg/runtime/lock_futex.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// +build freebsd linux
66

77
#include "runtime.h"
8+
#include "stack.h"
89

910
// This implementation depends on OS-specific implementations of
1011
//
@@ -99,6 +100,8 @@ runtime·unlock(Lock *l)
99100

100101
if(--m->locks < 0)
101102
runtime·throw("runtime·unlock: lock count");
103+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
104+
g->stackguard0 = StackPreempt;
102105
}
103106

104107
// One-time notifications.

src/pkg/runtime/lock_sema.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// +build darwin netbsd openbsd plan9 windows
66

77
#include "runtime.h"
8+
#include "stack.h"
89

910
// This implementation depends on OS-specific implementations of
1011
//
@@ -112,6 +113,8 @@ runtime·unlock(Lock *l)
112113

113114
if(--m->locks < 0)
114115
runtime·throw("runtime·unlock: lock count");
116+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
117+
g->stackguard0 = StackPreempt;
115118
}
116119

117120
// One-time notifications.

src/pkg/runtime/malloc.goc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ package runtime
1313
#include "type.h"
1414
#include "typekind.h"
1515
#include "race.h"
16+
#include "stack.h"
1617

1718
MHeap runtime·mheap;
1819

@@ -94,6 +95,8 @@ runtime·mallocgc(uintptr size, uint32 flag, int32 dogc, int32 zeroed)
9495
*(uintptr*)((uintptr)v+size-sizeof(uintptr)) = 0;
9596

9697
m->mallocing = 0;
98+
if(g->preempt) // restore the preemption request in case we've cleared it in newstack
99+
g->stackguard0 = StackPreempt;
97100

98101
if(!(flag & FlagNoProfiling) && (rate = runtime·MemProfileRate) > 0) {
99102
if(size >= rate)

src/pkg/runtime/mgc0.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2014,6 +2014,7 @@ runtime·gc(int32 force)
20142014
}
20152015

20162016
// all done
2017+
m->gcing = 0;
20172018
runtime·semrelease(&runtime·worldsema);
20182019
runtime·starttheworld();
20192020

@@ -2031,6 +2032,8 @@ runtime·gc(int32 force)
20312032
// give the queued finalizers, if any, a chance to run
20322033
runtime·gosched();
20332034
}
2035+
if(g->preempt) // restore the preemption request in case we've cleared it in newstack
2036+
g->stackguard0 = StackPreempt;
20342037
}
20352038

20362039
static void
@@ -2115,7 +2118,6 @@ gc(struct gc_args *args)
21152118

21162119
cachestats();
21172120
mstats.next_gc = mstats.heap_alloc+mstats.heap_alloc*gcpercent/100;
2118-
m->gcing = 0;
21192121

21202122
t4 = runtime·nanotime();
21212123
mstats.last_gc = t4;

src/pkg/runtime/proc.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ runtime·ready(G *gp)
294294
if(runtime·atomicload(&runtime·sched.npidle) != 0 && runtime·atomicload(&runtime·sched.nmspinning) == 0) // TODO: fast atomic
295295
wakep();
296296
m->locks--;
297+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
298+
g->stackguard0 = StackPreempt;
297299
}
298300

299301
int32
@@ -475,6 +477,8 @@ runtime·starttheworld(void)
475477
newm(mhelpgc, nil);
476478
}
477479
m->locks--;
480+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
481+
g->stackguard0 = StackPreempt;
478482
}
479483

480484
// Called to start an M.
@@ -564,6 +568,8 @@ runtime·allocm(P *p)
564568
if(p == m->p)
565569
releasep();
566570
m->locks--;
571+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
572+
g->stackguard0 = StackPreempt;
567573

568574
return mp;
569575
}
@@ -1008,6 +1014,7 @@ execute(G *gp)
10081014
runtime·throw("execute: bad g status");
10091015
}
10101016
gp->status = Grunning;
1017+
gp->preempt = false;
10111018
gp->stackguard0 = gp->stackguard;
10121019
m->p->tick++;
10131020
m->curg = gp;
@@ -1433,6 +1440,8 @@ runtime·exitsyscall(void)
14331440
// so okay to clear gcstack and gcsp.
14341441
g->gcstack = (uintptr)nil;
14351442
g->gcsp = (uintptr)nil;
1443+
if(g->preempt) // restore the preemption request in case we've cleared it in newstack
1444+
g->stackguard0 = StackPreempt;
14361445
return;
14371446
}
14381447

@@ -1450,6 +1459,8 @@ runtime·exitsyscall(void)
14501459
g->status = Grunning;
14511460
g->gcstack = (uintptr)nil;
14521461
g->gcsp = (uintptr)nil;
1462+
if(g->preempt) // restore the preemption request in case we've cleared it in newstack
1463+
g->stackguard0 = StackPreempt;
14531464
return;
14541465
}
14551466
}
@@ -1620,6 +1631,8 @@ runtime·newproc1(FuncVal *fn, byte *argp, int32 narg, int32 nret, void *callerp
16201631
if(runtime·atomicload(&runtime·sched.npidle) != 0 && runtime·atomicload(&runtime·sched.nmspinning) == 0 && fn->fn != runtime·main) // TODO: fast atomic
16211632
wakep();
16221633
m->locks--;
1634+
if(m->locks == 0 && g->preempt) // restore the preemption request in case we've cleared it in newstack
1635+
g->stackguard0 = StackPreempt;
16231636
return newg;
16241637
}
16251638

@@ -2174,6 +2187,7 @@ if(1) return;
21742187
gp = mp->curg;
21752188
if(gp == nil || gp == mp->g0)
21762189
return;
2190+
gp->preempt = true;
21772191
gp->stackguard0 = StackPreempt;
21782192
}
21792193

src/pkg/runtime/runtime.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ struct G
253253
bool issystem; // do not output in stack dump
254254
bool isbackground; // ignore in deadlock detector
255255
bool blockingsyscall; // hint that the next syscall will block
256+
bool preempt; // preemption signal, duplicates stackguard0 = StackPreempt
256257
int8 raceignore; // ignore race detection events
257258
M* m; // for debuggers, but offset not hard-coded
258259
M* lockedm;

src/pkg/runtime/stack.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ runtime·newstack(void)
250250
// We are interested in preempting user Go code, not runtime code.
251251
if(oldstatus != Grunning || m->locks || m->mallocing || m->gcing) {
252252
// Let the goroutine keep running for now.
253-
// TODO(dvyukov): remember but delay the preemption.
253+
// gp->preempt is set, so it will be preempted next time.
254254
gp->stackguard0 = gp->stackguard;
255255
gp->status = oldstatus;
256256
runtime·gogo(&gp->sched); // never return

0 commit comments

Comments
 (0)