From 8adc63b3eb21e8bbacd13335bcf8d6b3a9a507c4 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sat, 13 Aug 2022 04:13:56 +0000 Subject: [PATCH] Revert "runtime/trace: add missing events for the locked g in extra M." This reverts commit ea9c3fd42d94182ce6f87104b68a51ea92f1a571. Reason for revert: break linux/ricsv64, openbsd/arm, illumos/amd64 builders Change-Id: I98479a8f63e76eed89a0e8846acf2c73e8441377 Reviewed-on: https://go-review.googlesource.com/c/go/+/423437 Reviewed-by: Than McIntosh Auto-Submit: Michael Pratt Reviewed-by: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Cuong Manh Le --- src/internal/trace/goroutines.go | 5 +- src/runtime/crash_cgo_test.go | 13 ----- src/runtime/proc.go | 9 --- src/runtime/runtime2.go | 1 - .../testdata/testprogcgo/issue29707.go | 58 ------------------- src/runtime/trace.go | 13 +---- 6 files changed, 3 insertions(+), 96 deletions(-) delete mode 100644 src/runtime/testdata/testprogcgo/issue29707.go diff --git a/src/internal/trace/goroutines.go b/src/internal/trace/goroutines.go index 8df5e6c6c5..5da90e0b6d 100644 --- a/src/internal/trace/goroutines.go +++ b/src/internal/trace/goroutines.go @@ -187,7 +187,7 @@ func GoroutineStats(events []*Event) map[uint64]*GDesc { gs[g.ID] = g case EvGoStart, EvGoStartLabel: g := gs[ev.G] - if g.PC == 0 && len(ev.Stk) > 0 { + if g.PC == 0 { g.PC = ev.Stk[0].PC g.Name = ev.Stk[0].Fn } @@ -353,6 +353,5 @@ func RelatedGoroutines(events []*Event, goid uint64) map[uint64]bool { func IsSystemGoroutine(entryFn string) bool { // This mimics runtime.isSystemGoroutine as closely as // possible. - // Also, locked g in extra M (with empty entryFn) is system goroutine. - return entryFn == "" || entryFn != "runtime.main" && strings.HasPrefix(entryFn, "runtime.") + return entryFn != "runtime.main" && strings.HasPrefix(entryFn, "runtime.") } diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index d5df27cb11..5e58712297 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -710,16 +710,3 @@ func TestCgoTracebackGoroutineProfile(t *testing.T) { t.Fatalf("want %s, got %s\n", want, output) } } - -func TestCgoTraceParser(t *testing.T) { - // Test issue 29707. - switch runtime.GOOS { - case "windows", "plan9": - t.Skipf("skipping cgo trace parser test on %s", runtime.GOOS) - } - output := runTestProg(t, "testprogcgo", "CgoTraceParser") - want := "OK\n" - if output != want { - t.Fatalf("want %s, got %s\n", want, output) - } -} diff --git a/src/runtime/proc.go b/src/runtime/proc.go index a366f0264d..1de1ed781f 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1917,7 +1917,6 @@ func oneNewExtraM() { casgstatus(gp, _Gidle, _Gdead) gp.m = mp mp.curg = gp - mp.isextra = true mp.lockedInt++ mp.lockedg.set(gp) gp.lockedm.set(mp) @@ -1925,14 +1924,6 @@ func oneNewExtraM() { if raceenabled { gp.racectx = racegostart(abi.FuncPCABIInternal(newextram) + sys.PCQuantum) } - if trace.enabled { - // trigger two trace events for the locked g in the extra m, - // since the next event of the g will be traceEvGoSysExit in exitsyscall, - // while calling from C thread to Go. - traceGoCreate(gp, 0) // no start pc - gp.traceseq++ - traceEvent(traceEvGoInSyscall, -1, gp.goid) - } // put on allg for garbage collector allgadd(gp) diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index f95e11bec4..32ad34ccdf 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -546,7 +546,6 @@ type m struct { newSigstack bool // minit on C thread called sigaltstack printlock int8 incgo bool // m is executing a cgo call - isextra bool // m is an extra m freeWait uint32 // if == 0, safe to free g0 and delete m (atomic) fastrand uint64 needextram bool diff --git a/src/runtime/testdata/testprogcgo/issue29707.go b/src/runtime/testdata/testprogcgo/issue29707.go deleted file mode 100644 index 5a354301ae..0000000000 --- a/src/runtime/testdata/testprogcgo/issue29707.go +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2011 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. - -//go:build !plan9 && !windows -// +build !plan9,!windows - -// This is for issue #29707 - -package main - -/* -#include - -extern void* callback(void*); -typedef void* (*cb)(void*); - -static void testCallback(cb cb) { - pthread_t thread_id; - pthread_create(&thread_id, NULL, cb, NULL); - pthread_join(thread_id, NULL); -} -*/ -import "C" - -import ( - "bytes" - "fmt" - traceparser "internal/trace" - "runtime/trace" - "time" - "unsafe" -) - -func init() { - register("CgoTraceParser", CgoTraceParser) -} - -//export callback -func callback(unsafe.Pointer) unsafe.Pointer { - time.Sleep(time.Millisecond) - return nil -} - -func CgoTraceParser() { - buf := new(bytes.Buffer) - - trace.Start(buf) - C.testCallback(C.cb(C.callback)) - trace.Stop() - - _, err := traceparser.Parse(buf, "") - if err != nil { - fmt.Println("Parse error: ", err) - } else { - fmt.Println("OK") - } -} diff --git a/src/runtime/trace.go b/src/runtime/trace.go index d0921eeaa8..1b5e9df38b 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -272,17 +272,6 @@ func StartTrace() error { if status == _Gsyscall { gp.traceseq++ traceEvent(traceEvGoInSyscall, -1, gp.goid) - } else if status == _Gdead && gp.m != nil && gp.m.isextra { - // trigger two trace events for the dead g in the extra m, - // since the next event of the g will be traceEvGoSysExit in exitsyscall, - // while calling from C thread to Go. - gp.traceseq = 0 - gp.tracelastp = getg().m.p - // +PCQuantum because traceFrameForPC expects return PCs and subtracts PCQuantum. - id := trace.stackTab.put([]uintptr{startPCforTrace(0) + sys.PCQuantum}) // no start pc - traceEvent(traceEvGoCreate, -1, gp.goid, uint64(id), stackID) - gp.traceseq++ - traceEvent(traceEvGoInSyscall, -1, gp.goid) } else { gp.sysblocktraced = false } @@ -1566,7 +1555,7 @@ func trace_userLog(id uint64, category, message string) { func startPCforTrace(pc uintptr) uintptr { f := findfunc(pc) if !f.valid() { - return pc // may happen for locked g in extra M since its pc is 0. + return pc // should not happen, but don't care } w := funcdata(f, _FUNCDATA_WrapInfo) if w == nil { -- 2.50.0