From 60586034713cc94477868fb6911f34cfcc6a1ba4 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 27 Sep 2019 14:34:05 -0400 Subject: runtime: only shrink stacks at synchronous safe points We're about to introduce asynchronous safe points, where we won't have precise pointer maps for all stack frames. That's okay for scanning the stack (conservatively), but not for shrinking the stack. Hence, this CL prepares for this by only shrinking the stack as part of the stack scan if the goroutine is stopped at a synchronous safe point. Otherwise, it queues up the stack shrink for the next synchronous safe point. We already have one condition under which we can't shrink the stack for very similar reasons: syscalls. Currently, we just give up on shrinking the stack if it's in a syscall. But with this mechanism, we defer that stack shrink until the next synchronous safe point. For #10958, #24543. Change-Id: Ifa1dec6f33fdf30f9067be2ce3f7ab8a7f62ce38 Reviewed-on: https://go-review.googlesource.com/c/go/+/201438 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Cherry Zhang --- src/runtime/stack.go | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) (limited to 'src/runtime/stack.go') diff --git a/src/runtime/stack.go b/src/runtime/stack.go index e47f12a8dc..825826cacd 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -1010,6 +1010,13 @@ func newstack() { throw("runtime: g is running but p is not") } + if gp.preemptShrink { + // We're at a synchronous safe point now, so + // do the pending stack shrink. + gp.preemptShrink = false + shrinkstack(gp) + } + if gp.preemptStop { preemptPark(gp) // never returns } @@ -1057,16 +1064,36 @@ func gostartcallfn(gobuf *gobuf, fv *funcval) { gostartcall(gobuf, fn, unsafe.Pointer(fv)) } +// isShrinkStackSafe returns whether it's safe to attempt to shrink +// gp's stack. Shrinking the stack is only safe when we have precise +// pointer maps for all frames on the stack. +func isShrinkStackSafe(gp *g) bool { + // We can't copy the stack if we're in a syscall. + // The syscall might have pointers into the stack and + // often we don't have precise pointer maps for the innermost + // frames. + return gp.syscallsp == 0 +} + // Maybe shrink the stack being used by gp. -// Called at garbage collection time. -// gp must be stopped, but the world need not be. +// +// gp must be stopped and we must own its stack. It may be in +// _Grunning, but only if this is our own user G. func shrinkstack(gp *g) { - gstatus := readgstatus(gp) if gp.stack.lo == 0 { throw("missing stack in shrinkstack") } - if gstatus&_Gscan == 0 { - throw("bad status in shrinkstack") + if s := readgstatus(gp); s&_Gscan == 0 { + // We don't own the stack via _Gscan. We could still + // own it if this is our own user G and we're on the + // system stack. + if !(gp == getg().m.curg && getg() != getg().m.curg && s == _Grunning) { + // We don't own the stack. + throw("bad status in shrinkstack") + } + } + if !isShrinkStackSafe(gp) { + throw("shrinkstack at bad time") } // Check for self-shrinks while in a libcall. These may have // pointers into the stack disguised as uintptrs, but these @@ -1102,12 +1129,6 @@ func shrinkstack(gp *g) { return } - // We can't copy the stack if we're in a syscall. - // The syscall might have pointers into the stack. - if gp.syscallsp != 0 { - return - } - if stackDebug > 0 { print("shrinking stack ", oldsize, "->", newsize, "\n") } -- cgit v1.3-5-g9baa