Skip to content

Commit 9b4b3e5

Browse files
prattmicprattmic
committed
runtime: properly model rwmutex in lock ranking
Currently, lock ranking doesn't really try to model rwmutex. It records the internal locks rLock and wLock, but in a subpar fashion: 1. wLock is held from lock to unlock, so it works OK, but it conflates write locks of all rwmutexes as rwmutexW, rather than allowing different rwmutexes to have different rankings. 2. rLock is an internal implementation detail that is only taken when there is contention in rlock. As as result, the reader lock path is almost never checked. Add proper modeling. rwmutexR and rwmutexW remain as the ranks of the internal locks, which have their own ordering. The new init method is passed the ranks of the higher level lock that this represents, just like lockInit for mutex. execW ordered before MALLOC captures the case from #64722. i.e., there can be allocation between BeforeFork and AfterFork. For #64722. Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking Change-Id: I23335b28faa42fb04f1bc9da02fdf54d1616cd28 Reviewed-on: https://go-review.googlesource.com/c/go/+/549536 Reviewed-by: Michael Knyszek <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 7930971 commit 9b4b3e5

6 files changed

Lines changed: 138 additions & 46 deletions

File tree

‎src/runtime/export_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,10 @@ type RWMutex struct {
586586
rw rwmutex
587587
}
588588

589+
func (rw *RWMutex) Init() {
590+
rw.rw.init(lockRankTestR, lockRankTestW)
591+
}
592+
589593
func (rw *RWMutex) RLock() {
590594
rw.rw.rlock()
591595
}

‎src/runtime/lockrank.go‎

Lines changed: 53 additions & 34 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/runtime/mklockrank.go‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,30 +52,40 @@ NONE <
5252
assistQueue,
5353
sweep;
5454
55+
# Test only
56+
NONE < testR, testW;
57+
5558
# Scheduler, timers, netpoll
56-
NONE < pollDesc, cpuprof, wakeableSleep;
59+
NONE <
60+
allocmW,
61+
execW,
62+
cpuprof,
63+
pollDesc,
64+
wakeableSleep;
5765
assistQueue,
5866
cpuprof,
5967
forcegc,
6068
pollDesc, # pollDesc can interact with timers, which can lock sched.
6169
scavenge,
6270
sweep,
6371
sweepWaiters,
72+
testR,
6473
wakeableSleep
74+
# Above SCHED are things that can call into the scheduler.
75+
< SCHED
76+
# Below SCHED is the scheduler implementation.
77+
< allocmR,
78+
execR
6579
< sched;
6680
sched < allg, allp;
6781
allp, wakeableSleep < timers;
6882
timers < netpollInit;
6983
7084
# Channels
71-
scavenge, sweep, wakeableSleep < hchan;
85+
scavenge, sweep, testR, wakeableSleep < hchan;
7286
NONE < notifyList;
7387
hchan, notifyList < sudog;
7488
75-
# RWMutex
76-
NONE < rwmutexW;
77-
rwmutexW, sysmon < rwmutexR;
78-
7989
# Semaphores
8090
NONE < root;
8191
@@ -100,6 +110,9 @@ traceBuf < traceStrings;
100110
101111
# Malloc
102112
allg,
113+
allocmR,
114+
execR, # May grow stack
115+
execW, # May allocate after BeforeFork
103116
hchan,
104117
notifyList,
105118
reflectOffs,
@@ -136,7 +149,7 @@ gcBitsArenas,
136149
< STACKGROW
137150
# Below STACKGROW is the stack allocator/copying implementation.
138151
< gscan;
139-
gscan, rwmutexR < stackpool;
152+
gscan < stackpool;
140153
gscan < stackLarge;
141154
# Generally, hchan must be acquired before gscan. But in one case,
142155
# where we suspend a G and then shrink its stack, syncadjustsudogs
@@ -189,6 +202,18 @@ NONE < panic;
189202
panic < deadlock;
190203
# raceFini is only held while exiting.
191204
panic < raceFini;
205+
206+
# RWMutex
207+
allocmW,
208+
execW,
209+
testW
210+
< rwmutexW;
211+
212+
rwmutexW,
213+
allocmR,
214+
execR,
215+
testR
216+
< rwmutexR;
192217
`
193218

194219
// cyclicRanks lists lock ranks that allow multiple locks of the same

‎src/runtime/proc.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,8 @@ func schedinit() {
759759
lockInit(&reflectOffs.lock, lockRankReflectOffs)
760760
lockInit(&finlock, lockRankFin)
761761
lockInit(&cpuprof.lock, lockRankCpuprof)
762+
allocmLock.init(lockRankAllocmR, lockRankAllocmW)
763+
execLock.init(lockRankExecR, lockRankExecW)
762764
traceLockInit()
763765
// Enforce that this lock is always a leaf lock.
764766
// All of this lock's critical sections should be

‎src/runtime/rwmutex.go‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,37 @@ type rwmutex struct {
2525

2626
readerCount atomic.Int32 // number of pending readers
2727
readerWait atomic.Int32 // number of departing readers
28+
29+
readRank lockRank // semantic lock rank for read locking
30+
writeRank lockRank // semantic lock rank for write locking
31+
}
32+
33+
// Lock ranking an rwmutex has two aspects:
34+
//
35+
// Semantic ranking: this rwmutex represents some higher level lock that
36+
// protects some resource (e.g., allocmLock protects creation of new Ms). The
37+
// read and write locks of that resource need to be represented in the lock
38+
// rank.
39+
//
40+
// Internal ranking: as an implementation detail, rwmutex uses two mutexes:
41+
// rLock and wLock. These have lock order requirements: wLock must be locked
42+
// before rLock. This also needs to be represented in the lock rank.
43+
//
44+
// Internal ranking is represented by assigning ranks rwmutexR and rwmutexW to
45+
// rLock and wLock, respectively.
46+
//
47+
// Semantic ranking is represented by acquiring readRank during read lock and
48+
// writeRank during write lock.
49+
//
50+
// readRank is always taken before rwmutexR and writeRank is always taken
51+
// before rwmutexW, so each unique rwmutex must record this order in the lock
52+
// ranking.
53+
func (rw *rwmutex) init(readRank, writeRank lockRank) {
54+
rw.readRank = readRank
55+
rw.writeRank = writeRank
56+
57+
lockInit(&rw.rLock, lockRankRwmutexR)
58+
lockInit(&rw.wLock, lockRankRwmutexW)
2859
}
2960

3061
const rwmutexMaxReaders = 1 << 30
@@ -36,10 +67,14 @@ func (rw *rwmutex) rlock() {
3667
// deadlock (issue #20903). Alternatively, we could drop the P
3768
// while sleeping.
3869
acquirem()
70+
71+
acquireLockRank(rw.readRank)
72+
lockWithRankMayAcquire(&rw.rLock, getLockRank(&rw.rLock))
73+
3974
if rw.readerCount.Add(1) < 0 {
4075
// A writer is pending. Park on the reader queue.
4176
systemstack(func() {
42-
lockWithRank(&rw.rLock, lockRankRwmutexR)
77+
lock(&rw.rLock)
4378
if rw.readerPass > 0 {
4479
// Writer finished.
4580
rw.readerPass -= 1
@@ -67,26 +102,28 @@ func (rw *rwmutex) runlock() {
67102
// A writer is pending.
68103
if rw.readerWait.Add(-1) == 0 {
69104
// The last reader unblocks the writer.
70-
lockWithRank(&rw.rLock, lockRankRwmutexR)
105+
lock(&rw.rLock)
71106
w := rw.writer.ptr()
72107
if w != nil {
73108
notewakeup(&w.park)
74109
}
75110
unlock(&rw.rLock)
76111
}
77112
}
113+
releaseLockRank(rw.readRank)
78114
releasem(getg().m)
79115
}
80116

81117
// lock locks rw for writing.
82118
func (rw *rwmutex) lock() {
83119
// Resolve competition with other writers and stick to our P.
84-
lockWithRank(&rw.wLock, lockRankRwmutexW)
120+
acquireLockRank(rw.writeRank)
121+
lock(&rw.wLock)
85122
m := getg().m
86123
// Announce that there is a pending writer.
87124
r := rw.readerCount.Add(-rwmutexMaxReaders) + rwmutexMaxReaders
88125
// Wait for any active readers to complete.
89-
lockWithRank(&rw.rLock, lockRankRwmutexR)
126+
lock(&rw.rLock)
90127
if r != 0 && rw.readerWait.Add(r) != 0 {
91128
// Wait for reader to wake us up.
92129
systemstack(func() {
@@ -108,7 +145,7 @@ func (rw *rwmutex) unlock() {
108145
throw("unlock of unlocked rwmutex")
109146
}
110147
// Unblock blocked readers.
111-
lockWithRank(&rw.rLock, lockRankRwmutexR)
148+
lock(&rw.rLock)
112149
for rw.readers.ptr() != nil {
113150
reader := rw.readers.ptr()
114151
rw.readers = reader.schedlink
@@ -122,4 +159,5 @@ func (rw *rwmutex) unlock() {
122159
unlock(&rw.rLock)
123160
// Allow other writers to proceed.
124161
unlock(&rw.wLock)
162+
releaseLockRank(rw.writeRank)
125163
}

‎src/runtime/rwmutex_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ func parallelReader(m *RWMutex, clocked chan bool, cunlock *atomic.Bool, cdone c
2929
func doTestParallelReaders(numReaders int) {
3030
GOMAXPROCS(numReaders + 1)
3131
var m RWMutex
32+
m.Init()
3233
clocked := make(chan bool, numReaders)
3334
var cunlock atomic.Bool
3435
cdone := make(chan bool)
@@ -100,6 +101,7 @@ func HammerRWMutex(gomaxprocs, numReaders, num_iterations int) {
100101
// Number of active readers + 10000 * number of active writers.
101102
var activity int32
102103
var rwm RWMutex
104+
rwm.Init()
103105
cdone := make(chan bool)
104106
go writer(&rwm, num_iterations, &activity, cdone)
105107
var i int
@@ -141,6 +143,7 @@ func BenchmarkRWMutexUncontended(b *testing.B) {
141143
}
142144
b.RunParallel(func(pb *testing.PB) {
143145
var rwm PaddedRWMutex
146+
rwm.Init()
144147
for pb.Next() {
145148
rwm.RLock()
146149
rwm.RLock()
@@ -154,6 +157,7 @@ func BenchmarkRWMutexUncontended(b *testing.B) {
154157

155158
func benchmarkRWMutex(b *testing.B, localWork, writeRatio int) {
156159
var rwm RWMutex
160+
rwm.Init()
157161
b.RunParallel(func(pb *testing.PB) {
158162
foo := 0
159163
for pb.Next() {

0 commit comments

Comments
 (0)