From 20f276e237c4b312d4b62a1a83db84ce64229752 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Tue, 3 Nov 2015 19:47:07 -0500 Subject: [PATCH] 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 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot --- src/runtime/proc.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index eb0eac837f4..39c08265b4e 100644 --- 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)