Revert "runtime: push down systemstack requirement for tracer where possible"

This reverts CL 572095.

Reason for revert: Broke longtest builders.

Change-Id: Iac3a8159d3afb4156a49c7d6819cdd15fe9d4bbb
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/577376
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/runtime/trace2.go b/src/runtime/trace2.go
index a9be4e19622db060a983e952755777a6ffd5e7ba..d516001433680abe5902c59a92fad7d6a7b841cb 100644 (file)
--- a/src/runtime/trace2.go
+++ b/src/runtime/trace2.go
@@ -551,17 +551,17 @@ func traceAdvance(stopTrace bool) {
        // Read everything out of the last gen's CPU profile buffer.
        traceReadCPU(gen)
 
-       // Flush CPU samples, stacks, and strings for the last generation. This is safe,
-       // because we're now certain no M is writing to the last generation.
-       //
-       // Ordering is important here. traceCPUFlush may generate new stacks and dumping
-       // stacks may generate new strings.
-       traceCPUFlush(gen)
-       trace.stackTab[gen%2].dump(gen)
-       trace.stringTab[gen%2].reset(gen)
-
-       // That's it. This generation is done producing buffers.
        systemstack(func() {
+               // Flush CPU samples, stacks, and strings for the last generation. This is safe,
+               // because we're now certain no M is writing to the last generation.
+               //
+               // Ordering is important here. traceCPUFlush may generate new stacks and dumping
+               // stacks may generate new strings.
+               traceCPUFlush(gen)
+               trace.stackTab[gen%2].dump(gen)
+               trace.stringTab[gen%2].reset(gen)
+
+               // That's it. This generation is done producing buffers.
                lock(&trace.lock)
                trace.flushedGen.Store(gen)
                unlock(&trace.lock)

diff --git a/src/runtime/trace2cpu.go b/src/runtime/trace2cpu.go
index 2bb6f903f566237918cd632be95ffc71f4a30bcc..b3b0fb046d82b66e31b6e8463f008a746cddc1e8 100644 (file)
--- a/src/runtime/trace2cpu.go
+++ b/src/runtime/trace2cpu.go
@@ -195,15 +195,18 @@ func traceReadCPU(gen uintptr) bool {
 
 // traceCPUFlush flushes trace.cpuBuf[gen%2]. The caller must be certain that gen
 // has completed and that there are no more writers to it.
+//
+// Must run on the systemstack because it flushes buffers and acquires trace.lock
+// to do so.
+//
+//go:systemstack
 func traceCPUFlush(gen uintptr) {
        // Flush any remaining trace buffers containing CPU samples.
        if buf := trace.cpuBuf[gen%2]; buf != nil {
-               systemstack(func() {
-                       lock(&trace.lock)
-                       traceBufFlush(buf, gen)
-                       unlock(&trace.lock)
-                       trace.cpuBuf[gen%2] = nil
-               })
+               lock(&trace.lock)
+               traceBufFlush(buf, gen)
+               unlock(&trace.lock)
+               trace.cpuBuf[gen%2] = nil
        }
 }
 

diff --git a/src/runtime/trace2stack.go b/src/runtime/trace2stack.go
index 7d698c89d32c14c4ac36f571cce5b4e84d33cfe8..4ee3b32b05969a97b202a5a14bebd47009836601 100644 (file)
--- a/src/runtime/trace2stack.go
+++ b/src/runtime/trace2stack.go
@@ -138,6 +138,11 @@ func (t *traceStackTable) put(pcs []uintptr) uint64 {
 // dump writes all previously cached stacks to trace buffers,
 // releases all memory and resets state. It must only be called once the caller
 // can guarantee that there are no more writers to the table.
+//
+// This must run on the system stack because it flushes buffers and thus
+// may acquire trace.lock.
+//
+//go:systemstack
 func (t *traceStackTable) dump(gen uintptr) {
        w := unsafeTraceWriter(gen, nil)
 
@@ -189,11 +194,9 @@ func (t *traceStackTable) dump(gen uintptr) {
        }
        // Still, hold the lock over reset. The callee expects it, even though it's
        // not strictly necessary.
-       systemstack(func() {
-               lock(&t.tab.lock)
-               t.tab.reset()
-               unlock(&t.tab.lock)
-       })
+       lock(&t.tab.lock)
+       t.tab.reset()
+       unlock(&t.tab.lock)
 
        w.flush().end()
 }

diff --git a/src/runtime/trace2string.go b/src/runtime/trace2string.go
index 21ef5eaf980b26ab1ba1169204ba5674680d1b7e..cbb0ecfb37438001c64a582e3a905f02714930a9 100644 (file)
--- a/src/runtime/trace2string.go
+++ b/src/runtime/trace2string.go
@@ -49,7 +49,7 @@ func (t *traceStringTable) emit(gen uintptr, s string) uint64 {
 
 // writeString writes the string to t.buf.
 //
-// Must run on the systemstack because it acquires t.lock.
+// Must run on the systemstack because it may flush buffers and thus could acquire trace.lock.
 //
 //go:systemstack
 func (t *traceStringTable) writeString(gen uintptr, id uint64, s string) {
@@ -75,7 +75,7 @@ func (t *traceStringTable) writeString(gen uintptr, id uint64, s string) {
        w.varint(uint64(len(s)))
        w.stringData(s)
 
-       // Store back buf in case it was updated during ensure.
+       // Store back buf if it was updated during ensure.
        t.buf = w.traceBuf
        unlock(&t.lock)
 }
@@ -84,20 +84,21 @@ func (t *traceStringTable) writeString(gen uintptr, id uint64, s string) {
 //
 // Must be called only once the caller is certain nothing else will be
 // added to this table.
+//
+// Because it flushes buffers, this may acquire trace.lock and thus
+// must run on the systemstack.
+//
+//go:systemstack
 func (t *traceStringTable) reset(gen uintptr) {
        if t.buf != nil {
-               systemstack(func() {
-                       lock(&trace.lock)
-                       traceBufFlush(t.buf, gen)
-                       unlock(&trace.lock)
-               })
+               lock(&trace.lock)
+               traceBufFlush(t.buf, gen)
+               unlock(&trace.lock)
                t.buf = nil
        }
 
        // Reset the table.
-       systemstack(func() {
-               lock(&t.tab.lock)
-               t.tab.reset()
-               unlock(&t.tab.lock)
-       })
+       lock(&t.tab.lock)
+       t.tab.reset()
+       unlock(&t.tab.lock)
 }