From 41dbcc19ef09a15f464eb3931c60b04e33cf72bb Mon Sep 17 00:00:00 2001 From: Rick Hudson Date: Thu, 12 Mar 2015 14:19:21 -0400 Subject: [PATCH] runtime: Remove write barriers during STW. The GC assumes that there will be no asynchronous write barriers when the world is stopped. This keeps the synchronization between write barriers and the GC simple. However, currently, there are a few places in runtime code where this assumption does not hold. The GC stops the world by collecting all Ps, which stops all user Go code, but small parts of the runtime can run without a P. For example, the code that releases a P must still deschedule its G onto a runnable queue before stopping. Similarly, when a G returns from a long-running syscall, it must run code to reacquire a P. Currently, this code can contain write barriers. This can lead to the GC collecting reachable objects if something like the following sequence of events happens: 1. GC stops the world by collecting all Ps. 2. G #1 returns from a syscall (for example), tries to install a pointer to object X, and calls greyobject on X. 3. greyobject on G #1 marks X, but does not yet add it to a write buffer. At this point, X is effectively black, not grey, even though it may point to white objects. 4. GC reaches X through some other path and calls greyobject on X, but greyobject does nothing because X is already marked. 5. GC completes. 6. greyobject on G #1 adds X to a work buffer, but it's too late. 7. Objects that were reachable only through X are incorrectly collected. To fix this, we check the invariant that no asynchronous write barriers happen when the world is stopped by checking that write barriers always have a P, and modify all currently known sources of these writes to disable the write barrier. In all modified cases this is safe because the object in question will always be reachable via some other path. Some of the trace code was turned off, in particular the code that traces returning from a syscall. The GC assumes that as far as the heap is concerned the thread is stopped when it is in a syscall. Upon returning the trace code must not do any heap writes for the same reasons discussed above. Fixes #10098 Fixes #9953 Fixes #9951 Fixes #9884 May relate to #9610 #9771 Change-Id: Ic2e70b7caffa053e56156838eb8d89503e3c0c8a Reviewed-on: https://go-review.googlesource.com/7504 Reviewed-by: Austin Clements --- src/runtime/mbarrier.go | 18 +++++++++- src/runtime/netpoll.go | 16 +++++---- src/runtime/proc1.go | 76 ++++++++++++++++++++++++++++++----------- src/runtime/runtime2.go | 46 +++++++++++++++++++++---- 4 files changed, 122 insertions(+), 34 deletions(-) diff --git a/src/runtime/mbarrier.go b/src/runtime/mbarrier.go index 1f9bd95cae7..a64bd12815b 100644 --- a/src/runtime/mbarrier.go +++ b/src/runtime/mbarrier.go @@ -83,6 +83,19 @@ func needwb() bool { return gcphase == _GCmark || gcphase == _GCmarktermination || mheap_.shadow_enabled } +// Write barrier calls must not happen during critical GC and scheduler +// related operations. In particular there are times when the GC assumes +// that the world is stopped but scheduler related code is still being +// executed, dealing with syscalls, dealing with putting gs on runnable +// queues and so forth. This code can not execute write barriers because +// the GC might drop them on the floor. Stopping the world involves removing +// the p associated with an m. We use the fact that m.p == nil to indicate +// that we are in one these critical section and throw if the write is of +// a pointer to a heap object. +// The p, m, and g pointers are the pointers that are used by the scheduler +// and need to be operated on without write barriers. We use +// the setPNoWriteBarrier, setMNoWriteBarrier and setGNowriteBarrier to +// avoid having to do the write barrier. //go:nosplit func writebarrierptr_nostore1(dst *uintptr, src uintptr) { mp := acquirem() @@ -90,8 +103,11 @@ func writebarrierptr_nostore1(dst *uintptr, src uintptr) { releasem(mp) return } - mp.inwb = true systemstack(func() { + if mp.p == nil && memstats.enablegc && !mp.inwb && inheap(src) { + throw("writebarrierptr_nostore1 called with mp.p == nil") + } + mp.inwb = true gcmarkwb_m(dst, src) }) mp.inwb = false diff --git a/src/runtime/netpoll.go b/src/runtime/netpoll.go index a131da85421..4791e5eebea 100644 --- a/src/runtime/netpoll.go +++ b/src/runtime/netpoll.go @@ -274,21 +274,25 @@ func net_runtime_pollUnblock(pd *pollDesc) { } // make pd ready, newly runnable goroutines (if any) are returned in rg/wg +// May run during STW, so write barriers are not allowed. +// Eliminating WB calls using setGNoWriteBarrier are safe since the gs are +// reachable through allg. +//go:nowritebarrier func netpollready(gpp **g, pd *pollDesc, mode int32) { var rg, wg *g if mode == 'r' || mode == 'r'+'w' { - rg = netpollunblock(pd, 'r', true) + setGNoWriteBarrier(&rg, netpollunblock(pd, 'r', true)) } if mode == 'w' || mode == 'r'+'w' { - wg = netpollunblock(pd, 'w', true) + setGNoWriteBarrier(&wg, netpollunblock(pd, 'w', true)) } if rg != nil { - rg.schedlink = *gpp - *gpp = rg + setGNoWriteBarrier(&rg.schedlink, *gpp) + setGNoWriteBarrier(gpp, rg) } if wg != nil { - wg.schedlink = *gpp - *gpp = wg + setGNoWriteBarrier(&wg.schedlink, *gpp) + setGNoWriteBarrier(gpp, wg) } } diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go index 5e5d0efa0b2..0ff3bafbdb8 100644 --- a/src/runtime/proc1.go +++ b/src/runtime/proc1.go @@ -720,7 +720,7 @@ func mstart1() { initsig() } - if _g_.m.mstartfn != nil { + if _g_.m.mstartfn != 0 { fn := *(*func())(unsafe.Pointer(&_g_.m.mstartfn)) fn() } @@ -971,17 +971,22 @@ func unlockextra(mp *m) { } // Create a new m. It will start off with a call to fn, or else the scheduler. +// fn needs to be static and not a heap allocated closure. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func newm(fn func(), _p_ *p) { mp := allocm(_p_) - mp.nextp = _p_ - mp.mstartfn = *(*unsafe.Pointer)(unsafe.Pointer(&fn)) - + // procresize made _p_ reachable through allp, which doesn't change during GC, so WB can be eliminated + setPNoWriteBarrier(&mp.nextp, _p_) + // Store &fn as a uintptr since it is not heap allocated so the WB can be eliminated + mp.mstartfn = *(*uintptr)(unsafe.Pointer(&fn)) if iscgo { var ts cgothreadstart if _cgo_thread_start == nil { throw("_cgo_thread_start missing") } - ts.g = mp.g0 + // mp is reachable via allm and mp.g0 never changes, so WB can be eliminated. + setGNoWriteBarrier(&ts.g, mp.g0) ts.tls = (*uint64)(unsafe.Pointer(&mp.tls[0])) ts.fn = unsafe.Pointer(funcPC(mstart)) asmcgocall(_cgo_thread_start, unsafe.Pointer(&ts)) @@ -1029,6 +1034,8 @@ func mspinning() { // Schedules some M to run the p (creates an M if necessary). // If p==nil, tries to get an idle P, if no idle P's does nothing. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func startm(_p_ *p, spinning bool) { lock(&sched.lock) if _p_ == nil { @@ -1058,7 +1065,8 @@ func startm(_p_ *p, spinning bool) { throw("startm: m has p") } mp.spinning = spinning - mp.nextp = _p_ + // procresize made _p_ reachable through allp, which doesn't change during GC, so WB can be eliminated + setPNoWriteBarrier(&mp.nextp, _p_) notewakeup(&mp.park) } @@ -1139,6 +1147,8 @@ func stoplockedm() { } // Schedules the locked m to run the locked gp. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func startlockedm(gp *g) { _g_ := getg() @@ -1152,7 +1162,8 @@ func startlockedm(gp *g) { // directly handoff current P to the locked m incidlelocked(-1) _p_ := releasep() - mp.nextp = _p_ + // procresize made _p_ reachable through allp, which doesn't change during GC, so WB can be eliminated + setPNoWriteBarrier(&mp.nextp, _p_) notewakeup(&mp.park) stopm() } @@ -1805,7 +1816,11 @@ func exitsyscall(dummy int32) { for oldp != nil && oldp.syscalltick == _g_.m.syscalltick { osyield() } - systemstack(traceGoSysExit) + // This can't be done since the GC may be running and this code + // will invoke write barriers. + // TODO: Figure out how to get traceGoSysExit into the trace log or + // it is likely not to work as expected. + // systemstack(traceGoSysExit) } _g_.m.locks-- @@ -2569,6 +2584,8 @@ func procresize(nprocs int32) *p { } // Associate p and the current m. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func acquirep(_p_ *p) { _g_ := getg() @@ -2583,9 +2600,12 @@ func acquirep(_p_ *p) { print("acquirep: p->m=", _p_.m, "(", id, ") p->status=", _p_.status, "\n") throw("acquirep: invalid p state") } - _g_.m.mcache = _p_.mcache - _g_.m.p = _p_ - _p_.m = _g_.m + // _p_.mcache holds the mcache and _p_ is in allp, so WB can be eliminated + setMcacheNoWriteBarrier(&_g_.m.mcache, _p_.mcache) + // _p_ is in allp so WB can be eliminated + setPNoWriteBarrier(&_g_.m.p, _p_) + // m is in _g_.m and is reachable through allg, so WB can be eliminated + setMNoWriteBarrier(&_p_.m, _g_.m) _p_.status = _Prunning if trace.enabled { @@ -2991,19 +3011,26 @@ func schedtrace(detailed bool) { // Put mp on midle list. // Sched must be locked. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func mput(mp *m) { - mp.schedlink = sched.midle - sched.midle = mp + // sched.midle is reachable via allm, so WB can be eliminated. + setMNoWriteBarrier(&mp.schedlink, sched.midle) + // mp is reachable via allm, so WB can be eliminated. + setMNoWriteBarrier(&sched.midle, mp) sched.nmidle++ checkdead() } // Try to get an m from midle list. // Sched must be locked. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func mget() *m { mp := sched.midle if mp != nil { - sched.midle = mp.schedlink + // mp.schedlink is reachable via mp, which is on allm, so WB can be eliminated. + setMNoWriteBarrier(&sched.midle, mp.schedlink) sched.nmidle-- } return mp @@ -3011,14 +3038,17 @@ func mget() *m { // Put gp on the global runnable queue. // Sched must be locked. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func globrunqput(gp *g) { gp.schedlink = nil if sched.runqtail != nil { - sched.runqtail.schedlink = gp + // gp is on allg, so these three WBs can be eliminated. + setGNoWriteBarrier(&sched.runqtail.schedlink, gp) } else { - sched.runqhead = gp + setGNoWriteBarrier(&sched.runqhead, gp) } - sched.runqtail = gp + setGNoWriteBarrier(&sched.runqtail, gp) sched.runqsize++ } @@ -3071,18 +3101,24 @@ func globrunqget(_p_ *p, max int32) *g { // Put p to on _Pidle list. // Sched must be locked. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func pidleput(_p_ *p) { - _p_.link = sched.pidle - sched.pidle = _p_ + // sched.pidle, _p_.link and _p_ are reachable via allp, so WB can be eliminated. + setPNoWriteBarrier(&_p_.link, sched.pidle) + setPNoWriteBarrier(&sched.pidle, _p_) xadd(&sched.npidle, 1) // TODO: fast atomic } // Try get a p from _Pidle list. // Sched must be locked. +// May run during STW, so write barriers are not allowed. +//go:nowritebarrier func pidleget() *p { _p_ := sched.pidle if _p_ != nil { - sched.pidle = _p_.link + // _p_.link is reachable via a _p_ in allp, so WB can be eliminated. + setPNoWriteBarrier(&sched.pidle, _p_.link) xadd(&sched.npidle, -1) // TODO: fast atomic } return _p_ diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 13c5dee0781..27700b6217c 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -117,6 +117,38 @@ func (gp guintptr) ptr() *g { return (*g)(unsafe.Pointer(gp)) } +// ps, ms, gs, and mcache are structures that must be manipulated at a level +// lower than that of the normal Go language. For example the routine that +// stops the world removes the p from the m structure informing the GC that +// this P is stopped and then it moves the g to the global runnable queue. +// If write barriers were allowed to happen at this point not only does +// the GC think the thread is stopped but the underlying structures +// like a p or m are not in a state that is not coherent enough to +// support the write barrier actions. +// This is particularly painful since a partially executed write barrier +// may mark the object but be delinquent in informing the GC that the +// object needs to be scanned. + +// setGNoWriteBarriers does *gdst = gval without a write barrier. +func setGNoWriteBarrier(gdst **g, gval *g) { + *(*uintptr)(unsafe.Pointer(gdst)) = uintptr(unsafe.Pointer(gval)) +} + +// setMNoWriteBarriers does *mdst = mval without a write barrier. +func setMNoWriteBarrier(mdst **m, mval *m) { + *(*uintptr)(unsafe.Pointer(mdst)) = uintptr(unsafe.Pointer(mval)) +} + +// setPNoWriteBarriers does *pdst = pval without a write barrier. +func setPNoWriteBarrier(pdst **p, pval *p) { + *(*uintptr)(unsafe.Pointer(pdst)) = uintptr(unsafe.Pointer(pval)) +} + +// setMcacheNoWriteBarriers does *mcachedst = mcacheval without a write barrier. +func setMcacheNoWriteBarrier(mcachedst **mcache, mcacheval *mcache) { + *(*uintptr)(unsafe.Pointer(mcachedst)) = uintptr(unsafe.Pointer(mcacheval)) +} + type gobuf struct { // The offsets of sp, pc, and g are known to (hard-coded in) libmach. sp uintptr @@ -233,13 +265,13 @@ type m struct { morebuf gobuf // gobuf arg to morestack // Fields not known to debuggers. - procid uint64 // for debuggers, but offset not hard-coded - gsignal *g // signal-handling g - tls [4]uintptr // thread-local storage (for x86 extern register) - mstartfn unsafe.Pointer // todo go func() - curg *g // current running goroutine - caughtsig *g // goroutine running during fatal signal - p *p // attached p for executing go code (nil if not executing go code) + procid uint64 // for debuggers, but offset not hard-coded + gsignal *g // signal-handling g + tls [4]uintptr // thread-local storage (for x86 extern register) + mstartfn uintptr // TODO: type as func(); note: this is a non-heap allocated func() + curg *g // current running goroutine + caughtsig *g // goroutine running during fatal signal + p *p // attached p for executing go code (nil if not executing go code) nextp *p id int32 mallocing int32