runtime: replace STW for enabling write barriers with ragged barrier

Currently, we use a full stop-the-world around enabling write
barriers. This is to ensure that all Gs have enabled write barriers
before any blackening occurs (either in gcBgMarkWorker() or in
gcAssistAlloc()).

However, there's no need to bring the whole world to a synchronous
stop to ensure this. This change replaces the STW with a ragged
barrier that ensures each P has individually observed that write
barriers should be enabled before GC performs any blackening.

Change-Id: If2f129a6a55bd8bdd4308067af2b739f3fb41955
Reviewed-on: https://go-review.googlesource.com/8207
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

src/runtime/malloc.go

@@ -686,7 +686,7 @@ func mallocgc(size uintptr, typ *_type, flags uint32) unsafe.Pointer {
 
 	if shouldtriggergc() {
 		startGC(gcBackgroundMode)
-	} else if gcphase == _GCmark {
+	} else if gcBlackenEnabled != 0 {
 		// Assist garbage collector. We delay this until the
 		// epilogue so that it doesn't interfere with the
 		// inner working of malloc such as mcache refills that

src/runtime/mgc.go

@@ -465,10 +465,10 @@ func (c *gcControllerState) endCycle() {
 }
 
 // findRunnable returns the background mark worker for _p_ if it
-// should be run. This must only be called when gcphase == _GCmark.
+// should be run. This must only be called when gcBlackenEnabled != 0.
 func (c *gcControllerState) findRunnable(_p_ *p) *g {
-	if gcphase != _GCmark {
-		throw("gcControllerState.findRunnable: not in mark phase")
+	if gcBlackenEnabled == 0 {
+		throw("gcControllerState.findRunnable: blackening not enabled")
 	}
 	if _p_.gcBgMarkWorker == nil {
 		throw("gcControllerState.findRunnable: no background mark worker")
@@ -764,23 +764,24 @@ func gc(mode int) {
 			gcscan_m()
 			gctimer.cycle.installmarkwb = nanotime()
 
-			// Enter mark phase, enabling write barriers
-			// and mutator assists.
-			//
-			// TODO: Elimate this STW. This requires
-			// enabling write barriers in all mutators
-			// before enabling any mutator assists or
-			// background marking.
+			// Enter mark phase. This enables write
+			// barriers.
 			if debug.gctrace > 0 {
 				tInstallWB = nanotime()
 			}
-			stoptheworld()
-			gcBgMarkPrepare()
-			gcphase = _GCmark
-
-			// Concurrent mark.
-			starttheworld()
+			atomicstore(&gcphase, _GCmark)
+			// Ensure all Ps have observed the phase
+			// change and have write barriers enabled
+			// before any blackening occurs.
+			forEachP(func(*p) {})
 		})
+		// Concurrent mark.
+		gcBgMarkPrepare() // Must happen before assist enable.
+		// At this point all Ps have enabled the mark phase
+		// write barrier, thus maintaining the no white to
+		// black invariant. Mutator assists and mark workers
+		// can now be enabled to safely blacken grey objects.
+		atomicstore(&gcBlackenEnabled, 1)
 		gctimer.cycle.mark = nanotime()
 		if debug.gctrace > 0 {
 			tMark = nanotime()
@@ -824,6 +825,7 @@ func gc(mode int) {
 
 	// World is stopped.
 	// Start marktermination which includes enabling the write barrier.
+	atomicstore(&gcBlackenEnabled, 0)
 	gcphase = _GCmarktermination
 
 	if debug.gctrace > 0 {
@@ -921,7 +923,7 @@ func gc(mode int) {
 		// Update work.totaltime
 		sweepTermCpu := int64(stwprocs) * (tScan - tSweepTerm)
 		scanCpu := tInstallWB - tScan
-		installWBCpu := int64(stwprocs) * (tMark - tInstallWB)
+		installWBCpu := int64(0)
 		// We report idle marking time below, but omit it from
 		// the overall utilization here since it's "free".
 		markCpu := gcController.assistTime + gcController.dedicatedMarkTime + gcController.fractionalMarkTime
@@ -1047,12 +1049,18 @@ func gcBgMarkWorker(p *p) {
 		// dispose the gcw, and then preempt.
 		mp = acquirem()
 
+		if gcBlackenEnabled == 0 {
+			throw("gcBgMarkWorker: blackening not enabled")
+		}
+
 		startTime := nanotime()
 
 		xadd(&work.nwait, -1)
 
 		done := false
 		switch p.gcMarkWorkerMode {
+		default:
+			throw("gcBgMarkWorker: unexpected gcMarkWorkerMode")
 		case gcMarkWorkerDedicatedMode:
 			gcDrain(&p.gcw, gcBgCreditSlack)
 			// gcDrain did the xadd(&work.nwait +1) to

src/runtime/mgcmark.go

@@ -172,7 +172,7 @@ func markroot(desc *parfor, i uint32) {
 // allowAssist is true, may assist GC scanning in proportion to the
 // allocations performed by this mutator since the last assist.
 //
-// It should only be called during gcphase == _GCmark.
+// It should only be called if gcAssistAlloc != 0.
 //
 // This must be called with preemption disabled.
 //go:nowritebarrier

src/runtime/proc1.go

@@ -1441,9 +1441,10 @@ top:
 	}
 stop:
 
-	// We have nothing to do. If we're in the GC mark phase, run
-	// idle-time marking rather than give up the P.
-	if _p_ := _g_.m.p.ptr(); gcphase == _GCmark && _p_.gcBgMarkWorker != nil {
+	// We have nothing to do. If we're in the GC mark phaseand 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 {
 		_p_.gcMarkWorkerMode = gcMarkWorkerIdleMode
 		gp := _p_.gcBgMarkWorker
 		casgstatus(gp, _Gwaiting, _Grunnable)
@@ -1596,7 +1597,7 @@ top:
 			resetspinning()
 		}
 	}
-	if gp == nil && gcphase == _GCmark {
+	if gp == nil && gcBlackenEnabled != 0 {
 		gp = gcController.findRunnable(_g_.m.p.ptr())
 		if gp != nil {
 			resetspinning()

src/runtime/runtime2.go

@@ -533,6 +533,11 @@ type forcegcstate struct {
 
 var gcphase uint32
 
+// gcBlackenEnabled is 1 if mutator assists and background mark
+// workers are allowed to blacken objects. This must only be set when
+// gcphase == _GCmark.
+var gcBlackenEnabled uint32
+
 /*
  * known to compiler
  */