go/src/runtime/proc.go, branch ms-fix-multipart-readform

runtime: exclude extra M's from debug.SetMaxThreads

2023-05-09T17:55:20Z

The purpose of the debug.SetMaxThreads limit is to avoid accidental fork bomb from something like millions of goroutines blocking on system calls, causing the runtime to create millions of threads. By definition we don't create threads created in C, so this isn't a problem for those threads, and we can exclude them from the limit. If C wants to create tens of thousands of threads, who are we to say no? Fixes #60004. Change-Id: I62b875890718b406abca42a9a4078391e25aa21b Reviewed-on: https://go-review.googlesource.com/c/go/+/492743 Auto-Submit: Michael Pratt TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Run-TryBot: Michael Pratt

runtime: clean up extra M API

2023-05-08T16:39:39Z

There are quite a few locations that get/put Ms from the extra M list, but the API is pretty clumsy to use. Add an easier to use getExtraM / putExtraM API. There are only two minor semantic changes: 1. dropm no longer calls setg(nil) inside the lockextra critical section. It is important that this thread no longer references the G (and in turn M) once it is published to the extra M list and another thread could acquire it. But there is no reason that needs to happen only after lockextra. 2. extraMLength (renamed from extraMCount) is no longer protected by lockextra and is instead simply an atomic (though writes are still in the critical section). The previous readers all dropped lockextra before using the value they read anyway. For #60004. Change-Id: Ifca4d6c84d605423855d89f49af400ca07de56f4 Reviewed-on: https://go-review.googlesource.com/c/go/+/492742 Run-TryBot: Michael Pratt Commit-Queue: Michael Pratt TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Auto-Submit: Michael Pratt

Revert "runtime/cgo: store M for C-created thread in pthread key"

2023-05-05T14:37:29Z

This reverts CL 485500. Reason for revert: This breaks internal tests at Google, see b/280861579 and b/280820455. Change-Id: I426278d400f7611170918fc07c524cb059b9cc55 Reviewed-on: https://go-review.googlesource.com/c/go/+/492995 Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot Run-TryBot: Chressie Himpel

runtime/trace: avoid frame pointer unwinding for events during cgocallbackg

2023-04-28T21:07:22Z

The current mp.incgocallback() logic allows for trace events to be recorded using frame pointer unwinding during cgocallbackg when they shouldn't be. Specifically, mp.incgo will be false during the reentersyscall call at the end. It's possible to crash with tracing enabled because of this, if C code which uses the frame pointer register for other purposes calls into Go. This can be seen, for example, by forcing testprogcgo/trace_unix.c to write a garbage value to RBP prior to calling into Go. We can drop the mp.incgo check, and instead conservatively avoid doing frame pointer unwinding if there is any C on the stack. This is the case if mp.ncgo > 0, or if mp.isextra is true (meaning we're coming from a thread created by C). Rename incgocallback to reflect that we're checking if there's any C on the stack. We can also move the ncgo increment in cgocall closer to where the transition to C happens, which lets us use frame pointer unwinding for the entersyscall event during the first Go-to-C call on a stack, when there isn't yet any C on the stack. Fixes #59830. Change-Id: If178a705a9d38d0d2fb19589a9e669cd982d32cd Reviewed-on: https://go-review.googlesource.com/c/go/+/488755 Reviewed-by: Michael Knyszek Reviewed-by: Dmitri Shuralyov Reviewed-by: Felix Geisendörfer Auto-Submit: Michael Knyszek Run-TryBot: Nick Ripley TryBot-Result: Gopher Robot

runtime: resolve checkdead panic by refining `startm` lock handling in caller context

2023-04-28T15:57:36Z

This change addresses a `checkdead` panic caused by a race condition between `sysmon->startm` and `checkdead` callers, due to prematurely releasing the scheduler lock. The solution involves allowing a `startm` caller to acquire the scheduler lock and call `startm` in this context. A new `lockheld` bool argument is added to `startm`, which manages all lock and unlock calls within the function. The`startIdle` function variable in `injectglist` is updated to call `startm` with the lock held, ensuring proper lock handling in this specific case. This refined lock handling resolves the observed race condition issue. Fixes #59600 Change-Id: I11663a15536c10c773fc2fde291d959099aa71be Reviewed-on: https://go-review.googlesource.com/c/go/+/487316 Reviewed-by: Dmitri Shuralyov Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Michael Pratt

runtime/cgo: store M for C-created thread in pthread key

2023-04-26T19:25:46Z

This reapplies CL 481061, with the followup fixes in CL 482975, CL 485315, and CL 485316 incorporated. CL 481061, by doujiang24 , speed up C to Go calls by binding the M to the C thread. See below for its description. CL 482975 is a followup fix to a C declaration in testprogcgo. CL 485315 is a followup fix for x_cgo_getstackbound on Illumos. CL 485316 is a followup cleanup for ppc64 assembly. [Original CL 481061 description] This reapplies CL 392854, with the followup fixes in CL 479255, CL 479915, and CL 481057 incorporated. CL 392854, by doujiang24 , speed up C to Go calls by binding the M to the C thread. See below for its description. CL 479255 is a followup fix for a small bug in ARM assembly code. CL 479915 is another followup fix to address C to Go calls after the C code uses some stack, but that CL is also buggy. CL 481057, by Michael Knyszek, is a followup fix for a memory leak bug of CL 479915. [Original CL 392854 description] In a C thread, it's necessary to acquire an extra M by using needm while invoking a Go function from C. But, needm and dropm are heavy costs due to the signal-related syscalls. So, we change to not dropm while returning back to C, which means binding the extra M to the C thread until it exits, to avoid needm and dropm on each C to Go call. Instead, we only dropm while the C thread exits, so the extra M won't leak. When invoking a Go function from C: Allocate a pthread variable using pthread_key_create, only once per shared object, and register a thread-exit-time destructor. And store the g0 of the current m into the thread-specified value of the pthread key, only once per C thread, so that the destructor will put the extra M back onto the extra M list while the C thread exits. When returning back to C: Skip dropm in cgocallback, when the pthread variable has been created, so that the extra M will be reused the next time invoke a Go function from C. This is purely a performance optimization. The old version, in which needm & dropm happen on each cgo call, is still correct too, and we have to keep the old version on systems with cgo but without pthreads, like Windows. This optimization is significant, and the specific value depends on the OS system and CPU, but in general, it can be considered as 10x faster, for a simple Go function call from a C thread. For the newly added BenchmarkCGoInCThread, some benchmark results: 1. it's 28x faster, from 3395 ns/op to 121 ns/op, in darwin OS & Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz 2. it's 6.5x faster, from 1495 ns/op to 230 ns/op, in Linux OS & Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz [CL 479915 description] Currently, when C calls into Go the first time, we grab an M using needm, which sets m.g0's stack bounds using the SP. We don't know how big the stack is, so we simply assume 32K. Previously, when the Go function returns to C, we drop the M, and the next time C calls into Go, we put a new stack bound on the g0 based on the current SP. After CL 392854, we don't drop the M, and the next time C calls into Go, we reuse the same g0, without recomputing the stack bounds. If the C code uses quite a bit of stack space before calling into Go, the SP may be well below the 32K stack bound we assumed, so the runtime thinks the g0 stack overflows. This CL makes needm get a more accurate stack bound from pthread. (In some platforms this may still be a guess as we don't know exactly where we are in the C stack), but it is probably better than simply assuming 32K. [CL 485500 description] CL 479915 passed the G to _cgo_getstackbound for direct updates to gp.stack.lo. A G can be reused on a new thread after the previous thread exited. This could trigger the C TSAN race detector because it couldn't see the synchronization in Go (lockextra) preventing the same G from being used on multiple threads at the same time. We work around this by passing the address of a stack variable to _cgo_getstackbound rather than the G. The stack is generally unique per thread, so TSAN won't see the same address from multiple threads. Even if stacks are reused across threads by pthread, C TSAN should see the synchonization in the stack allocator. A regression test is added to misc/cgo/testsanitizer. Fixes #51676. Fixes #59294. Fixes #59678. Change-Id: Ic62be31a06ee83568215e875a891df37084e08ca Reviewed-on: https://go-review.googlesource.com/c/go/+/485500 TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui Run-TryBot: Michael Pratt

runtime: add raceFiniLock to lock ranking

2023-04-24T21:37:06Z

Also preserve the PC/SP in reentersyscall when doing lock ranking. The test is TestDestructorCallbackRace with the staticlockranking experiment enabled. For #59711 Change-Id: I87ac1d121ec0d399de369666834891ab9e7d11b0 Reviewed-on: https://go-review.googlesource.com/c/go/+/487955 Run-TryBot: Ian Lance Taylor Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor

runtime: prevent double lock in checkdead by unlocking before throws

2023-04-21T20:14:08Z

This change resolves an issue where checkdead could result in a double lock when shedtrace is enabled. This fix involves adding unlocks before all throws in the checkdead function to ensure the scheduler lock is properly released. Fixes #59758 Change-Id: If3ddf9969f4582c3c88dee9b9ecc355a63958103 Reviewed-on: https://go-review.googlesource.com/c/go/+/487375 Run-TryBot: Michael Pratt TryBot-Result: Gopher Robot Reviewed-by: Michael Knyszek Auto-Submit: Michael Pratt Reviewed-by: Michael Pratt

runtime: tidy _Stack* constant naming

2023-04-21T19:29:00Z

For #59670. Change-Id: I0efa743edc08e48dc8d906803ba45e9f641369db Reviewed-on: https://go-review.googlesource.com/c/go/+/486977 Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot Auto-Submit: Austin Clements Run-TryBot: Austin Clements

internal/abi, runtime, cmd: merge funcFlag_* consts into internal/abi

2023-04-21T19:28:46Z

For #59670. Change-Id: Ie784ba4dd2701e4f455e1abde4a6bfebee4b1387 Reviewed-on: https://go-review.googlesource.com/c/go/+/485496 Reviewed-by: David Chase TryBot-Result: Gopher Robot Run-TryBot: Austin Clements Auto-Submit: Austin Clements