Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/pprof: mips/mipsle may deadlock #20146

Closed
vstefanovic opened this issue Apr 27, 2017 · 6 comments
Closed

runtime/pprof: mips/mipsle may deadlock #20146

vstefanovic opened this issue Apr 27, 2017 · 6 comments

Comments

@vstefanovic
Copy link
Member

On mips/mipsle, 64bit atomics are emulated with spinlocks, in runtime/internal/atomic. If SIGPROF arrives while the program is inside a critical section, it creates a deadlock (when writing the sample).

This happens on the buildbot occasionally, e.g.:
https://build.golang.org/log/72ddc31061eb792759a7837e40cc4e38ed47d8ab
https://build.golang.org/log/53aed14580842bdf20a4ece60bae5c0d0b60c3bc
https://build.golang.org/log/0eb6d5078bf128e497aa27d406fe573e7dd79583

*** Test killed: ran too long (13m0s).
FAIL runtime/pprof 785.041s

If nothing else, I'll send a patch that ignores SIGPROF in this case.

@cherrymui
Copy link
Member

ARM also uses spinlocks. Why doesn't it deadlock?

@vstefanovic
Copy link
Member Author

ARM has lock striping, with 57 spinlocks, so the probability for a deadlock is much lower, although not zero.

@cherrymui
Copy link
Member

Ok, this is probably true. You can send you fix. Thanks!

Is there a simple test for this? Say running atomics in a loop and profiling it?

@gopherbot
Copy link

CL https://golang.org/cl/42652 mentions this issue.

gopherbot pushed a commit that referenced this issue Jul 26, 2017
64bit atomics on mips/mipsle are implemented using spinlocks. If SIGPROF
is received while the program is in the critical section, it will try to
write the sample using the same spinlock, creating a deadloop.
Prevent it by creating a counter of SIGPROFs during atomic64 and
postpone writing the sample(s) until called from elsewhere, with
pc set to _LostSIGPROFDuringAtomic64.

Added a test case, per Cherry's suggestion. Works around #20146.

Change-Id: Icff504180bae4ee83d78b19c0d9d6a80097087f9
Reviewed-on: https://go-review.googlesource.com/42652
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@andybons
Copy link
Member

Closing since the CL appears to fix the issue. Please re-open if that's not the case.

@gopherbot
Copy link

Change https://golang.org/cl/117057 mentions this issue: runtime: fix pprof livelock on arm

gopherbot pushed a commit that referenced this issue Jun 8, 2018
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>
@golang golang locked and limited conversation to collaborators Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants