|
| 1 | +From ff2636f45e0087a1c6d8e895257d9c4729710811 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Michael Pratt <mpratt@google.com> |
| 3 | +Date: Thu, 03 Apr 2025 03:26:25 +0000 |
| 4 | +Subject: [PATCH] [release-branch.go1.24] runtime: cleanup M vgetrandom state before dropping P |
| 5 | + |
| 6 | +When an M is destroyed, we put its vgetrandom state back on the shared |
| 7 | +list for another M to reuse. This list is simply a slice, so appending |
| 8 | +to the slice may allocate. Currently this operation is performed in |
| 9 | +mdestroy, after the P is released, meaning allocation is not allowed. |
| 10 | + |
| 11 | +More the cleanup earlier in mdestroy when allocation is still OK. |
| 12 | + |
| 13 | +Also add //go:nowritebarrierrec to mdestroy since it runs without a P, |
| 14 | +which would have caught this bug. |
| 15 | + |
| 16 | +Fixes #73144. |
| 17 | +For #73141. |
| 18 | + |
| 19 | +Change-Id: I6a6a636c3fbf5c6eec09d07a260e39dbb4d2db12 |
| 20 | +Reviewed-on: https://go-review.googlesource.com/c/go/+/662455 |
| 21 | +Reviewed-by: Jason Donenfeld <Jason@zx2c4.com> |
| 22 | +LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> |
| 23 | +Reviewed-by: Keith Randall <khr@golang.org> |
| 24 | +Reviewed-by: Keith Randall <khr@google.com> |
| 25 | +(cherry picked from commit 0b31e6d4cc804ab76ae8ced151ee2f50657aec14) |
| 26 | +--- |
| 27 | + |
| 28 | +diff --git a/src/runtime/os3_solaris.go b/src/runtime/os3_solaris.go |
| 29 | +index cf163a6..ded821b 100644 |
| 30 | +--- a/src/runtime/os3_solaris.go |
| 31 | ++++ b/src/runtime/os3_solaris.go |
| 32 | +@@ -234,8 +234,11 @@ |
| 33 | + getg().m.procid = 0 |
| 34 | + } |
| 35 | + |
| 36 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 37 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 38 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 39 | ++// |
| 40 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 41 | ++//go:nowritebarrierrec |
| 42 | + func mdestroy(mp *m) { |
| 43 | + } |
| 44 | + |
| 45 | +diff --git a/src/runtime/os_aix.go b/src/runtime/os_aix.go |
| 46 | +index 93464cb..1b483c2 100644 |
| 47 | +--- a/src/runtime/os_aix.go |
| 48 | ++++ b/src/runtime/os_aix.go |
| 49 | +@@ -186,8 +186,11 @@ |
| 50 | + getg().m.procid = 0 |
| 51 | + } |
| 52 | + |
| 53 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 54 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 55 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 56 | ++// |
| 57 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 58 | ++//go:nowritebarrierrec |
| 59 | + func mdestroy(mp *m) { |
| 60 | + } |
| 61 | + |
| 62 | +diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go |
| 63 | +index 0ecbea7..6eab3b5 100644 |
| 64 | +--- a/src/runtime/os_darwin.go |
| 65 | ++++ b/src/runtime/os_darwin.go |
| 66 | +@@ -344,8 +344,11 @@ |
| 67 | + getg().m.procid = 0 |
| 68 | + } |
| 69 | + |
| 70 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 71 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 72 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 73 | ++// |
| 74 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 75 | ++//go:nowritebarrierrec |
| 76 | + func mdestroy(mp *m) { |
| 77 | + } |
| 78 | + |
| 79 | +diff --git a/src/runtime/os_dragonfly.go b/src/runtime/os_dragonfly.go |
| 80 | +index a02696e..9b32350 100644 |
| 81 | +--- a/src/runtime/os_dragonfly.go |
| 82 | ++++ b/src/runtime/os_dragonfly.go |
| 83 | +@@ -216,8 +216,11 @@ |
| 84 | + getg().m.procid = 0 |
| 85 | + } |
| 86 | + |
| 87 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 88 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 89 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 90 | ++// |
| 91 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 92 | ++//go:nowritebarrierrec |
| 93 | + func mdestroy(mp *m) { |
| 94 | + } |
| 95 | + |
| 96 | +diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go |
| 97 | +index 8b3c4d0..fb46b81 100644 |
| 98 | +--- a/src/runtime/os_linux.go |
| 99 | ++++ b/src/runtime/os_linux.go |
| 100 | +@@ -412,13 +412,12 @@ |
| 101 | + getg().m.procid = 0 |
| 102 | + } |
| 103 | + |
| 104 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 105 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 106 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 107 | ++// |
| 108 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 109 | ++//go:nowritebarrierrec |
| 110 | + func mdestroy(mp *m) { |
| 111 | +- if mp.vgetrandomState != 0 { |
| 112 | +- vgetrandomPutState(mp.vgetrandomState) |
| 113 | +- mp.vgetrandomState = 0 |
| 114 | +- } |
| 115 | + } |
| 116 | + |
| 117 | + // #ifdef GOARCH_386 |
| 118 | +diff --git a/src/runtime/os_netbsd.go b/src/runtime/os_netbsd.go |
| 119 | +index 735ace2..a06e5fe 100644 |
| 120 | +--- a/src/runtime/os_netbsd.go |
| 121 | ++++ b/src/runtime/os_netbsd.go |
| 122 | +@@ -320,8 +320,11 @@ |
| 123 | + // must continue working after unminit. |
| 124 | + } |
| 125 | + |
| 126 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 127 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 128 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 129 | ++// |
| 130 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 131 | ++//go:nowritebarrierrec |
| 132 | + func mdestroy(mp *m) { |
| 133 | + } |
| 134 | + |
| 135 | +diff --git a/src/runtime/os_openbsd.go b/src/runtime/os_openbsd.go |
| 136 | +index 574bfa8..4ce4c3c 100644 |
| 137 | +--- a/src/runtime/os_openbsd.go |
| 138 | ++++ b/src/runtime/os_openbsd.go |
| 139 | +@@ -182,8 +182,11 @@ |
| 140 | + getg().m.procid = 0 |
| 141 | + } |
| 142 | + |
| 143 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 144 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 145 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 146 | ++// |
| 147 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 148 | ++//go:nowritebarrierrec |
| 149 | + func mdestroy(mp *m) { |
| 150 | + } |
| 151 | + |
| 152 | +diff --git a/src/runtime/os_plan9.go b/src/runtime/os_plan9.go |
| 153 | +index 2dbb42a..3b5965a 100644 |
| 154 | +--- a/src/runtime/os_plan9.go |
| 155 | ++++ b/src/runtime/os_plan9.go |
| 156 | +@@ -217,8 +217,11 @@ |
| 157 | + func unminit() { |
| 158 | + } |
| 159 | + |
| 160 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 161 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 162 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 163 | ++// |
| 164 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 165 | ++//go:nowritebarrierrec |
| 166 | + func mdestroy(mp *m) { |
| 167 | + } |
| 168 | + |
| 169 | +diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go |
| 170 | +index 7183e79..54407a3 100644 |
| 171 | +--- a/src/runtime/os_windows.go |
| 172 | ++++ b/src/runtime/os_windows.go |
| 173 | +@@ -906,9 +906,11 @@ |
| 174 | + mp.procid = 0 |
| 175 | + } |
| 176 | + |
| 177 | +-// Called from exitm, but not from drop, to undo the effect of thread-owned |
| 178 | ++// Called from mexit, but not from dropm, to undo the effect of thread-owned |
| 179 | + // resources in minit, semacreate, or elsewhere. Do not take locks after calling this. |
| 180 | + // |
| 181 | ++// This always runs without a P, so //go:nowritebarrierrec is required. |
| 182 | ++//go:nowritebarrierrec |
| 183 | + //go:nosplit |
| 184 | + func mdestroy(mp *m) { |
| 185 | + if mp.highResTimer != 0 { |
| 186 | +diff --git a/src/runtime/proc.go b/src/runtime/proc.go |
| 187 | +index e9873e5..21bee4d 100644 |
| 188 | +--- a/src/runtime/proc.go |
| 189 | ++++ b/src/runtime/proc.go |
| 190 | +@@ -1935,6 +1935,9 @@ |
| 191 | + mp.gsignal = nil |
| 192 | + } |
| 193 | + |
| 194 | ++ // Free vgetrandom state. |
| 195 | ++ vgetrandomDestroy(mp) |
| 196 | ++ |
| 197 | + // Remove m from allm. |
| 198 | + lock(&sched.lock) |
| 199 | + for pprev := &allm; *pprev != nil; pprev = &(*pprev).alllink { |
| 200 | +diff --git a/src/runtime/vgetrandom_linux.go b/src/runtime/vgetrandom_linux.go |
| 201 | +index a6ec4b7..40be022 100644 |
| 202 | +--- a/src/runtime/vgetrandom_linux.go |
| 203 | ++++ b/src/runtime/vgetrandom_linux.go |
| 204 | +@@ -73,9 +73,16 @@ |
| 205 | + return state |
| 206 | + } |
| 207 | + |
| 208 | +-func vgetrandomPutState(state uintptr) { |
| 209 | ++// Free vgetrandom state from the M (if any) prior to destroying the M. |
| 210 | ++// |
| 211 | ++// This may allocate, so it must have a P. |
| 212 | ++func vgetrandomDestroy(mp *m) { |
| 213 | ++ if mp.vgetrandomState == 0 { |
| 214 | ++ return |
| 215 | ++ } |
| 216 | ++ |
| 217 | + lock(&vgetrandomAlloc.statesLock) |
| 218 | +- vgetrandomAlloc.states = append(vgetrandomAlloc.states, state) |
| 219 | ++ vgetrandomAlloc.states = append(vgetrandomAlloc.states, mp.vgetrandomState) |
| 220 | + unlock(&vgetrandomAlloc.statesLock) |
| 221 | + } |
| 222 | + |
| 223 | +diff --git a/src/runtime/vgetrandom_unsupported.go b/src/runtime/vgetrandom_unsupported.go |
| 224 | +index 070392c..43c53e1 100644 |
| 225 | +--- a/src/runtime/vgetrandom_unsupported.go |
| 226 | ++++ b/src/runtime/vgetrandom_unsupported.go |
| 227 | +@@ -13,6 +13,6 @@ |
| 228 | + return -1, false |
| 229 | + } |
| 230 | + |
| 231 | +-func vgetrandomPutState(state uintptr) {} |
| 232 | ++func vgetrandomDestroy(mp *m) {} |
| 233 | + |
| 234 | + func vgetrandomInit() {} |
0 commit comments