runtime: fix pagesInUse accounting

When we grow the heap, we create a temporary "in use" span for the
memory acquired from the OS and then free that span to link it into
the heap. Hence, we (1) increase pagesInUse when we make the temporary
span so that (2) freeing the span will correctly decrease it.

However, currently step (1) increases pagesInUse by the number of
pages requested from the heap, while step (2) decreases it by the
number of pages requested from the OS (the size of the temporary
span). These aren't necessarily the same, since we round up the number
of pages we request from the OS, so steps 1 and 2 don't necessarily
cancel out like they're supposed to. Over time, this can add up and
cause pagesInUse to underflow and wrap around to 2^64. The garbage
collector computes the sweep ratio from this, so if this happens, the
sweep ratio becomes effectively infinite, causing the first allocation
on each P in a sweep cycle to sweep the entire heap. This makes
sweeping effectively STW.

Fix this by increasing pagesInUse in step 1 by the number of pages
requested from the OS, so that the two steps correctly cancel out. We
add a test that checks that the running total matches the actual state
of the heap.

Fixes #15022. For 1.6.x.

Change-Id: Iefd9d6abe37d0d447cbdbdf9941662e4f18eeffc
Reviewed-on: https://go-review.googlesource.com/21280
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index 3994d5caf8cff76ef488ad7c8dc04f92d439be2e..fd33c9c3c8762f3940bd3204770e1b8f96799917 100644 (file)
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -196,3 +196,19 @@ func SetTracebackEnv(level string) {
 
 var ReadUnaligned32 = readUnaligned32
 var ReadUnaligned64 = readUnaligned64
+
+func CountPagesInUse() (pagesInUse, counted uintptr) {
+       stopTheWorld("CountPagesInUse")
+
+       pagesInUse = uintptr(mheap_.pagesInUse)
+
+       for _, s := range h_allspans {
+               if s.state == mSpanInUse {
+                       counted += s.npages
+               }
+       }
+
+       startTheWorld()
+
+       return
+}

diff --git a/src/runtime/gc_test.go b/src/runtime/gc_test.go
index c8c96bb4ee407ed1cfb20e400118be4ddb96ba11..d53d3ee000b3ff1ce54adfce8ef58b8ff6d7a47b 100644 (file)
--- a/src/runtime/gc_test.go
+++ b/src/runtime/gc_test.go
@@ -473,3 +473,20 @@ func testIfaceEqual(x interface{}) {
                a = true
        }
 }
+
+func TestPageAccounting(t *testing.T) {
+       // Grow the heap in small increments. This used to drop the
+       // pages-in-use count below zero because of a rounding
+       // mismatch (golang.org/issue/15022).
+       const blockSize = 64 << 10
+       blocks := make([]*[blockSize]byte, (64<<20)/blockSize)
+       for i := range blocks {
+               blocks[i] = new([blockSize]byte)
+       }
+
+       // Check that the running page count matches reality.
+       pagesInUse, counted := runtime.CountPagesInUse()
+       if pagesInUse != counted {
+               t.Fatalf("mheap_.pagesInUse is %d, but direct count is %d", pagesInUse, counted)
+       }
+}

diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index 0f2f0637d2b055a280a6dd4f4d7835ce69da2970..895af9f07c2da52a767437c4f3177b4d53a9cb74 100644 (file)
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -671,7 +671,7 @@ func (h *mheap) grow(npage uintptr) bool {
        }
        atomic.Store(&s.sweepgen, h.sweepgen)
        s.state = _MSpanInUse
-       h.pagesInUse += uint64(npage)
+       h.pagesInUse += uint64(s.npages)
        h.freeSpanLocked(s, false, true, 0)
        return true
 }