From 8f81dfe8b47e975b90bb4a2f8dd314d32c633176 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 22 Aug 2016 16:02:54 -0400 Subject: runtime: perform write barrier before pointer write Currently, we perform write barriers after performing pointer writes. At the moment, it simply doesn't matter what order this happens in, as long as they appear atomic to GC. But both the hybrid barrier and ROC are going to require a pre-write write barrier. For the hybrid barrier, this is important because the barrier needs to observe both the current value of the slot and the value that will be written to it. (Alternatively, the caller could do the write and pass in the old value, but it seems easier and more useful to just swap the order of the barrier and the write.) For ROC, this is necessary because, if the pointer write is going to make the pointer reachable to some goroutine that it currently is not visible to, the garbage collector must take some special action before that pointer becomes more broadly visible. This commits swaps pointer writes around so the write barrier occurs before the pointer write. The main subtlety here is bulk memory writes. Currently, these copy to the destination first and then use the pointer bitmap of the destination to find the copied pointers and invoke the write barrier. This is necessary because the source may not have a pointer bitmap. To handle these, we pass both the source and the destination to the bulk memory barrier, which uses the pointer bitmap of the destination, but reads the pointer values from the source. Updates #17503. Change-Id: I78ecc0c5c94ee81c29019c305b3d232069294a55 Reviewed-on: https://go-review.googlesource.com/31763 Reviewed-by: Rick Hudson --- src/runtime/atomic_pointer.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'src/runtime/atomic_pointer.go') diff --git a/src/runtime/atomic_pointer.go b/src/runtime/atomic_pointer.go index 4fe334014d..292b3517ad 100644 --- a/src/runtime/atomic_pointer.go +++ b/src/runtime/atomic_pointer.go @@ -20,17 +20,17 @@ import ( // //go:nosplit func atomicstorep(ptr unsafe.Pointer, new unsafe.Pointer) { + writebarrierptr_prewrite((*uintptr)(ptr), uintptr(new)) atomic.StorepNoWB(noescape(ptr), new) - writebarrierptr_nostore((*uintptr)(ptr), uintptr(new)) } //go:nosplit func casp(ptr *unsafe.Pointer, old, new unsafe.Pointer) bool { - if !atomic.Casp1((*unsafe.Pointer)(noescape(unsafe.Pointer(ptr))), noescape(old), new) { - return false - } - writebarrierptr_nostore((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) - return true + // The write barrier is only necessary if the CAS succeeds, + // but since it needs to happen before the write becomes + // public, we have to do it conservatively all the time. + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + return atomic.Casp1((*unsafe.Pointer)(noescape(unsafe.Pointer(ptr))), noescape(old), new) } // Like above, but implement in terms of sync/atomic's uintptr operations. @@ -43,8 +43,8 @@ func sync_atomic_StoreUintptr(ptr *uintptr, new uintptr) //go:linkname sync_atomic_StorePointer sync/atomic.StorePointer //go:nosplit func sync_atomic_StorePointer(ptr *unsafe.Pointer, new unsafe.Pointer) { + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) sync_atomic_StoreUintptr((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) - writebarrierptr_nostore((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) } //go:linkname sync_atomic_SwapUintptr sync/atomic.SwapUintptr @@ -53,8 +53,8 @@ func sync_atomic_SwapUintptr(ptr *uintptr, new uintptr) uintptr //go:linkname sync_atomic_SwapPointer sync/atomic.SwapPointer //go:nosplit func sync_atomic_SwapPointer(ptr *unsafe.Pointer, new unsafe.Pointer) unsafe.Pointer { + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) old := unsafe.Pointer(sync_atomic_SwapUintptr((*uintptr)(noescape(unsafe.Pointer(ptr))), uintptr(new))) - writebarrierptr_nostore((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) return old } @@ -64,9 +64,6 @@ func sync_atomic_CompareAndSwapUintptr(ptr *uintptr, old, new uintptr) bool //go:linkname sync_atomic_CompareAndSwapPointer sync/atomic.CompareAndSwapPointer //go:nosplit func sync_atomic_CompareAndSwapPointer(ptr *unsafe.Pointer, old, new unsafe.Pointer) bool { - if !sync_atomic_CompareAndSwapUintptr((*uintptr)(noescape(unsafe.Pointer(ptr))), uintptr(old), uintptr(new)) { - return false - } - writebarrierptr_nostore((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) - return true + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + return sync_atomic_CompareAndSwapUintptr((*uintptr)(noescape(unsafe.Pointer(ptr))), uintptr(old), uintptr(new)) } -- cgit v1.3