runtime: don't start idle mark workers when barriers are cleared

Currently, we don't start dedicated or fractional mark workers unless
the mark 1 or mark 2 barriers have been cleared. One intended
consequence of this is that no background workers run between the
forEachP that disposes all gcWork caches and the beginning of mark 2.

However, we (unintentionally) did not apply this restriction to idle
mark workers. As a result, these can start in the interim between mark
1 completion and mark 2 starting. This explains why it was necessary
to reset the root marking jobs using carefully ordered atomic writes
when setting up mark 2. It also means that, even though we definitely
enqueue work before starting mark 2, it may be drained by the time we
reset the mark 2 barrier. If this happens, currently the only thing
preventing the runtime from deadlocking is that the scheduler itself
also checks for mark completion and will signal mark 2 completion.
Were it not for the odd behavior of idle workers, this check in the
scheduler would not be necessary.

Clean all of this up and prepare to remove this check in the scheduler
by applying the same restriction to starting idle mark workers.

Change-Id: Ic1b479e1591bd7773dc27b320ca399a215603b5a
Reviewed-on: https://go-review.googlesource.com/16631
Reviewed-by: Rick Hudson <rlh@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index eb0eac837f4ee034c91545c728e7029086e19499..39c08265b4e33bf49f0f9ba29a7b6f3672b1b032 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1772,10 +1772,11 @@ top:
 
 stop:
 
-       // We have nothing to do. If we're in the GC mark phase and can
-       // safely scan and blacken objects, run idle-time marking
-       // rather than give up the P.
-       if _p_ := _g_.m.p.ptr(); gcBlackenEnabled != 0 && _p_.gcBgMarkWorker != nil && gcMarkWorkAvailable(_p_) {
+       // We have nothing to do. If we're in the GC mark phase, can
+       // safely scan and blacken objects, can start a worker, and
+       // have work to do, run idle-time marking rather than give up
+       // the P.
+       if _p_ := _g_.m.p.ptr(); gcBlackenEnabled != 0 && _p_.gcBgMarkWorker != nil && (work.bgMark1.done == 0 || work.bgMark2.done == 0) && gcMarkWorkAvailable(_p_) {
                _p_.gcMarkWorkerMode = gcMarkWorkerIdleMode
                gp := _p_.gcBgMarkWorker
                casgstatus(gp, _Gwaiting, _Grunnable)