runtime: check partial lock ranking order

To ease readability we typically keep the partial order lists sorted by
rank. This isn't required for correctness, it just makes it (slightly)
easier to read the lists.

Currently we must notice out-of-order entries during code review, which
is an error-prone process.

Add a test to enforce ordering, and fix the errors that have crept in.
Most of the existing errors were misordered lockRankHchan or
lockRankPollDesc.

While we're here, I've moved the correctness check that the partial
ordering satisfies the total ordering from init to a test case. This
will allow us to catch these errors without even running
staticlockranking.

Change-Id: I9c11abe49ea26c556439822bb6a3183129600c3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/300171
Trust: Michael Pratt <mpratt@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>

diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index a48bb2636f7dd51aa2088e02bfff1ed7d179a4cd..5a87024b8a34671b08af444852296e4a0b1a1568 100644 (file)
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -46,6 +46,14 @@ var NetpollGenericInit = netpollGenericInit
 var Memmove = memmove
 var MemclrNoHeapPointers = memclrNoHeapPointers
 
+var LockPartialOrder = lockPartialOrder
+
+type LockRank lockRank
+
+func (l LockRank) String() string {
+       return lockRank(l).String()
+}
+
 const PreemptMSupported = preemptMSupported
 
 type LFNode struct {

diff --git a/src/runtime/lockrank.go b/src/runtime/lockrank.go
index 5a908b470f29991058d5bd34ed894c4d4ed82598..b600c2132b6f6f557f228bda9a591308328e7e24 100644 (file)
--- a/src/runtime/lockrank.go
+++ b/src/runtime/lockrank.go
@@ -204,7 +204,7 @@ var lockPartialOrder [][]lockRank = [][]lockRank{
        lockRankDeadlock:      {lockRankDeadlock},
        lockRankAllg:          {lockRankSysmon, lockRankSched},
        lockRankAllp:          {lockRankSysmon, lockRankSched},
-       lockRankTimers:        {lockRankSysmon, lockRankScavenge, lockRankSched, lockRankAllp, lockRankPollDesc, lockRankTimers},
+       lockRankTimers:        {lockRankSysmon, lockRankScavenge, lockRankPollDesc, lockRankSched, lockRankAllp, lockRankTimers},
        lockRankItab:          {},
        lockRankReflectOffs:   {lockRankItab},
        lockRankHchan:         {lockRankScavenge, lockRankSweep, lockRankHchan},
@@ -213,25 +213,25 @@ var lockPartialOrder [][]lockRank = [][]lockRank{
        lockRankTraceBuf:      {lockRankSysmon, lockRankScavenge},
        lockRankTraceStrings:  {lockRankTraceBuf},
        lockRankMspanSpecial:  {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings},
-       lockRankProf:          {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan},
-       lockRankGcBitsArenas:  {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan},
+       lockRankProf:          {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings},
+       lockRankGcBitsArenas:  {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings},
        lockRankRoot:          {},
-       lockRankTrace:         {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankSched, lockRankHchan, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankSweep},
+       lockRankTrace:         {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankSweep, lockRankSched, lockRankHchan, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot},
        lockRankTraceStackTab: {lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankSched, lockRankAllg, lockRankTimers, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankTrace},
        lockRankNetpollInit:   {lockRankTimers},
 
        lockRankRwmutexW: {},
        lockRankRwmutexR: {lockRankSysmon, lockRankRwmutexW},
 
-       lockRankSpanSetSpine: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan},
-       lockRankGscan:        {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankTraceBuf, lockRankTraceStrings, lockRankRoot, lockRankNotifyList, lockRankProf, lockRankGcBitsArenas, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankSpanSetSpine},
-       lockRankStackpool:    {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankSched, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankRwmutexR, lockRankSpanSetSpine, lockRankGscan},
+       lockRankSpanSetSpine: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings},
+       lockRankGscan:        {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankSpanSetSpine},
+       lockRankStackpool:    {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankTrace, lockRankTraceStackTab, lockRankNetpollInit, lockRankRwmutexR, lockRankSpanSetSpine, lockRankGscan},
        lockRankStackLarge:   {lockRankSysmon, lockRankAssistQueue, lockRankSched, lockRankItab, lockRankHchan, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankSpanSetSpine, lockRankGscan},
        lockRankDefer:        {},
-       lockRankSudog:        {lockRankNotifyList, lockRankHchan},
-       lockRankWbufSpans:    {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankSched, lockRankAllg, lockRankPollDesc, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceStrings, lockRankMspanSpecial, lockRankProf, lockRankRoot, lockRankGscan, lockRankDefer, lockRankSudog},
-       lockRankMheap:        {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankFin, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan, lockRankMspanSpecial, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankGscan, lockRankStackpool, lockRankStackLarge, lockRankDefer, lockRankSudog, lockRankWbufSpans, lockRankSpanSetSpine},
-       lockRankMheapSpecial: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankHchan},
+       lockRankSudog:        {lockRankHchan, lockRankNotifyList},
+       lockRankWbufSpans:    {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceStrings, lockRankMspanSpecial, lockRankProf, lockRankRoot, lockRankGscan, lockRankDefer, lockRankSudog},
+       lockRankMheap:        {lockRankSysmon, lockRankScavenge, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankFin, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings, lockRankMspanSpecial, lockRankProf, lockRankGcBitsArenas, lockRankRoot, lockRankSpanSetSpine, lockRankGscan, lockRankStackpool, lockRankStackLarge, lockRankDefer, lockRankSudog, lockRankWbufSpans},
+       lockRankMheapSpecial: {lockRankSysmon, lockRankScavenge, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankItab, lockRankReflectOffs, lockRankHchan, lockRankNotifyList, lockRankTraceBuf, lockRankTraceStrings},
        lockRankGlobalAlloc:  {lockRankProf, lockRankSpanSetSpine, lockRankMheap, lockRankMheapSpecial},
 
        lockRankGFree:     {lockRankSched},

diff --git a/src/runtime/lockrank_on.go b/src/runtime/lockrank_on.go
index 3958d9eeaa92af6822688a2a816605ca4af1da94..fc8d2dc8d1179fd1813561ff0a6283550b7a0bfe 100644 (file)
--- a/src/runtime/lockrank_on.go
+++ b/src/runtime/lockrank_on.go
@@ -25,19 +25,6 @@ type lockRankStruct struct {
        pad int
 }
 
-// init checks that the partial order in lockPartialOrder fits within the total
-// order determined by the order of the lockRank constants.
-func init() {
-       for rank, list := range lockPartialOrder {
-               for _, entry := range list {
-                       if entry > lockRank(rank) {
-                               println("lockPartial order row", lockRank(rank).String(), "entry", entry.String())
-                               throw("lockPartialOrder table is inconsistent with total lock ranking order")
-                       }
-               }
-       }
-}
-
 func lockInit(l *mutex, rank lockRank) {
        l.rank = rank
 }

diff --git a/src/runtime/lockrank_test.go b/src/runtime/lockrank_test.go
new file mode 100644 (file)
index 0000000..4b2fc0e
--- /dev/null
+++ b/src/runtime/lockrank_test.go
@@ -0,0 +1,41 @@
+// Copyright 2021 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.
+
+package runtime_test
+
+import (
+       . "runtime"
+       "testing"
+)
+
+// Check that the partial order in lockPartialOrder fits within the total order
+// determined by the order of the lockRank constants.
+func TestLockRankPartialOrder(t *testing.T) {
+       for r, list := range LockPartialOrder {
+               rank := LockRank(r)
+               for _, e := range list {
+                       entry := LockRank(e)
+                       if entry > rank {
+                               t.Errorf("lockPartialOrder row %v entry %v is inconsistent with total lock ranking order", rank, entry)
+                       }
+               }
+       }
+}
+
+// Verify that partial order lists are kept sorted. This is a purely cosemetic
+// check to make manual reviews simpler. It does not affect correctness, unlike
+// the above test.
+func TestLockRankPartialOrderSortedEntries(t *testing.T) {
+       for r, list := range LockPartialOrder {
+               rank := LockRank(r)
+               var prev LockRank
+               for _, e := range list {
+                       entry := LockRank(e)
+                       if entry <= prev {
+                               t.Errorf("Partial order for rank %v out of order: %v <= %v in %v", rank, entry, prev, list)
+                       }
+                       prev = entry
+               }
+       }
+}