diff options
| author | Michael Anthony Knyszek <mknyszek@google.com> | 2023-11-10 22:46:47 +0000 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2023-11-14 16:35:09 +0000 |
| commit | c06beb9eff441271bedf34ea9777a92db8018f9d (patch) | |
| tree | 5fe125caa0587845801ef2122ec6e6322e9ce151 /src/runtime | |
| parent | 859c13a5ad80308125c79521cfc047200fd5a512 (diff) | |
| download | go-c06beb9eff441271bedf34ea9777a92db8018f9d.tar.xz | |
internal/trace/v2: resolve syscall parsing ambiguity
After landing the new execution tracer, the Windows builders failed with
some new errors.
Currently the GoSyscallBegin event has no indicator that its the target
of a ProcSteal event. This can lead to an ambiguous situation that is
unresolvable if timestamps are broken. For instance, if the tracer sees
the ProcSteal event while a goroutine has been observed to be in a
syscall (one that, for instance, did not actually lose its P), it will
proceed with the ProcSteal incorrectly.
This is a little abstract. For a more concrete example, see the
go122-syscall-steal-proc-ambiguous test.
This change resolves this ambiguity by interleaving GoSyscallBegin
events into how Ps are sequenced. Because a ProcSteal has a sequence
number (it has to, it's stopping a P from a distance) it necessarily
has to synchronize with a precise ProcStart event. This change basically
just extends this synchronization to GoSyscallBegin, so the ProcSteal
can't advance until _exactly the right_ syscall has been entered.
This change removes the test skip, since it and CL 541695 fix the two
main issues observed on Windows platforms.
For #60773.
Fixes #64061.
Change-Id: I069389cd7fe1ea903edf42d79912f6e2bcc23f62
Reviewed-on: https://go-review.googlesource.com/c/go/+/541696
Auto-Submit: 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>
Diffstat (limited to 'src/runtime')
| -rw-r--r-- | src/runtime/trace2event.go | 2 | ||||
| -rw-r--r-- | src/runtime/trace2runtime.go | 5 |
2 files changed, 4 insertions, 3 deletions
diff --git a/src/runtime/trace2event.go b/src/runtime/trace2event.go index f7abf60c5f..1f2a9f754b 100644 --- a/src/runtime/trace2event.go +++ b/src/runtime/trace2event.go @@ -52,7 +52,7 @@ const ( traceEvGoStop // goroutine yields its time, but is runnable [timestamp, reason, stack ID] traceEvGoBlock // goroutine blocks [timestamp, reason, stack ID] traceEvGoUnblock // goroutine is unblocked [timestamp, goroutine ID, goroutine seq, stack ID] - traceEvGoSyscallBegin // syscall enter [timestamp, stack ID] + traceEvGoSyscallBegin // syscall enter [timestamp, P seq, stack ID] traceEvGoSyscallEnd // syscall exit [timestamp] traceEvGoSyscallEndBlocked // syscall exit and it blocked at some point [timestamp] traceEvGoStatus // goroutine status at the start of a generation [timestamp, goroutine ID, M ID, status] diff --git a/src/runtime/trace2runtime.go b/src/runtime/trace2runtime.go index 74aeb57d80..b6837d0360 100644 --- a/src/runtime/trace2runtime.go +++ b/src/runtime/trace2runtime.go @@ -465,8 +465,9 @@ func (tl traceLocker) GoSysCall() { skip = 4 } // Scribble down the M that the P is currently attached to. - tl.mp.p.ptr().trace.mSyscallID = int64(tl.mp.procid) - tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, tl.stack(skip)) + pp := tl.mp.p.ptr() + pp.trace.mSyscallID = int64(tl.mp.procid) + tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, pp.trace.nextSeq(tl.gen), tl.stack(skip)) } // GoSysExit emits a GoSyscallEnd event, possibly along with a GoSyscallBlocked event |