]> Cypherpunks.ru repositories - gostls13.git/commit
runtime: fix pprof cpu profile corruption on arm/mips/mipsle
authorRuss Cox <rsc@golang.org>
Fri, 28 Jun 2019 05:42:08 +0000 (01:42 -0400)
committerRuss Cox <rsc@golang.org>
Fri, 28 Jun 2019 20:09:48 +0000 (20:09 +0000)
commit91c385b3936e757e4cda01c9265de3b4abf601c3
treebe4eda2d59f107bd0a548fa63ce748a63c7f5cd3
parent3b040b7e8088ad2c02e413a4abf7effcd62373d0
runtime: fix pprof cpu profile corruption on arm/mips/mipsle

CL 42652 changed the profile handler for mips/mipsle to
avoid recording a profile when in atomic functions, for fear
of interrupting the 32-bit simulation of a 64-bit atomic with
a lock. The profile logger itself uses 64-bit atomics and might
deadlock (#20146).

The change was to accumulate a count of dropped profile events
and then send the count when the next ordinary event was sent:

if prof.hz != 0 {
+ if (GOARCH == "mips" || GOARCH == "mipsle") && lostAtomic64Count > 0 {
+ cpuprof.addLostAtomic64(lostAtomic64Count)
+ lostAtomic64Count = 0
+ }
  cpuprof.add(gp, stk[:n])
  }

CL 117057 extended this behavior to include GOARCH == "arm".

Unfortunately, the inserted cpuprof.addLostAtomic64 differs from
the original cpuprof.add in that it neglects to acquire the lock
protecting the profile buffer.

This has caused a steady stream of flakes on the arm builders
for the past 12 months, ever since CL 117057 landed.

This CL moves the lostAtomic count into the profile buffer and
then lets the existing addExtra calls take care of it, instead of
duplicating the locking logic.

Fixes #24991.

Change-Id: Ia386c40034fcf46b31f080ce18f2420df4bb8004
Reviewed-on: https://go-review.googlesource.com/c/go/+/184164
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/runtime/cpuprof.go
src/runtime/proc.go