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>

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)

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
 	}
 }
 

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()
 }

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)
 }