-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
Comments
ARM also uses spinlocks. Why doesn't it deadlock? |
ARM has lock striping, with 57 spinlocks, so the probability for a deadlock is much lower, although not zero. |
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? |
CL https://golang.org/cl/42652 mentions this issue. |
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
Closing since the CL appears to fix the issue. Please re-open if that's not the case. |
Change https://golang.org/cl/117057 mentions this issue: |
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
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.
The text was updated successfully, but these errors were encountered: