Skip to content

Commit

Permalink
runtime: fix pprof livelock on arm
Browse files Browse the repository at this point in the history
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: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
  • Loading branch information
philhofer authored and cherrymui committed Jun 8, 2018
1 parent abeac09 commit afae876
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/runtime/proc.go
Expand Up @@ -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
// to store the count, and pass it to sigprof.add() later when SIGPROF is
// 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 hasprefix(funcname(f), "runtime/internal/atomic") {
lostAtomic64Count++
Expand Down Expand Up @@ -3839,7 +3839,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
}

if prof.hz != 0 {
if (GOARCH == "mips" || GOARCH == "mipsle") && lostAtomic64Count > 0 {
if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "arm") && lostAtomic64Count > 0 {
cpuprof.addLostAtomic64(lostAtomic64Count)
lostAtomic64Count = 0
}
Expand Down

0 comments on commit afae876

Please sign in to comment.