From aae7734658e5f302c0e3a10f6c5c596fd384dbd7 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Tue, 28 Nov 2023 17:10:46 +0000 Subject: [PATCH] internal/trace/v2: tolerate having a P in GoCreateSyscall 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 LUCI-TryBot-Result: Go LUCI --- src/internal/trace/v2/order.go | 2 +- .../generators/go122-create-syscall-with-p.go | 54 +++++++++++++++++++ .../tests/go122-create-syscall-with-p.test | 22 ++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/internal/trace/v2/testdata/generators/go122-create-syscall-with-p.go create mode 100644 src/internal/trace/v2/testdata/tests/go122-create-syscall-with-p.test diff --git a/src/internal/trace/v2/order.go b/src/internal/trace/v2/order.go index 83cccb4722..bfc2c5c44d 100644 --- a/src/internal/trace/v2/order.go +++ b/src/internal/trace/v2/order.go @@ -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 index 0000000000..59055e5e62 --- /dev/null +++ b/src/internal/trace/v2/testdata/generators/go122-create-syscall-with-p.go @@ -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 index 0000000000..95f86b6f2f --- /dev/null +++ b/src/internal/trace/v2/testdata/tests/go122-create-syscall-with-p.test @@ -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 -- 2.48.1