Skip to content

Commit 36bae22

Browse files
committed
runtime: always acquire M when acquiring locks by rank
Profiling of runtime-internal locks checks gp.m.locks to see if it's safe to add a new record to the profile, but direct use of acquireLockRank can change the list of the M's active lock ranks without updating gp.m.locks to match. The runtime's internal rwmutex implementation makes a point of calling acquirem/releasem when manipulating the lock rank list, but the other user of acquireLockRank (the GC's Gscan bit) relied on the GC's invariants to avoid deadlocks. Codify the rwmutex approach by adding a variant of acquireLockRank, acquireLockRankAndM, include a call to aquirem. Do the same for release. Leave runtime/time.go's use of the old variants intact for the moment. For #64706 For #66004 Change-Id: Id18e4d8de1036de743d2937fad002c6feebe2faf
1 parent 34d28ba commit 36bae22

File tree

4 files changed

+57
-10
lines changed

4 files changed

+57
-10
lines changed

src/runtime/lockrank_off.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ func lockWithRank(l *mutex, rank lockRank) {
2727
// This function may be called in nosplit context and thus must be nosplit.
2828
//
2929
//go:nosplit
30+
func acquireLockRankAndM(rank lockRank) {
31+
acquirem()
32+
}
33+
34+
// This function may be called in nosplit context and thus must be nosplit.
35+
//
36+
// Deprecated: use acquireLockRankAndM instead to ensure that m.locks!=0 while
37+
// appearing (to the static lock rank checker) to hold locks.
38+
//
39+
//go:nosplit
3040
func acquireLockRank(rank lockRank) {
3141
}
3242

@@ -37,6 +47,16 @@ func unlockWithRank(l *mutex) {
3747
// This function may be called in nosplit context and thus must be nosplit.
3848
//
3949
//go:nosplit
50+
func releaseLockRankAndM(rank lockRank) {
51+
releasem(getg().m)
52+
}
53+
54+
// This function may be called in nosplit context and thus must be nosplit.
55+
//
56+
// Deprecated: use releaseLockRankAndM instead to ensure that m.locks!=0 while
57+
// appearing (to the static lock rank checker) to hold locks.
58+
//
59+
//go:nosplit
4060
func releaseLockRank(rank lockRank) {
4161
}
4262

src/runtime/lockrank_on.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,23 @@ func printHeldLocks(gp *g) {
104104
}
105105
}
106106

107+
// acquireLockRankAndM acquires a rank which is not associated with a mutex
108+
// lock. To maintain the invariant that an M with m.locks==0 does not hold any
109+
// lock-like resources, it also acquires the M.
110+
//
111+
// This function may be called in nosplit context and thus must be nosplit.
112+
//
113+
//go:nosplit
114+
func acquireLockRankAndM(rank lockRank) {
115+
acquirem()
116+
acquireLockRank(rank)
117+
}
118+
107119
// acquireLockRank acquires a rank which is not associated with a mutex lock
108120
//
121+
// Deprecated: use acquireLockRankAndM instead to ensure that m.locks!=0 while
122+
// appearing (to the static lock rank checker) to hold locks.
123+
//
109124
// This function may be called in nosplit context and thus must be nosplit.
110125
//
111126
//go:nosplit
@@ -189,8 +204,23 @@ func unlockWithRank(l *mutex) {
189204
})
190205
}
191206

207+
// releaseLockRankAndM releases a rank which is not associated with a mutex
208+
// lock. To maintain the invariant that an M with m.locks==0 does not hold any
209+
// lock-like resources, it also releases the M.
210+
//
211+
// This function may be called in nosplit context and thus must be nosplit.
212+
//
213+
//go:nosplit
214+
func releaseLockRankAndM(rank lockRank) {
215+
releaseLockRank(rank)
216+
releasem(gp.m)
217+
}
218+
192219
// releaseLockRank releases a rank which is not associated with a mutex lock
193220
//
221+
// Deprecated: use releaseLockRankAndM instead to ensure that m.locks!=0 while
222+
// appearing (to the static lock rank checker) to hold locks.
223+
//
194224
// This function may be called in nosplit context and thus must be nosplit.
195225
//
196226
//go:nosplit

src/runtime/proc.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ func casfrom_Gscanstatus(gp *g, oldval, newval uint32) {
10681068
dumpgstatus(gp)
10691069
throw("casfrom_Gscanstatus: gp->status is not in scan state")
10701070
}
1071-
releaseLockRank(lockRankGscan)
1071+
releaseLockRankAndM(lockRankGscan)
10721072
}
10731073

10741074
// This will return false if the gp is not in the expected status and the cas fails.
@@ -1082,7 +1082,7 @@ func castogscanstatus(gp *g, oldval, newval uint32) bool {
10821082
if newval == oldval|_Gscan {
10831083
r := gp.atomicstatus.CompareAndSwap(oldval, newval)
10841084
if r {
1085-
acquireLockRank(lockRankGscan)
1085+
acquireLockRankAndM(lockRankGscan)
10861086
}
10871087
return r
10881088

@@ -1111,8 +1111,8 @@ func casgstatus(gp *g, oldval, newval uint32) {
11111111
})
11121112
}
11131113

1114-
acquireLockRank(lockRankGscan)
1115-
releaseLockRank(lockRankGscan)
1114+
acquireLockRankAndM(lockRankGscan)
1115+
releaseLockRankAndM(lockRankGscan)
11161116

11171117
// See https://golang.org/cl/21503 for justification of the yield delay.
11181118
const yieldDelay = 5 * 1000
@@ -1235,7 +1235,7 @@ func casGToPreemptScan(gp *g, old, new uint32) {
12351235
if old != _Grunning || new != _Gscan|_Gpreempted {
12361236
throw("bad g transition")
12371237
}
1238-
acquireLockRank(lockRankGscan)
1238+
acquireLockRankAndM(lockRankGscan)
12391239
for !gp.atomicstatus.CompareAndSwap(_Grunning, _Gscan|_Gpreempted) {
12401240
}
12411241
}

src/runtime/rwmutex.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ func (rw *rwmutex) rlock() {
7272
// things blocking on the lock may consume all of the Ps and
7373
// deadlock (issue #20903). Alternatively, we could drop the P
7474
// while sleeping.
75-
acquirem()
76-
77-
acquireLockRank(rw.readRank)
75+
acquireLockRankAndM(rw.readRank)
7876
lockWithRankMayAcquire(&rw.rLock, getLockRank(&rw.rLock))
7977

8078
if rw.readerCount.Add(1) < 0 {
@@ -116,8 +114,7 @@ func (rw *rwmutex) runlock() {
116114
unlock(&rw.rLock)
117115
}
118116
}
119-
releaseLockRank(rw.readRank)
120-
releasem(getg().m)
117+
releaseLockRankAndM(rw.readRank)
121118
}
122119

123120
// lock locks rw for writing.

0 commit comments

Comments
 (0)