runtime: don't count nGsyscallNoP for extra Ms in C

In #76435, it turns out that the new metric
/sched/goroutines/not-in-go:goroutines counts C threads that have called
into Go before (on Linux) as not-in-go goroutines. The reason for this
is that the M is still attached to the C thread on Linux as an
optimization, so we don't go through all the trouble of detaching the M
and, of course, decrementing nGsyscallNoP.

There's an easy fix to this accounting issue. The flag on the M,
isExtraInC, says whether a thread with an extra M attached no longer has
any Go on its (logical) stack. When we take the P from an M in this
state, we simply just don't increment nGsyscallNoP. When it calls back
into Go, we similarly skip the decrement to nGsyscallNoP.

This is more efficient than alternatives, like always updating
nGsyscallNoP in cgocallbackg, since that would add a new
read-modify-write atomic onto that fast path. It does mean we count
threads in C with a P still attached as not-in-go, but this transient in
most real programs, assuming the thread indeed does not call back into
Go any time soon.

Fixes #76435.

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

1 files changed, 108 insertions, 0 deletions

diff --git a/src/runtime/testdata/testprogcgo/notingo.go b/src/runtime/testdata/testprogcgo/notingo.go
new file mode 100644
index 0000000000..e5b1062e9e
--- /dev/null
+++ b/src/runtime/testdata/testprogcgo/notingo.go
@@ -0,0 +1,108 @@
+// Copyright 2025 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
+
+package main
+
+/*
+#include <stdatomic.h>
+#include <stddef.h>
+#include <pthread.h>
+
+extern void Ready();
+
+static int spinning;
+static int released;
+
+static void* enterGoThenSpinTwice(void* arg __attribute__ ((unused))) {
+	Ready();
+	atomic_fetch_add(&spinning, 1);
+	while(atomic_load(&released) == 0) {};
+
+	Ready();
+	atomic_fetch_add(&spinning, 1);
+	while(1) {};
+	return NULL;
+}
+
+static void SpinTwiceInNewCThread() {
+	pthread_t tid;
+	pthread_create(&tid, NULL, enterGoThenSpinTwice, NULL);
+}
+
+static int Spinning() {
+	return atomic_load(&spinning);
+}
+
+static void Release() {
+	atomic_store(&spinning, 0);
+	atomic_store(&released, 1);
+}
+*/
+import "C"
+
+import (
+	"os"
+	"runtime"
+	"runtime/metrics"
+)
+
+func init() {
+	register("NotInGoMetricCallback", NotInGoMetricCallback)
+}
+
+func NotInGoMetricCallback() {
+	const N = 10
+	s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}}
+
+	// Create N new C threads that have called into Go at least once.
+	for range N {
+		C.SpinTwiceInNewCThread()
+	}
+
+	// Synchronize with spinning threads twice.
+	//
+	// This helps catch bad accounting by taking at least a couple other
+	// codepaths which would cause the accounting to change.
+	for i := range 2 {
+		// Make sure they pass through Go.
+		// N.B. Ready is called twice by the new threads.
+		for j := range N {
+			<-readyCh
+			if j == 2 {
+				// Try to trigger an update in the immediate STW handoff case.
+				runtime.ReadMemStats(&m)
+			}
+		}
+
+		// Make sure they're back in C.
+		for C.Spinning() < N {
+		}
+
+		// Do something that stops the world to take all the Ps back.
+		runtime.ReadMemStats(&m)
+
+		if i == 0 {
+			C.Release()
+		}
+	}
+
+	// Read not-in-go.
+	metrics.Read(s)
+	if n := s[0].Value.Uint64(); n != 0 {
+		println("expected 0 not-in-go goroutines, found", n)
+		os.Exit(2)
+	}
+	println("OK")
+}
+
+var m runtime.MemStats
+var readyCh = make(chan bool)
+
+//export Ready
+func Ready() {
+	readyCh <- true
+}