]> Cypherpunks repositories - gostls13.git/commitdiff
internal/trace/v2: tolerate having a P in GoCreateSyscall
authorMichael Anthony Knyszek <mknyszek@google.com>
Tue, 28 Nov 2023 17:10:46 +0000 (17:10 +0000)
committerMichael Knyszek <mknyszek@google.com>
Tue, 28 Nov 2023 20:49:54 +0000 (20:49 +0000)
On non-pthread platforms, it's totally possible for the same M to
GoCreateSyscall/GoDestroySyscall on the same thread multiple times. That
same thread may hold onto its P through all those calls.

For #64060.

Change-Id: Ib968bfd439ecd5bc24fc98d78c06145b0d4b7802
Reviewed-on: https://go-review.googlesource.com/c/go/+/545515
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/internal/trace/v2/order.go
src/internal/trace/v2/testdata/generators/go122-create-syscall-with-p.go [new file with mode: 0644]
src/internal/trace/v2/testdata/tests/go122-create-syscall-with-p.test [new file with mode: 0644]

index 83cccb4722c0ae5b59bd6eabd718a785f3ce6d21..bfc2c5c44d2a38bbd28f5389fec37d38564c5a8d 100644 (file)
@@ -485,7 +485,7 @@ func (o *ordering) advance(ev *baseEvent, evt *evTable, m ThreadID, gen uint64)
                // This event indicates that a goroutine is effectively
                // being created out of a cgo callback. Such a goroutine
                // is 'created' in the syscall state.
-               if err := validateCtx(curCtx, event.SchedReqs{Thread: event.MustHave, Proc: event.MustNotHave, Goroutine: event.MustNotHave}); err != nil {
+               if err := validateCtx(curCtx, event.SchedReqs{Thread: event.MustHave, Proc: event.MayHave, Goroutine: event.MustNotHave}); err != nil {
                        return curCtx, false, err
                }
                // This goroutine is effectively being created. Add a state for it.
diff --git a/src/internal/trace/v2/testdata/generators/go122-create-syscall-with-p.go b/src/internal/trace/v2/testdata/generators/go122-create-syscall-with-p.go
new file mode 100644 (file)
index 0000000..59055e5
--- /dev/null
@@ -0,0 +1,54 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Tests a G being created from within a syscall.
+//
+// Specifically, it tests a scenerio wherein a C
+// thread is calling into Go, creating a goroutine in
+// a syscall (in the tracer's model). Because the actual
+// m can be reused, it's possible for that m to have never
+// had its P (in _Psyscall) stolen.
+//
+// This is a regression test. The trace parser once required
+// GoCreateSyscall to not have a P, but it can in the scenario
+// described above.
+
+package main
+
+import (
+       "internal/trace/v2"
+       "internal/trace/v2/event/go122"
+       testgen "internal/trace/v2/internal/testgen/go122"
+)
+
+func main() {
+       testgen.Main(gen)
+}
+
+func gen(t *testgen.Trace) {
+       t.DisableTimestamps()
+
+       g := t.Generation(1)
+
+       // A C thread calls into Go and acquires a P. It returns
+       // back to C, destroying the G. It then comes back to Go
+       // on the same thread and again returns to C.
+       //
+       // Note: on pthread platforms this can't happen on the
+       // same thread because the m is stashed in TLS between
+       // calls into Go, until the thread dies. This is still
+       // possible on other platforms, however.
+       b0 := g.Batch(trace.ThreadID(0), 0)
+       b0.Event("GoCreateSyscall", trace.GoID(4))
+       b0.Event("ProcStatus", trace.ProcID(0), go122.ProcIdle)
+       b0.Event("ProcStart", trace.ProcID(0), testgen.Seq(1))
+       b0.Event("GoSyscallEndBlocked")
+       b0.Event("GoStart", trace.GoID(4), testgen.Seq(1))
+       b0.Event("GoSyscallBegin", testgen.Seq(2), testgen.NoStack)
+       b0.Event("GoDestroySyscall")
+       b0.Event("GoCreateSyscall", trace.GoID(4))
+       b0.Event("GoSyscallEnd")
+       b0.Event("GoSyscallBegin", testgen.Seq(3), testgen.NoStack)
+       b0.Event("GoDestroySyscall")
+}
diff --git a/src/internal/trace/v2/testdata/tests/go122-create-syscall-with-p.test b/src/internal/trace/v2/testdata/tests/go122-create-syscall-with-p.test
new file mode 100644 (file)
index 0000000..95f86b6
--- /dev/null
@@ -0,0 +1,22 @@
+-- expect --
+SUCCESS
+-- trace --
+Trace Go1.22
+EventBatch gen=1 m=0 time=0 size=34
+GoCreateSyscall dt=0 new_g=4
+ProcStatus dt=0 p=0 pstatus=2
+ProcStart dt=0 p=0 p_seq=1
+GoSyscallEndBlocked dt=0
+GoStart dt=0 g=4 g_seq=1
+GoSyscallBegin dt=0 p_seq=2 stack=0
+GoDestroySyscall dt=0
+GoCreateSyscall dt=0 new_g=4
+GoSyscallEnd dt=0
+GoSyscallBegin dt=0 p_seq=3 stack=0
+GoDestroySyscall dt=0
+EventBatch gen=1 m=18446744073709551615 time=0 size=5
+Frequency freq=15625000
+EventBatch gen=1 m=18446744073709551615 time=0 size=1
+Stacks
+EventBatch gen=1 m=18446744073709551615 time=0 size=1
+Strings