diff options
author | Michael Pratt <mpratt@google.com> | 2020-12-07 09:21:33 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-12-07 09:23:17 -0800 |
commit | b28dc25aea0bddfca1481cebe24ad3aa71930844 (patch) | |
tree | c6f43b5d37142708d319fa91095b8dd807b0cf73 /pkg/p9/server.go | |
parent | eeb23531ebef4fc44af317b4e4a8834c8b069dd9 (diff) |
Remove p9.fidRef.openedMu
openedMu has lock ordering violations. Most locks go through OpenedFlag(),
which is usually taken after renameMu and opMu. On the other hand, Tlopen takes
openedMu before renameMu and opMu (via safelyRead).
Resolving this violation is simple: just drop openedMu. The opened and
openFlags fields are already protected by opMu in most cases, renameMu (for
write) in one case (via safelyGlobal), and only in doWalk by neither.
This is a bit ugly because opMu is supposed to be a "semantic" lock, but it
works. I'm open to other suggestions.
Note that doWalk has a race condition where a FID may open after the open check
but before actually walking. This race existed before this change as well; it
is not clear if it is problematic.
PiperOrigin-RevId: 346108483
Diffstat (limited to 'pkg/p9/server.go')
-rw-r--r-- | pkg/p9/server.go | 14 |
1 files changed, 4 insertions, 10 deletions
diff --git a/pkg/p9/server.go b/pkg/p9/server.go index 3736f12a3..8c5c434fd 100644 --- a/pkg/p9/server.go +++ b/pkg/p9/server.go @@ -134,12 +134,11 @@ type fidRef struct { // The node above will be closed only when refs reaches zero. refs int64 - // openedMu protects opened and openFlags. - openedMu sync.Mutex - // opened indicates whether this has been opened already. // // This is updated in handlers.go. + // + // opened is protected by pathNode.opMu or renameMu (for write). opened bool // mode is the fidRef's mode from the walk. Only the type bits are @@ -151,6 +150,8 @@ type fidRef struct { // openFlags is the mode used in the open. // // This is updated in handlers.go. + // + // openFlags is protected by pathNode.opMu or renameMu (for write). openFlags OpenFlags // pathNode is the current pathNode for this FID. @@ -177,13 +178,6 @@ type fidRef struct { deleted uint32 } -// OpenFlags returns the flags the file was opened with and true iff the fid was opened previously. -func (f *fidRef) OpenFlags() (OpenFlags, bool) { - f.openedMu.Lock() - defer f.openedMu.Unlock() - return f.openFlags, f.opened -} - // IncRef increases the references on a fid. func (f *fidRef) IncRef() { atomic.AddInt64(&f.refs, 1) |