runtime: fix a few GC-related bugs

1) Move non-preemption check even earlier in newstack.
This avoids a few priority inversion problems.

2) Always use atomic operations to update bitmap for 1-word objects.
This avoids lost mark bits during concurrent GC.

3) Stop using work.nproc == 1 as a signal for being single-threaded.
The concurrent GC runs with work.nproc == 1 but other procs are
running mutator code.

The use of work.nproc == 1 in getfull *is* safe, but remove it anyway,
since it is saving only a single atomic operation per GC round.

Fixes #9225.

Change-Id: I24134f100ad592ea8cb59efb6a54f5a1311093dc
Reviewed-on: https://go-review.googlesource.com/2745
Reviewed-by: Rick Hudson <rlh@golang.org>
This commit is contained in:
Russ Cox 2015-01-13 15:55:16 -05:00
parent b8d67596f6
commit aae0f074c0
3 changed files with 42 additions and 36 deletions

View file

@ -634,21 +634,39 @@ func newstack() {
throw("runtime: stack split at bad time")
}
// The goroutine must be executing in order to call newstack,
// so it must be Grunning or Gscanrunning.
gp := thisg.m.curg
morebuf := thisg.m.morebuf
thisg.m.morebuf.pc = 0
thisg.m.morebuf.lr = 0
thisg.m.morebuf.sp = 0
thisg.m.morebuf.g = 0
rewindmorestack(&gp.sched)
// Be conservative about where we preempt.
// We are interested in preempting user Go code, not runtime code.
// If we're holding locks, mallocing, or GCing, don't preempt.
// This check is very early in newstack so that even the status change
// from Grunning to Gwaiting and back doesn't happen in this case.
// That status change by itself can be viewed as a small preemption,
// because the GC might change Gwaiting to Gscanwaiting, and then
// this goroutine has to wait for the GC to finish before continuing.
// If the GC is in some way dependent on this goroutine (for example,
// it needs a lock held by the goroutine), that small preemption turns
// into a real deadlock.
if gp.stackguard0 == stackPreempt {
if thisg.m.locks != 0 || thisg.m.mallocing != 0 || thisg.m.gcing != 0 || thisg.m.p.status != _Prunning {
// Let the goroutine keep running for now.
// gp->preempt is set, so it will be preempted next time.
gp.stackguard0 = gp.stack.lo + _StackGuard
gogo(&gp.sched) // never return
}
}
// The goroutine must be executing in order to call newstack,
// so it must be Grunning (or Gscanrunning).
casgstatus(gp, _Grunning, _Gwaiting)
gp.waitreason = "stack growth"
rewindmorestack(&gp.sched)
if gp.stack.lo == 0 {
throw("missing stack in newstack")
}
@ -697,16 +715,6 @@ func newstack() {
gogo(&gp.sched) // never return
}
// Be conservative about where we preempt.
// We are interested in preempting user Go code, not runtime code.
if thisg.m.locks != 0 || thisg.m.mallocing != 0 || thisg.m.gcing != 0 || thisg.m.p.status != _Prunning {
// Let the goroutine keep running for now.
// gp->preempt is set, so it will be preempted next time.
gp.stackguard0 = gp.stack.lo + _StackGuard
casgstatus(gp, _Gwaiting, _Grunning)
gogo(&gp.sched) // never return
}
// Act like goroutine called runtime.Gosched.
casgstatus(gp, _Gwaiting, _Grunning)
gosched_m(gp) // never return