| Age | Commit message (Collapse) | Author |
|---|
|
Change-Id: I6a6a636c341d4ba3518be7f6806093bbdff11c88
Reviewed-on: https://go-review.googlesource.com/c/go/+/689535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
It seems to be pretty flaky. I've seen:
retained=289438024
limit=268435456
bound=285212672
Which is ~4MB over the bound.
Not sure why this tends to be darwin-specific, but we'll fix
just darwin for now.
(It isn't quite darwin-only, as it appeared in #66893.
But it is certainly worse on darwin.)
Fixes #73136
Update #66893
Change-Id: If609e909bc6c65c2663dd46b7a9bad4fd291c3da
Reviewed-on: https://go-review.googlesource.com/c/go/+/689315
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Make sure we keep the M in X21. The instruction at line 87
needs to be loading from the M, not from the gsignal.
Fixes #73979, maybe?
Change-Id: I3aba1c35454636138b305bca26e20dbc8e6b1d6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/688975
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This change fixes a bug in the accounting of sched.idleTime. In just the
case where the GC CPU limiter needs up-to-date data, sched.idleTime is
incremented in both the P-idle-time and idle-mark-work paths, but it
should only be incremented in the former case.
Fixes #74627.
Change-Id: If41b03da102d47d25bec48ff750a9da27019b71d
Reviewed-on: https://go-review.googlesource.com/c/go/+/687998
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
table
In the type descriptor's method table, it contains relative PCs of
the methods (relative to the start of the text section) stored as
32-bit offsets. On Wasm, a PC is PC_F<<16 + PC_B, where PC_F is
the function index, and PC_B is the block index. When there are
more than 65536 functions, the PC will not fit into 32-bit (and
relative to the section start doesn't help). Since there are no
more bits for the function index, and the method table always
targets the entry of a method, we put just the PC_F there, and
rewrite back to a full PC at run time when we need the PC. This
way we can have more than 65536 functions.
The func table also contains 32-bit relative PCs, and it also
always points to function entries. Do the same there, as well
as other places where we use relative text offsets.
Also add the relocation type in the relocation overflow error
message.
Also add check for function too big on Wasm. If a function has
more than 65536 blocks, PC_B will overflow and PC = PC_F<<16 + PC_B
will points to the wrong function.
Fixes #64856.
Change-Id: If9c307e9fb1641f367a5f19c39f88f455805d0bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/552835
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Following CL 567896, this is one more place we used only 16 bits
for the function index. Change it to load 32 bits.
For #64856.
Change-Id: I66a78c086e67165604053313751c097a70c50ba9
Reviewed-on: https://go-review.googlesource.com/c/go/+/609118
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Clearing the inline mark bits with memclrNoHeapPointers is slightly
better than having the compiler insert, e.g. duffzero, since it can take
advantage of wider SIMD instructions. duffzero is likely going away, but
we know things the compiler doesn't, such as the fact that this memory
is nicely aligned. In this particular case, memclrNoHeapPointers does a
better job.
For #73581.
Change-Id: I3918096929acfe6efe6f469fb089ebe04b4acff5
Reviewed-on: https://go-review.googlesource.com/c/go/+/687938
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
|
This change modifies initInlineMarkBits to only clear mark bits if the
span wasn't just freshly allocated from the OS, where we know the bits
are already zeroed. This probably doesn't make a huge difference most of
the time, but it's an easy optimization and helps rule it out as a
source of slowdown.
For #73581.
Change-Id: I78cd4d8968bb0bf6536c0a38ef9397475c39f0ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/687937
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
This is conceptually simpler, as the sweeper doesn't have to worry about
clearing them separately. It also doesn't have a use for them.
This will also be useful to avoiding unnecessary zeroing in
initInlineMarkBits at allocation time. Currently, because it's used in
both span allocation and at sweep time, we cannot blindly trust
needzero.
This change also renames mergeInlineMarkBits to moveInlineMarkBits to
make this change in semantics clearer from the name.
For #73581.
Change-Id: Ib154738a945633b7ff5b2ae27235baa310400139
Reviewed-on: https://go-review.googlesource.com/c/go/+/687936
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Currently, with Green Tea GC, we need to copy (really bitwise-or) mark
bits back into mspan.gcmarkBits, so that it can propagate to
mspan.allocBits at sweep time. This function does actually seem to make
sweeping small spans a good bit more expensive, though sweeping is still
relatively cheap. There's some low-hanging fruit here though, in that
the merge is performed one byte at a time, but this is pretty
inefficient. We can almost as easily perform this merge one word at a
time instead, which seems to make this operation about 33% faster.
For #73581.
Change-Id: I170d36e7a2193199c423dcd556cba048ebd698af
Reviewed-on: https://go-review.googlesource.com/c/go/+/687935
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
|
Expand the GOMAXPROCS documentation to include details of how defaults
are selected, as this is something that inquisitive minds will want to
know. I've added an additional warning that these details may changed.
While we are here, add a bit more structure to make it easier to find
the relevant parts of the documentation.
For #73193.
Change-Id: I6a6a636cae93237e3e3174822490d51805e70990
Reviewed-on: https://go-review.googlesource.com/c/go/+/685318
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
Change-Id: I3103325ebe29509c00b129a317b5708aece575a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/687715
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
|
|
Just like we do for race mode. They are just too slow when running
with the sanitizers.
Fixes #59448
Change-Id: I86e3e3488ec5c4c29e410955e9dc4cbc99d39b84
Reviewed-on: https://go-review.googlesource.com/c/go/+/687535
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
|
|
TestSignalDuringExec sends a SIGWINCH to the whole process group.
However, it may execute concurrently with other copies of the runtime
tests, especially through `go tool dist`, and gdb version <12.1 has a
bug in non-interactive mode where recieving a SIGWINCH causes a crash.
This change modifies SignalDuringExec in the testprog to first fork
itself into a new process group. To avoid issues with Ctrl+C and the new
process group hanging, the new process blocks on a pipe that is passed
down to it. This pipe is automatically closed when its parent exits,
which should ensure that the subprocess also exits.
Fixes #58932.
Change-Id: I3906afa28cf8b15d22ae612d071bce7f30fc3e6c
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-noswissmap,gotip-linux-amd64-longtest-aliastypeparams,gotip-linux-amd64-longtest,gotip-linux-386-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/686875
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Removes the somewhat redundant vcs.FromDir, "allowNesting" argument,
which was always enabled, and disallow multiple VCS metadata folders
being present in a single directory. This makes VCS injection attacks
much more difficult.
Also adds a GODEBUG, allowmultiplevcs, which re-enables this behavior.
Thanks to RyotaK (https://ryotak.net) of GMO Flatt Security Inc for reporting this issue.
Fixes #74380
Fixes CVE-2025-4674
Change-Id: I5787d90cdca8deb3aca6f154efb627df1e7d2789
Reviewed-on: https://go-review.googlesource.com/c/go/+/686515
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Commit-Queue: Carlos Amedee <carlos@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
Change-Id: Ide4daa5eee3fd4f3007d6ef23aa84b8916562c39
Reviewed-on: https://go-review.googlesource.com/c/go/+/684457
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
It should get the caller's SP. The current code gets the address
of the first parameter, which is one word above the caller's SP.
There is a slot for saving the LR at 0(SP) in the caller's frame.
Fixes #62086 (for s390x).
Change-Id: Ie8cbfabc8161b98658c884a32e0af72df189ea56
Reviewed-on: https://go-review.googlesource.com/c/go/+/685715
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
findRunnable takes a snapshot of allp prior to dropping the P because
afterwards procresize may mutate allp without synchronization.
procresize is careful to never mutate the contents up to cap(allp), so
findRunnable can still safely access the Ps in the slice.
Unfortunately, growing allp is problematic. If procresize grows the allp
backing array, it drops the reference to the old array. allpSnapshot
still refers to the old array, but allpSnapshot is on the system stack
in findRunnable, which also likely no longer has a P at all.
This means that a future GC will not find the reference and can free the
array and use it for another allocation. This would corrupt later reads
that findRunnable does from the array.
The fix is simple: the M struct itself is reachable by the GC, so we can
stash the snapshot in the M to ensure it is visible to the GC.
The ugliest part of the CL is the cleanup when we are done with the
snapshot because there are so many return/goto top sites. I am tempted
to put mp.clearAllpSnapshot() in the caller and at top to make this less
error prone, at the expensive of extra unnecessary writes.
Fixes #74414.
Change-Id: I6a6a636c484e4f4b34794fd07910b3fffeca830b
Reviewed-on: https://go-review.googlesource.com/c/go/+/684460
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
We encountered a new type of "no such process" error on loong64, it's like this
"Couldn't get NT_PRSTATUS registers: No such process.", I checked the source code
of gdb, NT_PRSTATUS is not fixed, it may be another name, so I use regular
expression here to match possible cases.
Updates #50838
Fixes #74389
Change-Id: I3e3f7455b2dc6b8aa10c084f24f6a2a114790855
Reviewed-on: https://go-review.googlesource.com/c/go/+/684195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
|
|
The NeedmDeadlock test program currently has a 5-second timeout,
which is sort of arbitrary. It is long enough in regular mode
(which usually takes 0.0X seconds), but not quite so for
configurations like ASAN. Instead of using an arbitrary timeout,
just use the test's deadline. The test program is invoked with
testenv.Command, which will send it a SIGQUIT before the deadline
expires.
Fixes #56420 (at least for the asan builder).
Change-Id: I0b13651cb07241401837ca2e60eaa1b83275b093
Reviewed-on: https://go-review.googlesource.com/c/go/+/684697
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
|
|
If a goroutine is synchronously preempted, then taking a
frame-pointer-based stack trace at that preemption will skip PC of the
caller of the function which called into morestack. This happens because
the frame pointer is pushed to the stack after the preamble, leaving the
stack in an odd state for frame pointer unwinding.
Deal with this by marking a goroutine as synchronously preempted and
using that signal to load the missing PC from the stack. On LR platforms
this is available in gp.sched.lr. On non-LR platforms like x86, it's at
gp.sched.sp, because there are no args, no locals, and no frame pointer
pushed to the SP yet.
For #68090.
Change-Id: I73a1206d8b84eecb8a96dbe727195da30088f288
Reviewed-on: https://go-review.googlesource.com/c/go/+/684435
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
|
|
The hugo binary gets slower, potentially dramatically so, with
GOEXPERIMENT=greenteagc. The root cause is page mapping churn. The Green
Tea code introduced a new implicit nil check on value in a
freshly-allocated span to clear some new heap metadata. This nil check
would read the fresh memory, causing Linux to back that virtual address
space with an RO page. This would then be almost immediately written to,
causing Linux to possibly flush the TLB and find memory to replace that
read-only page (likely deduplicated as just the zero page).
This CL fixes the issue by replacing the implicit nil check, which is a
memory read expected to fault if it's truly nil, with an explicit one.
The explicit nil check is a branch, and thus makes no reads to memory.
The result is that the hugo binary no longer gets slower.
No regression test because it doesn't seem possible without access to OS
internals, like Linux tracepoints. We briefly experimented with RSS
metrics, but they're inconsistent. Some system RSS metrics count the
deduplicated zero page, while others (like those produced by
/proc/self/smaps) do not.
Instead, we'll add a new benchmark to our benchmark suite, separately.
For #73581.
Fixes #74375.
Change-Id: I708321c14749a94ccff55072663012eba18b3b91
Reviewed-on: https://go-review.googlesource.com/c/go/+/684015
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
|
|
When an application calls runtime.GOMAXPROCS(runtime.GOMAXPROCS(0)), the
runtime does not need to change the actual GOMAXPROCS value (via STW).
However, this call must still transition from "automatic" to "custom"
GOMAXPROCS state, thus disabling background updates.
Thus this case shouldn't return quite as early as it currently does.
Change-Id: I6a6a636c42f73996532bd9f7beb95e933256c9e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/683815
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
sysReserveAlignedSbrk locks memlock at entry, but it is not
unlocked at one of the return path. Add the missing unlock.
Fixes #74339.
Change-Id: Ib641bc348aca41494ec410e2c4eb9857f3362484
Reviewed-on: https://go-review.googlesource.com/c/go/+/683295
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: I0c33830b13c8a187ac82504c7653abb8f8cf7530
Reviewed-on: https://go-review.googlesource.com/c/go/+/681655
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sean Liao <sean@liao.dev>
|
|
Currently the mspan limit field is set after allocSpan returns, *after*
the span has already been published to the GC (including the
conservative scanner). But the limit field is load-bearing, because it's
checked to filter out invalid pointers. A stale limit value could cause
a crash by having the conservative scanner access allocBits out of
bounds.
Fix this by setting the mspan limit field before publishing the span.
For large objects and arena chunks, we adjust the limit down after
allocSpan because we don't have access to the true object's size from
allocSpan. However this is safe, since we first initialize the limit to
something definitely safe (the actual span bounds) and only adjust it
down after. Adjusting it down has the benefit of more precise debug
output, but the window in which it's imprecise is also fine because a
single object (logically, with arena chunks) occupies the whole span, so
the 'invalid' part of the memory will just safely point back to that
object. We can't do this for smaller objects because the limit will
include space that does *not* contain any valid objects.
Fixes #74288.
Change-Id: I0a22e5b9bccc1bfdf51d2b73ea7130f1b99c0c7c
Reviewed-on: https://go-review.googlesource.com/c/go/+/682655
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Almost everywhere we stop the world we casGToWaitingForGC to prevent
mutual deadlock with the GC trying to scan our stack. This historically
was only necessary if we weren't stopping the world to change the GC
phase, because what we were worried about was mutual deadlock with mark
workers' use of suspendG. And, they were the only users of suspendG.
In Go 1.22 this changed. The execution tracer began using suspendG, too.
This leads to the possibility of mutual deadlock between the execution
tracer and a goroutine trying to start or end the GC mark phase. The fix
is simple: make the stop-the-world calls for the GC also call
casGToWaitingForGC. This way, suspendG is guaranteed to make progress in
this circumstance, and once it completes, the stop-the-world can
complete as well.
We can take this a step further, though, and move casGToWaitingForGC
into stopTheWorldWithSema, since there's no longer really a place we can
afford to skip this detail.
While we're here, rename casGToWaitingForGC to casGToWaitingForSuspendG,
since the GC is now not the only potential source of mutual deadlock.
Fixes #72740.
Change-Id: I5e3739a463ef3e8173ad33c531e696e46260692f
Reviewed-on: https://go-review.googlesource.com/c/go/+/681501
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Issue #74045 describes a scenario in which gopark is inlined into
readTrace, such that there are no preemption points. This is only a
problem because readTrace spins if trace.shutdown is set, through
traceReaderAvailable. However, trace.shutdown is almost certainly
overkill for traceReaderAvailable. The first condition, checking whether
the reader gen and the flushed gen match, should be sufficient to ensure
the reader wakes up and finishes flushing all buffers. The first
condition is also safe because it guarantees progress. In the case of
shutdown, all the trace work that will be flushed has been flushed, and
so the trace reader will exit into a regular goroutine context when
it's finished. If not shutting down, then the trace reader will release
doneSema, increase readerGen, and then the gopark unlockf will let it
block until new work actually comes in.
Fixes #74045.
Change-Id: Id9b15c277cb731618488771bd484577341b68675
Reviewed-on: https://go-review.googlesource.com/c/go/+/680738
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: I118d1ee09dfd6fd0075f9e5eeeb54441328bff4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/681495
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
It was added in CL 650256, and then the use in the unique package
was removed in CL 650697.
Change-Id: Id95f5dff7e11a2dc3eb544fda2586a305d3d91ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/681476
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Before CL 650697, there was only one system goroutine that could
dynamically change between being a user goroutine and a system
goroutine, and that was the finalizer/cleanup goroutine. In goroutine
profiles, it was handled explicitly. It's status would be checked during
the first STW, and its stack would be recorded. This let the goroutine
profiler completely ignore system goroutines once the world was started
again.
CL 650697 added dedicated cleanup goroutines (there may be more than
one), and with this, the logic for finalizer goroutines no longer
scaled. In that CL, I let the isSystemGoroutine check be dynamic and
dropped the special case, but this was based on incorrect assumptions.
Namely, it's possible for the scheduler to observe, for example, the
finalizer goroutine as a system goroutine and ignore it, but then later
the goroutine profiler itself sees it as a user goroutine. At that point
it's too late and already running. This violates the invariant of the
goroutine profile that all goroutines are handled by the profiler before
they start executing. In practice, the result is that the goroutine
profiler can crash when it checks this invariant (not checking the
invariant means racily reading goroutine stack memory).
The root cause of the problem is that these system goroutines do not
participate in the goroutine profiler's state machine. Normally, when
profiling, goroutines transition from 'absent' to 'in-progress' to
'satisfied'. However with system goroutines, the state machine is
ignored entirely. They always stay in the 'absent' state. This means
that if a goroutine transitions from system to user, it is eligible for
a profile record when it shouldn't be. That transition shouldn't be
allowed to occur with respect to the goroutine profiler, because the
goroutine profiler is trying to snapshot the state of every goroutine.
The fix to this problem is simple: don't ignore system goroutines. Let
them participate in the goroutine profile state machine. Instead, decide
whether or not to record the stack after the goroutine has been acquired
for goroutine profiling. This means if the scheduler observes the
finalizer goroutine as a system goroutine, it will get promoted in the
goroutine profiler's state machine, and no other part of the goroutine
profiler will observe the goroutine again. Simultaneously, the
stack record for the goroutine will be correctly skipped.
Fixes #74090.
Change-Id: Icb9a164a033be22aaa942d19e828e895f700ca74
Reviewed-on: https://go-review.googlesource.com/c/go/+/680477
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
We associate WaitGroups with synctest bubbles by attaching a
special to the WaitGroup. It is not possible to attach a special
to a linker-allocated value, such as:
var wg sync.WaitGroup
Avoid panicking when accessing a linker-allocated WaitGroup
from a bubble. We have no way to associate these WaitGroups
with a bubble, so just treat them as always unbubbled.
This is probably fine, since the WaitGroup was always
created outside the bubble in this case.
Fixes #74005
Change-Id: Ic71514b0b8d0cecd62e45cc929ffcbeb16f54a55
Reviewed-on: https://go-review.googlesource.com/c/go/+/679695
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
semrelease is safe to call on the system stack (since it just readies
goroutines) except for the fact that it might perform a direct G
handoff and call into the scheduler. If handoff is set to false this is
exceptionally rare, but could happen, and has happened for the trace
reader goroutine which releases a trace.doneSema.
Fixes #73469.
Change-Id: I37ece678bc4721bbb6e5879d74daac762b7d742a
Reviewed-on: https://go-review.googlesource.com/c/go/+/680315
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
Finalizers and cleanup funcs weren't running on the windows-arm64
builder. Put finalizers/cleanups on a small struct containing a pointer
rather than an *int, which fixes the problem.
Also uncomment a synctest.Wait that was accidentally commented out.
Fixes #73977
Change-Id: Ia6f18d74d6fccf2c5a9222317977c7458d67f158
Reviewed-on: https://go-review.googlesource.com/c/go/+/679696
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Use the synctest bubble ID to identify bubbles in traces,
rather than the goroutine ID of the bubble's root goroutine.
Some waitReasons include a "(synctest)" suffix to distinguish
a durably blocking state from a non-durable one. For example,
"chan send" vs. "chan send (synctest)". Change this suffix
to "(durable)".
Always print a "(durable)" sufix for the state of durably
blocked bubbled goroutines. For example, print "sleep (durable)".
Drop the "[not] durably blocked" text from goroutine states,
since this is now entirely redundant with the waitReason.
Old:
goroutine 8 [chan receive (synctest), synctest bubble 7, durably blocked]:
goroutine 9 [select (no cases), synctest bubble 7, durably blocked]:
New:
goroutine 8 [chan receive (durable), synctest bubble 1]:
goroutine 9 [select (no cases) (durable), synctest bubble 1]:
Change-Id: I89112efb25150a98a2954f54d1910ccec52a5824
Reviewed-on: https://go-review.googlesource.com/c/go/+/679376
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
The synctest.Test function waits for all goroutines in a bubble to
exit before returning. If there is ever a point when all goroutines
in a bubble are durably blocked, it panics and reports a deadlock.
Panic with a different message depending on whether the bubble's
main goroutine has returned or not. The main goroutine returning
stops the bubble clock, so knowing whether it is running or not
is useful debugging information.
The new panic messages are:
deadlock: all goroutines in bubble are blocked
deadlock: main bubble goroutine has exited but blocked goroutines remain
Change-Id: I94a69e79121c272d9c86f412c1c9c7de57ef27ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/679375
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This issue has been fixed for amd64, arm64 and other platforms
in CL 643875, but it was missed when the race support was
submitted for loong64.
Fixes #71395.
Change-Id: I678f381e868214f1b3399be43187db49e1660933
Reviewed-on: https://go-review.googlesource.com/c/go/+/679055
Reviewed-by: Meidan Li <limeidan@loongson.cn>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
This CL makes two changes to reduce the predictability
with which bubbled timers fire.
When asynctimerchan=0 (the default), regular timers with an associated
channel are only added to a timer heap when some channel operation
is blocked on that channel. This allows us to garbage collect
unreferenced, unstopped timers. Timers in a synctest bubble, in
contrast, are always added to the bubble's timer heap.
This CL changes bubbled timers with a channel to be handled the
same as unbubbled ones, adding them to the bubble's timer heap only
when some channel operation is blocked on the timer's channel.
This permits unstopped bubbled timers to be garbage collected,
but more importantly it makes all timers past their deadline
behave identically, regardless of whether they are in a bubble.
This CL also changes timer scheduling to execute bubbled timers
immediately when possible rather than adding them to a heap.
Timers in a bubble's heap are executed when the bubble is idle.
Executing timers immediately avoids creating a predictable
order of execution.
For #73850
Fixes #73934
Change-Id: If82e441546408f780f6af6fb7f6e416d3160295d
Reviewed-on: https://go-review.googlesource.com/c/go/+/678075
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
There are two additional sources of memory overhead per P that come from
greenteagc. One is for ptrBuf, but on platforms other than Windows it
doesn't actually cost anything due to demand-paging (Windows also
demand-pages, but the memory is 'committed' so it still counts against
OS RSS metrics). The other is for per-sizeclass scan stats. However when
greenteagc is disabled, most of these scan stats are completely unused.
The worst-case memory overhead from these two sources is relatively
small (about 10 KiB per P), but for programs with a small memory
footprint running on a machine with a lot of cores, this can be
significant (single-digit percent).
This change does two things. First, it puts ptrBuf initialization behind
the greenteagc experiment, so now that memory is never allocated by
default. Second, it abstracts the implementation details of scan stat
collection and emission, such that we can have two different
implementations depending on the build tag. This lets us remove all the
unused stats when the greenteagc experiment is disabled, reducing the
memory overhead of the stats from ~2.6 KiB per P to 536 bytes per P.
This is enough to make the difference no longer noticable in our
benchmark suite.
Fixes #73931.
Change-Id: I4351f1cbb3f6743d8f5922d757d73442c6d6ad3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/678535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
For testing out duffcopy changes.
Change-Id: I93b4a52d75418a6e31aae5ad99f95d1870812b69
Reviewed-on: https://go-review.googlesource.com/c/go/+/678215
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
|
|
In synctest bubbles, fire timers scheduled for the same instant
in a randomized order.
Pending timers are added to a heap ordered by the timer's wakeup time.
Add a per-timer random value, set when the timer is added to a heap,
to break ties between timers scheduled for the same instant.
Only inject this randomness in synctest bubbles. We could do so
for all timers at the cost of one cheaprand call per timer,
but given that it's effectively impossible to create two timers
scheduled for the same instant outside of a fake-time environment,
don't bother.
Fixes #73876
For #73850
Change-Id: Ie96c86a816f548d4c31e4e014bf9293639155bd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/677276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
When the GC is disabled, the tracer should emit a heap goal of 0. Not
setting the heap goal to 0 causes an inaccurate NextGC value to be
emmited.
Fixes #63864
Change-Id: Iecceaca86c0a43c1cc4d9433f1f9bb736f01ccbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/639417
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
sync.Cond.Wait is durably blocking. Waking a goroutine out of Cond.Wait
from outside its bubble panics.
Make this panic a fatal panic, since it leaves the notifyList in an
inconsistent state. We could do some work to make this a recoverable
panic, but the complexity doesn't seem worth the outcome.
For #67434
Change-Id: I88874c1519c2e5c0063175297a9b120cedabcd07
Reviewed-on: https://go-review.googlesource.com/c/go/+/675617
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
|
|
vgetrandomGetState can call malloc, so this is not a leaf lock.
Our staticlockrank builder doesn't support vgetrandom, so it didn't
catch this.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking
Change-Id: I6a6a636c36c9172e4ebf9493c10cb23cac29a13f
Reviewed-on: https://go-review.googlesource.com/c/go/+/677255
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
We already guarantee that no automatic updates to GOMAXPROCS occur after
a GOMAXPROCS call returns. This is easily achieved by having the update
goroutine double-check that updates are still allowed during STW before
committing the new value.
However, it is possible for sysmon to concurrently run defaultGOMAXPROCS
to compute a new GOMAXPROCS value after GOMAXPROCS returns. This new
value will be discarded later, but we'll still perform the system calls
necessary to compute the new value.
Normally this distinction doesn't matter, but if you want to sandbox a
Go program, then you may want to disable GOMAXPROCS updates to reduce
the system call footprint. A call to GOMAXPROCS will disable updates,
but without a guarantee on when sysmon will observe the change it is
somewhat fragile.
Add explicit synchronization between GOMAXPROCS and sysmon to guarantee
that sysmon won't run defaultGOMAXPROCS after GOMAXPROCS returns.
The synchronization is a bit complex because we can't hold a mutex
across STW, nor take a semaphore from sysmon, but the result isn't too
bad.
One oddity is that sched.customGOMAXPROCS and gomaxprocs are no longer
updated in lockstep (even though both are protected by sched.lock), but
I don't believe anything should depend on that.
For #73193.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking
Change-Id: I6a6a636cff243a9b69ac1b5d2f98925648e60236
Reviewed-on: https://go-review.googlesource.com/c/go/+/677037
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Add support to internal/synctest for managing associations between
arbitrary pointers and synctest bubbles. (Implemented internally to
the runtime package by attaching a special to the pointer.)
Associate WaitGroups with bubbles.
Since WaitGroups don't have a constructor,
perform the association when Add is called.
All Add calls must be made from within the same bubble,
or outside any bubble.
When a bubbled goroutine calls WaitGroup.Wait,
the wait is durably blocking iff the WaitGroup is associated
with the current bubble.
Change-Id: I77e2701e734ac2fa2b32b28d5b0c853b7b2825c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/676656
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
|
|
The updatemaxprocs metric logic is currently backwards. We only
increment the metric when we update GOMAXPROCS, but that only occurs if
updatemaxprocs is enabled.
Instead, the metric is supposed to increment when updatemaxprocs is
disabled and there would be different behavior if it were enabled.
Theoretically we should run the entire update system in a dry run mode,
and only bail out right before committing updates. But that is an awful
lot of effort for a feature that is disabled. Plus some users (like
sandboxes) want to completely disable the update syscalls
(sched_getaffinity and pread64). If we still do dry run updates then we
need an additional GODEBUG for completely disabling functionality.
This CL also avoids starting the update goroutine at all if disabled,
since it isn't needed.
For #73193.
Change-Id: I6a6a636ceec8fced44e36cb27dcb1b4ba51fce33
Reviewed-on: https://go-review.googlesource.com/c/go/+/677036
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
There are other parts to updating GOMAXPROCS than just the helper
goroutine, so make the naming more specific.
For #73193.
Change-Id: I6a6a636c31ac80c8d76afe90c0bfc29d3086af4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/677035
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Cleanup functions and finalizers must not run in a synctest bubble.
If they did, a function run by the GC at an unpredictable time
could unblock a bubble that synctest believes is durably
blocked.
Add a test verifying that cleanups and finalizers are always
run by non-bubbled goroutines. (This is already the case because
we never add system goroutines to a bubble.)
For #67434
Change-Id: I5a48db2b26f9712c3b0dc1f425d99814031a2fc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/675257
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
|
|
Fixes #73817
Change-Id: I0101bdc797237b4c7eb58b414c71b009b0b44447
Reviewed-on: https://go-review.googlesource.com/c/go/+/675176
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
|
