Lockdep: Print more info in the "unbalanced unlock" case.

This CL does the following:

- Add the ability for nested locks to have names.
- Give names to all current uses of nested locks in the codebase.
- Truncate `lockdep` debug stack traces to avoid the clutter from the
  `lockdep` code itself
- Simplify `lockdep` to not longer require `classMap`.

PiperOrigin-RevId: 491486620
This commit is contained in:
Etienne Perot
2022-11-28 17:53:09 -08:00
committed by gVisor bot
parent ae731e0394
commit 445fa6f40c
27 changed files with 327 additions and 213 deletions
+17
View File
@@ -38,6 +38,7 @@ import (
"io"
stdlog "log"
"os"
"regexp"
"runtime"
"sync/atomic"
"time"
@@ -325,6 +326,22 @@ func Stacks(all bool) []byte {
return trace
}
// stackRegexp matches one level within a stack trace.
var stackRegexp = regexp.MustCompile("(?m)^\\S+\\(.*\\)$\\r?\\n^\\t\\S+:\\d+.*$\\r?\\n")
// LocalStack returns the local goroutine stack, excluding the top N entries.
// LocalStack's own entry is excluded by default and does not need to be counted in excludeTopN.
func LocalStack(excludeTopN int) []byte {
replaceNext := excludeTopN + 1
return stackRegexp.ReplaceAllFunc(Stacks(false), func(s []byte) []byte {
if replaceNext > 0 {
replaceNext--
return nil
}
return s
})
}
// Traceback logs the given message and dumps a stacktrace of the current
// goroutine.
//
+5
View File
@@ -7,6 +7,11 @@ licenses(["notice"])
declare_mutex(
name = "dir_mutex",
out = "dir_mutex.go",
nested_lock_names = [
"new",
"replaced",
"child",
],
package = "overlay",
prefix = "dir",
)
+6 -6
View File
@@ -1137,8 +1137,8 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if err := newParent.checkPermissions(creds, vfs.MayWrite|vfs.MayExec); err != nil {
return err
}
newParent.dirMu.NestedLock()
defer newParent.dirMu.NestedUnlock()
newParent.dirMu.NestedLock(dirLockNew)
defer newParent.dirMu.NestedUnlock(dirLockNew)
}
if newParent.vfsd.IsDead() {
return linuxerr.ENOENT
@@ -1165,8 +1165,8 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if genericIsAncestorDentry(replaced, renamed) {
return linuxerr.ENOTEMPTY
}
replaced.dirMu.NestedLock()
defer replaced.dirMu.NestedUnlock()
replaced.dirMu.NestedLock(dirLockReplaced)
defer replaced.dirMu.NestedUnlock(dirLockReplaced)
whiteouts, err = replaced.collectWhiteoutsForRmdirLocked(ctx)
if err != nil {
return err
@@ -1376,8 +1376,8 @@ func (fs *filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error
if err := parent.mayDelete(rp.Credentials(), child); err != nil {
return err
}
child.dirMu.NestedLock()
defer child.dirMu.NestedUnlock()
child.dirMu.NestedLock(dirLockChild)
defer child.dirMu.NestedUnlock(dirLockChild)
whiteouts, err := child.collectWhiteoutsForRmdirLocked(ctx)
if err != nil {
return err
+2
View File
@@ -28,6 +28,7 @@ declare_rwmutex(
declare_mutex(
name = "task_mutex",
out = "task_mutex.go",
nested_lock_names = ["child"],
package = "kernel",
prefix = "task",
)
@@ -56,6 +57,7 @@ declare_mutex(
declare_mutex(
name = "signal_handlers_mutex",
out = "signal_handlers_mutex.go",
nested_lock_names = ["tg"],
package = "kernel",
prefix = "signalHandlers",
)
+3 -5
View File
@@ -1,5 +1,6 @@
load("//tools:defs.bzl", "go_library")
load("//tools/go_generics:defs.bzl", "go_template_instance")
load("//pkg/sync/locking:locking.bzl", "declare_mutex")
package(licenses = ["notice"])
@@ -42,15 +43,12 @@ go_template_instance(
},
)
go_template_instance(
declare_mutex(
name = "user_namespace_mutex",
out = "user_namespace_mutex.go",
nested_lock_names = ["ns"],
package = "auth",
prefix = "userNamespace",
substrs = {
"genericMark": "userNamespace",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_library(
+2 -2
View File
@@ -69,8 +69,8 @@ func (ns *UserNamespace) mapID(m *idMapSet, id uint32) uint32 {
//
// Preconditions: end >= start.
func (ns *UserNamespace) allIDsMapped(m *idMapSet, start, end uint32) bool {
ns.mu.NestedLock()
defer ns.mu.NestedUnlock()
ns.mu.NestedLock(userNamespaceLockNs)
defer ns.mu.NestedUnlock(userNamespaceLockNs)
return m.SpanRange(idMapRange{start, end}) == end-start
}
+5 -5
View File
@@ -1,17 +1,17 @@
load("//tools:defs.bzl", "go_library", "go_test")
load("//tools/go_generics:defs.bzl", "go_template_instance")
load("//pkg/sync/locking:locking.bzl", "declare_mutex")
package(licenses = ["notice"])
go_template_instance(
declare_mutex(
name = "futex_mutex",
out = "futex_mutex.go",
nested_lock_names = [
"b",
],
package = "futex",
prefix = "futexBucket",
substrs = {
"genericMark": "futexBucket",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
+32 -21
View File
@@ -408,9 +408,12 @@ func (m *Manager) lockBucket(k *Key) (b *bucket) {
}
// lockBuckets returns locked buckets for the given keys.
// +checklocksacquire:b1.mu
// +checklocksacquire:b2.mu
func (m *Manager) lockBuckets(k1, k2 *Key) (b1 *bucket, b2 *bucket) {
// It returns which bucket was locked first and second. They may be nil in case the buckets are
// identical or they did not need locking.
//
// +checklocksacquire:lockedFirst.mu
// +checklocksacquire:lockedSecond.mu
func (m *Manager) lockBuckets(k1, k2 *Key) (b1, b2, lockedFirst, lockedSecond *bucket) {
// Buckets must be consistently ordered to avoid circular lock
// dependencies. We order buckets in m.privateBuckets by index (lowest
// index first), and all buckets in m.privateBuckets precede
@@ -425,14 +428,16 @@ func (m *Manager) lockBuckets(k1, k2 *Key) (b1 *bucket, b2 *bucket) {
switch {
case i1 < i2:
b1.mu.Lock()
b2.mu.NestedLock()
b2.mu.NestedLock(futexBucketLockB)
return b1, b2, b1, b2
case i2 < i1:
b2.mu.Lock()
b1.mu.NestedLock()
b1.mu.NestedLock(futexBucketLockB)
return b1, b2, b2, b1
default:
b1.mu.Lock()
return b1, b2, b1, nil // +checklocksforce
}
return b1, b2 // +checklocksforce
}
// At least one of b1 or b2 should be m.sharedBucket.
@@ -440,22 +445,28 @@ func (m *Manager) lockBuckets(k1, k2 *Key) (b1 *bucket, b2 *bucket) {
b2 = m.sharedBucket
if k1.Kind != KindSharedMappable {
b1 = m.lockBucket(k1)
} else if k2.Kind != KindSharedMappable {
b2 = m.lockBucket(k2)
b2.mu.NestedLock(futexBucketLockB)
return b1, b2, b1, b2
}
m.sharedBucket.mu.Lock()
return b1, b2 // +checklocksforce
if k2.Kind != KindSharedMappable {
b2 = m.lockBucket(k2)
b1.mu.NestedLock(futexBucketLockB)
return b1, b2, b2, b1
}
return b1, b2, nil, nil // +checklocksforce
}
// unlockBuckets unlocks two buckets.
// +checklocksrelease:b1.mu
// +checklocksrelease:b2.mu
func (m *Manager) unlockBuckets(b1, b2 *bucket) {
b1.mu.NestedUnlock()
if b1 != b2 {
b2.mu.Unlock()
// +checklocksrelease:lockedFirst.mu
// +checklocksrelease:lockedSecond.mu
func (m *Manager) unlockBuckets(lockedFirst, lockedSecond *bucket) {
if lockedSecond != nil {
lockedSecond.mu.NestedUnlock(futexBucketLockB)
}
return // +checklocksforce
if lockedFirst != nil && lockedFirst != lockedSecond {
lockedFirst.mu.Unlock()
}
return
}
// Wake wakes up to n waiters matching the bitmask on the given addr.
@@ -487,8 +498,8 @@ func (m *Manager) doRequeue(t Target, addr, naddr hostarch.Addr, private bool, c
}
defer k2.release(t)
b1, b2 := m.lockBuckets(&k1, &k2)
defer m.unlockBuckets(b1, b2)
b1, b2, lockedFirst, lockedSecond := m.lockBuckets(&k1, &k2)
defer m.unlockBuckets(lockedFirst, lockedSecond)
if checkval {
if err := check(t, addr, val); err != nil {
@@ -534,8 +545,8 @@ func (m *Manager) WakeOp(t Target, addr1, addr2 hostarch.Addr, private bool, nwa
}
defer k2.release(t)
b1, b2 := m.lockBuckets(&k1, &k2)
defer m.unlockBuckets(b1, b2)
b1, b2, lockedFirst, lockedSecond := m.lockBuckets(&k1, &k2)
defer m.unlockBuckets(lockedFirst, lockedSecond)
done := 0
cond, err := atomicOp(t, addr2, op)
+5 -16
View File
@@ -1,39 +1,28 @@
load("//tools:defs.bzl", "go_library", "go_test")
load("//tools/go_generics:defs.bzl", "go_template_instance")
load("//pkg/sync/locking:locking.bzl", "declare_mutex")
package(licenses = ["notice"])
go_template_instance(
declare_mutex(
name = "vfs_mutex",
out = "vfs_mutex.go",
package = "pipe",
prefix = "vfs",
substrs = {
"genericMark": "vfs",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
declare_mutex(
name = "pipe_mutex",
out = "pipe_mutex.go",
nested_lock_names = ["pipe"],
package = "pipe",
prefix = "pipe",
substrs = {
"genericMark": "pipe",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
declare_mutex(
name = "inode_mutex",
out = "inode_mutex.go",
package = "pipe",
prefix = "inode",
substrs = {
"genericMark": "inode",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_library(
+9 -5
View File
@@ -22,16 +22,20 @@ import (
// consistent for both lockTwoPipes(x, y) and lockTwoPipes(y, x), such that
// concurrent calls cannot deadlock.
//
// Returns the two pipes in order (first locked pipe, second locked pipe).
// The caller should unlock the second pipe first.
//
// Preconditions: x != y.
// +checklocksacquire:x.mu
// +checklocksacquire:y.mu
func lockTwoPipes(x, y *Pipe) {
func lockTwoPipes(x, y *Pipe) (*Pipe, *Pipe) {
// Lock the two pipes in order of increasing address.
if uintptr(unsafe.Pointer(x)) < uintptr(unsafe.Pointer(y)) {
x.mu.Lock()
y.mu.NestedLock()
} else {
y.mu.Lock()
x.mu.NestedLock()
y.mu.NestedLock(pipeLockPipe)
return x, y
}
y.mu.Lock()
x.mu.NestedLock(pipeLockPipe)
return y, x
}
+3 -3
View File
@@ -404,7 +404,7 @@ func spliceOrTee(ctx context.Context, dst, src *VFSPipeFD, count int64, removeFr
return 0, linuxerr.EINVAL
}
lockTwoPipes(dst.pipe, src.pipe)
firstLocked, secondLocked := lockTwoPipes(dst.pipe, src.pipe)
n, err := dst.pipe.writeLocked(count, func(dsts safemem.BlockSeq) (uint64, error) {
n, err := src.pipe.peekLocked(int64(dsts.NumBytes()), func(srcs safemem.BlockSeq) (uint64, error) {
return safemem.CopySeq(dsts, srcs)
@@ -414,8 +414,8 @@ func spliceOrTee(ctx context.Context, dst, src *VFSPipeFD, count int64, removeFr
}
return uint64(n), err
})
dst.pipe.mu.Unlock()
src.pipe.mu.NestedUnlock()
secondLocked.mu.NestedUnlock(pipeLockPipe)
firstLocked.mu.Unlock()
if n > 0 {
dst.pipe.queue.Notify(waiter.ReadableEvents)
+4 -4
View File
@@ -202,11 +202,11 @@ func (pg *ProcessGroup) handleOrphan() {
if tg.processGroup != pg {
return
}
tg.signalHandlers.mu.NestedLock()
tg.signalHandlers.mu.NestedLock(signalHandlersLockTg)
if tg.groupStopComplete {
hasStopped = true
}
tg.signalHandlers.mu.NestedUnlock()
tg.signalHandlers.mu.NestedUnlock(signalHandlersLockTg)
})
if !hasStopped {
return
@@ -217,10 +217,10 @@ func (pg *ProcessGroup) handleOrphan() {
if tg.processGroup != pg {
return
}
tg.signalHandlers.mu.NestedLock()
tg.signalHandlers.mu.NestedLock(signalHandlersLockTg)
tg.leader.sendSignalLocked(SignalInfoPriv(linux.SIGHUP), true /* group */)
tg.leader.sendSignalLocked(SignalInfoPriv(linux.SIGCONT), true /* group */)
tg.signalHandlers.mu.NestedUnlock()
tg.signalHandlers.mu.NestedUnlock(signalHandlersLockTg)
})
return
+2 -2
View File
@@ -38,8 +38,8 @@ func (t *Task) EnterInitialCgroups(parent *Task) {
}
joinSet := t.k.cgroupRegistry.computeInitialGroups(inherit)
t.mu.NestedLock()
defer t.mu.NestedUnlock()
t.mu.NestedLock(taskLockChild)
defer t.mu.NestedUnlock(taskLockChild)
// Transfer ownership of joinSet refs to the task's cgset.
t.cgroups = joinSet
for c := range t.cgroups {
+2 -2
View File
@@ -368,9 +368,9 @@ func (tg *ThreadGroup) SetControllingTTY(tty *TTY, steal bool, isReadable bool)
// the same session as the tty's controlling thread
// group.
if othertg.processGroup.session == tty.tg.processGroup.session {
othertg.signalHandlers.mu.NestedLock()
othertg.signalHandlers.mu.NestedLock(signalHandlersLockTg)
othertg.tty = nil
othertg.signalHandlers.mu.NestedUnlock()
othertg.signalHandlers.mu.NestedUnlock(signalHandlersLockTg)
}
}
}
+1
View File
@@ -28,6 +28,7 @@ declare_rwmutex(
declare_rwmutex(
name = "active_mutex",
out = "active_mutex.go",
nested_lock_names = ["forked"],
package = "mm",
prefix = "active",
)
+4 -4
View File
@@ -139,10 +139,10 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
// regenerated by calling memmap.Mappable.Translate is a waste of time.
// (Linux does the same; compare kernel/fork.c:dup_mmap() =>
// mm/memory.c:copy_page_range().)
mm2.activeMu.Lock()
defer mm2.activeMu.Unlock()
mm.activeMu.NestedLock()
defer mm.activeMu.NestedUnlock()
mm.activeMu.Lock()
defer mm.activeMu.Unlock()
mm2.activeMu.NestedLock(activeLockForked)
defer mm2.activeMu.NestedUnlock(activeLockForked)
if dontforks {
defer mm.pmas.MergeRange(mm.applicationAddrRange())
}
+8 -15
View File
@@ -1,39 +1,32 @@
load("//tools:defs.bzl", "go_library")
load("//pkg/sync/locking:locking.bzl", "declare_mutex")
load("//tools/go_generics:defs.bzl", "go_template_instance")
package(licenses = ["notice"])
go_template_instance(
declare_mutex(
name = "queue_mutex",
out = "queue_mutex.go",
package = "transport",
prefix = "queue",
substrs = {
"genericMark": "unixQueue",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
declare_mutex(
name = "stream_queue_receiver_mutex",
out = "stream_queue_receiver_mutex.go",
package = "transport",
prefix = "streamQueueReceiver",
substrs = {
"genericMark": "streamQueueReceiver",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
declare_mutex(
name = "endpoint_mutex",
out = "endpoint_mutex.go",
nested_lock_names = [
"e",
"ce",
],
package = "transport",
prefix = "endpoint",
substrs = {
"genericMark": "unixEndpoint",
},
template = "//pkg/sync/locking:generic_mutex",
)
go_template_instance(
@@ -28,8 +28,8 @@ import (
type locker interface {
Lock()
Unlock()
NestedLock()
NestedUnlock()
NestedLock(endpointlockNameIndex)
NestedUnlock(endpointlockNameIndex)
}
// A ConnectingEndpoint is a connectioned unix endpoint that is attempting to
@@ -288,27 +288,27 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn
// Do a dance to safely acquire locks on both endpoints.
if e.id < ce.ID() {
e.Lock()
ce.NestedLock()
ce.NestedLock(endpointLockCe)
} else {
ce.Lock()
e.NestedLock()
e.NestedLock(endpointLockE)
}
// Check connecting state.
if ce.Connected() {
e.NestedUnlock()
e.NestedUnlock(endpointLockE)
ce.Unlock()
return syserr.ErrAlreadyConnected
}
if ce.ListeningLocked() {
e.NestedUnlock()
e.NestedUnlock(endpointLockE)
ce.Unlock()
return syserr.ErrInvalidEndpointState
}
// Check bound state.
if !e.ListeningLocked() {
e.NestedUnlock()
e.NestedUnlock(endpointLockE)
ce.Unlock()
return syserr.ErrConnectionRefused
}
@@ -359,7 +359,7 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn
}
// Notify can deadlock if we are holding these locks.
e.NestedUnlock()
e.NestedUnlock(endpointLockE)
ce.Unlock()
// Notify on both ends.
@@ -369,7 +369,7 @@ func (e *connectionedEndpoint) BidirectionalConnect(ctx context.Context, ce Conn
return nil
default:
// Busy; return EAGAIN per spec.
e.NestedUnlock()
e.NestedUnlock(endpointLockE)
ce.Unlock()
ne.Close(ctx)
return syserr.ErrTryAgain
+4 -32
View File
@@ -11,9 +11,7 @@ go_library(
name = "locking",
srcs = [
"atomicptrmap_ancestors_unsafe.go",
"atomicptrmap_class_unsafe.go",
"atomicptrmap_goroutine_unsafe.go",
"atomicptrmap_subclass_unsafe.go",
"lockdep.go",
"lockdep_norace.go",
"locking.go",
@@ -41,36 +39,6 @@ go_template_instance(
},
)
go_template_instance(
name = "atomicptrmap_class",
out = "atomicptrmap_class_unsafe.go",
imports = {
"reflect": "reflect",
},
package = "locking",
prefix = "class",
template = "//pkg/sync/atomicptrmap:generic_atomicptrmap",
types = {
"Key": "*MutexClass",
"Value": "reflect.Type",
},
)
go_template_instance(
name = "atomicptrmap_subclass",
out = "atomicptrmap_subclass_unsafe.go",
imports = {
"reflect": "reflect",
},
package = "locking",
prefix = "subclass",
template = "//pkg/sync/atomicptrmap:generic_atomicptrmap",
types = {
"Key": "uint32",
"Value": "MutexClass",
},
)
go_template_instance(
name = "atomicptrmap_ancestors",
out = "atomicptrmap_ancestors_unsafe.go",
@@ -101,6 +69,10 @@ go_template(
declare_mutex(
name = "mutex_test",
out = "mutex_test.go",
nested_lock_names = [
"m2",
"m3",
],
package = "locking_test",
prefix = "test",
)
+25 -8
View File
@@ -26,36 +26,53 @@ type Mutex struct {
mu sync.Mutex
}
var genericMarkIndex *locking.MutexClass
// lockNames is a list of user-friendly lock names.
// Populated in init.
var lockNames []string
// lockNameIndex is used as an index passed to NestedLock and NestedUnlock,
// refering to an index within lockNames.
// Values are specified using the "consts" field of go_template_instance.
type lockNameIndex int
// DO NOT REMOVE: The following function automatically replaced with lock index constants.
// LOCK_NAME_INDEX_CONSTANTS
const ()
// Lock locks m.
// +checklocksignore
func (m *Mutex) Lock() {
locking.AddGLock(genericMarkIndex, 0)
locking.AddGLock(genericMarkIndex, -1)
m.mu.Lock()
}
// NestedLock locks m knowing that another lock of the same type is held.
// +checklocksignore
func (m *Mutex) NestedLock() {
locking.AddGLock(genericMarkIndex, 1)
func (m *Mutex) NestedLock(i lockNameIndex) {
locking.AddGLock(genericMarkIndex, int(i))
m.mu.Lock()
}
// Unlock unlocks m.
// +checklocksignore
func (m *Mutex) Unlock() {
locking.DelGLock(genericMarkIndex, 0)
locking.DelGLock(genericMarkIndex, -1)
m.mu.Unlock()
}
// NestedUnlock unlocks m knowing that another lock of the same type is held.
// +checklocksignore
func (m *Mutex) NestedUnlock() {
locking.DelGLock(genericMarkIndex, 1)
func (m *Mutex) NestedUnlock(i lockNameIndex) {
locking.DelGLock(genericMarkIndex, int(i))
m.mu.Unlock()
}
var genericMarkIndex *locking.MutexClass
// DO NOT REMOVE: The following function is automatically replaced.
func initLockNames() {}
func init() {
genericMarkIndex = locking.NewMutexClass(reflect.TypeOf(Mutex{}))
initLockNames()
genericMarkIndex = locking.NewMutexClass(reflect.TypeOf(Mutex{}), lockNames)
}

Some files were not shown because too many files have changed in this diff Show More