runtime: cleanup M vgetrandom state before dropping P

When an M is destroyed, we put its vgetrandom state back on the shared
list for another M to reuse. This list is simply a slice, so appending
to the slice may allocate. Currently this operation is performed in
mdestroy, after the P is released, meaning allocation is not allowed.

More the cleanup earlier in mdestroy when allocation is still OK.

Also add //go:nowritebarrierrec to mdestroy since it runs without a P,
which would have caught this bug.

Fixes #73141.

Change-Id: I6a6a636c3fbf5c6eec09d07a260e39dbb4d2db12
Reviewed-on: https://go-review.googlesource.com/c/go/+/662455
Reviewed-by: Jason Donenfeld <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

Author: prattmic

src/runtime/os3_solaris.go

@@ -234,8 +234,11 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_aix.go

@@ -186,8 +186,11 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_darwin.go

@@ -344,8 +344,11 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_dragonfly.go

@@ -216,8 +216,11 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_linux.go

@@ -412,13 +412,12 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
-	if mp.vgetrandomState != 0 {
-		vgetrandomPutState(mp.vgetrandomState)
-		mp.vgetrandomState = 0
-	}
 }
 
 // #ifdef GOARCH_386

src/runtime/os_netbsd.go

@@ -320,8 +320,11 @@ func unminit() {
 	// must continue working after unminit.
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_openbsd.go

@@ -182,8 +182,11 @@ func unminit() {
 	getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_plan9.go

@@ -218,8 +218,11 @@ func minit() {
 func unminit() {
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

src/runtime/os_windows.go

@@ -905,9 +905,11 @@ func unminit() {
 	mp.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
 //
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 //go:nosplit
 func mdestroy(mp *m) {
 	if mp.highResTimer != 0 {

src/runtime/proc.go

@@ -1958,6 +1958,9 @@ func mexit(osStack bool) {
 		mp.gsignal = nil
 	}
 
+	// Free vgetrandom state.
+	vgetrandomDestroy(mp)
+
 	// Remove m from allm.
 	lock(&sched.lock)
 	for pprev := &allm; *pprev != nil; pprev = &(*pprev).alllink {

src/runtime/vgetrandom_linux.go

@@ -74,9 +74,16 @@ func vgetrandomGetState() uintptr {
 	return state
 }
 
-func vgetrandomPutState(state uintptr) {
+// Free vgetrandom state from the M (if any) prior to destroying the M.
+//
+// This may allocate, so it must have a P.
+func vgetrandomDestroy(mp *m) {
+	if mp.vgetrandomState == 0 {
+		return
+	}
+
 	lock(&vgetrandomAlloc.statesLock)
-	vgetrandomAlloc.states = append(vgetrandomAlloc.states, state)
+	vgetrandomAlloc.states = append(vgetrandomAlloc.states, mp.vgetrandomState)
 	unlock(&vgetrandomAlloc.statesLock)
 }
 

src/runtime/vgetrandom_unsupported.go

@@ -13,6 +13,6 @@ func vgetrandom(p []byte, flags uint32) (ret int, supported bool) {
 	return -1, false
 }
 
-func vgetrandomPutState(state uintptr) {}
+func vgetrandomDestroy(mp *m) {}
 
 func vgetrandomInit() {}