runtime: load gcControllerState.scanWork atomically in revise

gcControllerState.scanWork's docs state that it must be accessed
atomically during a GC cycle, but gcControllerState.revise does not do
this (even when called with the heap lock held).

This change makes it so that gcControllerState.revise accesses scanWork
atomically and explicitly.

Note that we don't update gcControllerState.revise's erroneous doc
comment here because this change isn't about revise's guarantees, just
about heap_scan. The comment is updated in a later change.

Change-Id: Iafc3ad214e517190bfd8a219896d23da19f7659d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246961
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 94539dd7704c1972a98e4f5d38fbdc113bcb7e63..4b9a6da3b33fb15dca828a22b9c95274c7437eac 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -495,6 +495,7 @@ func (c *gcControllerState) revise() {
        }
        live := atomic.Load64(&memstats.heap_live)
        scan := atomic.Load64(&memstats.heap_scan)
+       work := atomic.Loadint64(&c.scanWork)
 
        // Assume we're under the soft goal. Pace GC to complete at
        // next_gc assuming the heap is in steady-state.
@@ -511,7 +512,7 @@ func (c *gcControllerState) revise() {
        // 100*heap_scan.)
        scanWorkExpected := int64(float64(scan) * 100 / float64(100+gcpercent))
 
-       if live > memstats.next_gc || c.scanWork > scanWorkExpected {
+       if live > memstats.next_gc || work > scanWorkExpected {
                // We're past the soft goal, or we've already done more scan
                // work than we expected. Pace GC so that in the worst case it
                // will complete by the hard goal.
@@ -529,7 +530,7 @@ func (c *gcControllerState) revise() {
        // (scanWork), so allocation will change this difference
        // slowly in the soft regime and not at all in the hard
        // regime.
-       scanWorkRemaining := scanWorkExpected - c.scanWork
+       scanWorkRemaining := scanWorkExpected - work
        if scanWorkRemaining < 1000 {
                // We set a somewhat arbitrary lower bound on
                // remaining scan work since if we aim a little high,