runtime: document relaxed access to arena_used

The unsynchronized accesses to mheap_.arena_used in the concurrent
part of the garbage collector look like a problem waiting to happen.
In fact, they are safe, but the reason is somewhat subtle and
undocumented. This commit documents this reasoning.

Related to issue #9984.

Change-Id: Icdbf2329c1aa11dbe2396a71eb5fc2a85bd4afd5
Reviewed-on: https://go-review.googlesource.com/11254
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

diff --git a/src/runtime/mbarrier.go b/src/runtime/mbarrier.go
index 95ee2ab672df061922b591768a423eb6d284205f..b83955b112489a7041688eedf4372373bf4da6ce 100644 (file)
--- a/src/runtime/mbarrier.go
+++ b/src/runtime/mbarrier.go
@@ -58,6 +58,19 @@ import "unsafe"
 // barriers, which will slow down both the mutator and the GC, we always grey
 // the ptr object regardless of the slot's color.
 //
+// Another place where we intentionally omit memory barriers is when
+// accessing mheap_.arena_used to check if a pointer points into the
+// heap. On relaxed memory machines, it's possible for a mutator to
+// extend the size of the heap by updating arena_used, allocate an
+// object from this new region, and publish a pointer to that object,
+// but for tracing running on another processor to observe the pointer
+// but use the old value of arena_used. In this case, tracing will not
+// mark the object, even though it's reachable. However, the mutator
+// is guaranteed to execute a write barrier when it publishes the
+// pointer, so it will take care of marking the object. A general
+// consequence of this is that the garbage collector may cache the
+// value of mheap_.arena_used. (See issue #9984.)
+//
 //
 // Stack writes:
 //

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 57dc2560ddace912e5883a59093ef7a207cfe13a..c7d175b1f8bbea3533125c150517eae4e9926f9f 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -768,6 +768,15 @@ func scanblock(b0, n0 uintptr, ptrmask *uint8, gcw *gcWork) {
 // object (it ignores n).
 //go:nowritebarrier
 func scanobject(b uintptr, gcw *gcWork) {
+       // Note that arena_used may change concurrently during
+       // scanobject and hence scanobject may encounter a pointer to
+       // a newly allocated heap object that is *not* in
+       // [start,used). It will not mark this object; however, we
+       // know that it was just installed by a mutator, which means
+       // that mutator will execute a write barrier and take care of
+       // marking it. This is even more pronounced on relaxed memory
+       // architectures since we access arena_used without barriers
+       // or synchronization, but the same logic applies.
        arena_start := mheap_.arena_start
        arena_used := mheap_.arena_used