Commit afae876b authored by philhofer's avatar philhofer Committed by Cherry Zhang

runtime: fix pprof livelock on arm

On 32-bit architectures without native 64-bit atomic instructions,
64-bit atomics are emulated using spinlocks. However,
the sigprof handling code expects to be able to perform
64-bit atomic operations in signal handlers. Spinning on an
acquired spinlock in a signal handler leads to a livelock.
This is issue #20146.

The original fix for #20146 did not include arm in
the list of architectures that need to work around portability
issues in the sigprof handler code. The unit test designed to
catch this issue does not fail on arm builds because arm uses
striped spinlocks, and thus the livelock takes many minutes
to reproduce. This is issue #24260. (This patch doesn't completely
fix #24260 on go1.10.2 due to issue #25785, which is probably
related to the arm cas kernel helpers. Those have been removed
at tip.)

With this patch applied, I was able to run the reproducer for
issue #24260 for more than 90 minutes without reproducing the
livelock. Without this patch, the livelock took as little as
8 minutes to reproduce.

Fixes #20146
Updates #24260

Change-Id: I64bf53a14d53c4932367d919ac55e17c99d87484
Reviewed-on: https://go-review.googlesource.com/117057
Run-TryBot: Philip Hofer <phofer@umich.edu>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: 's avatarCherry Zhang <cherryyz@google.com>
parent abeac091
...@@ -3699,7 +3699,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) { ...@@ -3699,7 +3699,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
// As a workaround, create a counter of SIGPROFs while in critical section // As a workaround, create a counter of SIGPROFs while in critical section
// to store the count, and pass it to sigprof.add() later when SIGPROF is // to store the count, and pass it to sigprof.add() later when SIGPROF is
// received from somewhere else (with _LostSIGPROFDuringAtomic64 as pc). // received from somewhere else (with _LostSIGPROFDuringAtomic64 as pc).
if GOARCH == "mips" || GOARCH == "mipsle" { if GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "arm" {
if f := findfunc(pc); f.valid() { if f := findfunc(pc); f.valid() {
if hasprefix(funcname(f), "runtime/internal/atomic") { if hasprefix(funcname(f), "runtime/internal/atomic") {
lostAtomic64Count++ lostAtomic64Count++
...@@ -3839,7 +3839,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) { ...@@ -3839,7 +3839,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
} }
if prof.hz != 0 { if prof.hz != 0 {
if (GOARCH == "mips" || GOARCH == "mipsle") && lostAtomic64Count > 0 { if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "arm") && lostAtomic64Count > 0 {
cpuprof.addLostAtomic64(lostAtomic64Count) cpuprof.addLostAtomic64(lostAtomic64Count)
lostAtomic64Count = 0 lostAtomic64Count = 0
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment