diff options
| author | Michael Anthony Knyszek <mknyszek@google.com> | 2019-10-25 18:23:35 +0000 |
|---|---|---|
| committer | Michael Knyszek <mknyszek@google.com> | 2019-11-01 21:32:33 +0000 |
| commit | 08a72c851c782a5a0190c1647283432a1bb09889 (patch) | |
| tree | 4c91817c99e21dda84640b2696ab7f96b0b04027 /src/runtime/mgcscavenge.go | |
| parent | d0160bc32defcb054b59b8501bfadc8ec5b64073 (diff) | |
| download | go-08a72c851c782a5a0190c1647283432a1bb09889.tar.gz go-08a72c851c782a5a0190c1647283432a1bb09889.zip | |
runtime: turn off scavenger when there's <1 physical page of work
This change turns off the scavenger if there's less than one physical
page of work to do. If there's less than one phyiscal page of work
today, then the computed time for the work to be done will be zero,
resulting in a floating point division by zero.
This is bad on two accounts. On the one hand it could cause a fault on
some systems. On the other hand, it could cause the pacing computations
done by the scavenger to be nonsense. While this is generally harmless
in the case where there's a very small amount of work to do anyway (the
scavenger might just back off expontentially forever, or do some work
and immediately sleep, because there's not much of it to do), it causes
problems for the deadlock checker. On platforms with a larger physical
page size, such as 64 KiB, we might hit this path in a deadlock
scenario, in which case the deadlock checker will never fire and we'll
just hang.
Specifically, this happens on ppc64 trybot tests, which is where the
issue was discovered.
Fixes #34575.
Change-Id: I8677db539447b2f0e75b8cfcbe33932244e1508c
Reviewed-on: https://go-review.googlesource.com/c/go/+/203517
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Diffstat (limited to 'src/runtime/mgcscavenge.go')
| -rw-r--r-- | src/runtime/mgcscavenge.go | 10 |
1 files changed, 7 insertions, 3 deletions
diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 0a67f74150..d17da7ebb4 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -136,9 +136,13 @@ func gcPaceScavenger() { // physical page. retainedNow := heapRetained() - // If we're already below our goal, publish the goal in case it changed - // then disable the background scavenger. - if retainedNow <= retainedGoal { + // If we're already below our goal or there's less the one physical page + // worth of work to do, publish the goal in case it changed then disable + // the background scavenger. We disable the background scavenger if there's + // less than one physical page of work to do to avoid a potential divide-by-zero + // in the calculations below (totalTime will be zero), and it's not worth + // turning on the scavenger for less than one page of work. + if retainedNow <= retainedGoal || retainedNow-retainedGoal < uint64(physPageSize) { mheap_.scavengeRetainedGoal = retainedGoal mheap_.scavengeBytesPerNS = 0 return |