Merge c0317b28 (automated)

6 files changed, 49 insertions, 43 deletions

diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go
index 1a976bd13..9e622227b 100644
--- a/pkg/p9/handlers.go
+++ b/pkg/p9/handlers.go
@@ -545,7 +545,7 @@ func (t *Tunlinkat) handle(cs *connState) message {
 		// Before we do the unlink itself, we need to ensure that there
 		// are no operations in flight on associated path node. The
 		// child's path node lock must be held to ensure that the
-		// unlink at marking the child deleted below is atomic with
+		// unlinkat marking the child deleted below is atomic with
 		// respect to any other read or write operations.
 		//
 		// This is one case where we have a lock ordering issue, but
diff --git a/pkg/p9/path_tree.go b/pkg/p9/path_tree.go
index f37ad4ab2..5a209b291 100644
--- a/pkg/p9/path_tree.go
+++ b/pkg/p9/path_tree.go
@@ -22,39 +22,44 @@ import (
 // pathNode is a single node in a path traversal.
 //
 // These are shared by all fidRefs that point to the same path.
-//
-// These are not synchronized because we allow certain operations (file walk)
-// to proceed without having to acquire a write lock. The lock in this
-// structure exists to synchronize high-level, semantic operations, such as the
-// simultaneous creation and deletion of a file.
-//
-// (+) below is the path component string.
 type pathNode struct {
-	mu       sync.RWMutex // See above.
-	fidRefs  sync.Map     // => map[*fidRef]string(+)
-	children sync.Map     // => map[string(+)]*pathNode
-	count    int64
+	// fidRefName is a map[*fidRef]string mapping child fidRefs to their
+	// path component name.
+	fidRefNames sync.Map
+
+	// childNodes is a map[string]*pathNode mapping child path component
+	// names to their pathNode.
+	childNodes sync.Map
+
+	// mu does *not* protect the fields above.
+	//
+	// They are not synchronized because we allow certain operations (file
+	// walk) to proceed without having to acquire a write lock. mu exists
+	// to synchronize high-level, semantic operations, such as the
+	// simultaneous creation and deletion of a file.
+	mu sync.RWMutex
 }
 
 // pathNodeFor returns the path node for the given name, or a new one.
 //
-// Precondition: mu must be held in a readable fashion.
+// Precondition: This call is synchronized w.r.t. other adding or removing of
+// children.
 func (p *pathNode) pathNodeFor(name string) *pathNode {
 	// Load the existing path node.
-	if pn, ok := p.children.Load(name); ok {
+	if pn, ok := p.childNodes.Load(name); ok {
 		return pn.(*pathNode)
 	}
 
 	// Create a new pathNode for shared use.
-	pn, _ := p.children.LoadOrStore(name, new(pathNode))
+	pn, _ := p.childNodes.LoadOrStore(name, new(pathNode))
 	return pn.(*pathNode)
 }
 
 // nameFor returns the name for the given fidRef.
 //
-// Precondition: mu must be held in a readable fashion.
+// Precondition: addChild is called for ref before nameFor.
 func (p *pathNode) nameFor(ref *fidRef) string {
-	if s, ok := p.fidRefs.Load(ref); ok {
+	if s, ok := p.fidRefNames.Load(ref); ok {
 		return s.(string)
 	}
 
@@ -62,27 +67,26 @@ func (p *pathNode) nameFor(ref *fidRef) string {
 	panic(fmt.Sprintf("expected name for %+v, none found", ref))
 }
 
-// addChild adds a child to the given pathNode.
+// addChild adds a child to p.
 //
 // This applies only to an individual fidRef.
 //
-// Precondition: mu must be held in a writable fashion.
+// Precondition: ref is added only once unless it is removed before adding with
+// a new name.
 func (p *pathNode) addChild(ref *fidRef, name string) {
-	if s, ok := p.fidRefs.Load(ref); ok {
+	if s, ok := p.fidRefNames.Load(ref); ok {
 		// This should not happen, don't proceed.
 		panic(fmt.Sprintf("unexpected fidRef %+v with path %q, wanted %q", ref, s, name))
 	}
 
-	p.fidRefs.Store(ref, name)
+	p.fidRefNames.Store(ref, name)
 }
 
 // removeChild removes the given child.
 //
 // This applies only to an individual fidRef.
-//
-// Precondition: mu must be held in a writable fashion.
 func (p *pathNode) removeChild(ref *fidRef) {
-	p.fidRefs.Delete(ref)
+	p.fidRefNames.Delete(ref)
 }
 
 // removeWithName removes all references with the given name.
@@ -92,11 +96,12 @@ func (p *pathNode) removeChild(ref *fidRef) {
 //
 // The provided function is executed after removal.
 //
-// Precondition: mu must be held in a writable fashion.
+// Precondition: This call is synchronized w.r.t. other adding or removing of
+// children.
 func (p *pathNode) removeWithName(name string, fn func(ref *fidRef)) *pathNode {
-	p.fidRefs.Range(func(key, value interface{}) bool {
+	p.fidRefNames.Range(func(key, value interface{}) bool {
 		if value.(string) == name {
-			p.fidRefs.Delete(key)
+			p.fidRefNames.Delete(key)
 			fn(key.(*fidRef))
 		}
 		return true
@@ -104,6 +109,6 @@ func (p *pathNode) removeWithName(name string, fn func(ref *fidRef)) *pathNode {
 
 	// Return the original path node.
 	origPathNode := p.pathNodeFor(name)
-	p.children.Delete(name)
+	p.childNodes.Delete(name)
 	return origPathNode
 }
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index bdeb495c2..c56b8e439 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -201,17 +201,17 @@ func (f *fidRef) maybeParent() *fidRef {
 
 // notifyDelete marks all fidRefs as deleted.
 //
-// Precondition: the write lock must be held on the given pathNode.
+// Precondition: this must be called via safelyWrite or safelyGlobal.
 func notifyDelete(pn *pathNode) {
 	// Call on all local references.
-	pn.fidRefs.Range(func(key, _ interface{}) bool {
+	pn.fidRefNames.Range(func(key, _ interface{}) bool {
 		ref := key.(*fidRef)
 		atomic.StoreUint32(&ref.deleted, 1)
 		return true
 	})
 
 	// Call on all subtrees.
-	pn.children.Range(func(_, value interface{}) bool {
+	pn.childNodes.Range(func(_, value interface{}) bool {
 		notifyDelete(value.(*pathNode))
 		return true
 	})
@@ -233,10 +233,10 @@ func (f *fidRef) markChildDeleted(name string) {
 // recursively. Note that this applies only for subtrees, as these
 // notifications do not apply to the actual file whose name has changed.
 //
-// Precondition: the write lock must be held on the given pathNode.
+// Precondition: this must be called via safelyGlobal.
 func notifyNameChange(pn *pathNode) {
 	// Call on all local references.
-	pn.fidRefs.Range(func(key, value interface{}) bool {
+	pn.fidRefNames.Range(func(key, value interface{}) bool {
 		ref := key.(*fidRef)
 		name := value.(string)
 		ref.file.Renamed(ref.parent.file, name)
@@ -244,7 +244,7 @@ func notifyNameChange(pn *pathNode) {
 	})
 
 	// Call on all subtrees.
-	pn.children.Range(func(_, value interface{}) bool {
+	pn.childNodes.Range(func(name, value interface{}) bool {
 		notifyNameChange(value.(*pathNode))
 		return true
 	})
@@ -264,7 +264,7 @@ func (f *fidRef) renameChildTo(oldName string, target *fidRef, newName string) {
 	})
 
 	// Replace the previous (now deleted) path node.
-	target.pathNode.children.Store(newName, origPathNode)
+	target.pathNode.childNodes.Store(newName, origPathNode)
 
 	// Call Renamed on everything above.
 	notifyNameChange(origPathNode)
diff --git a/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo.go b/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo.go
index 25ad17a4e..895abb129 100755
--- a/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo.go
+++ b/pkg/sentry/kernel/seqatomic_taskgoroutineschedinfo.go
@@ -1,12 +1,12 @@
 package kernel
 
 import (
-	"fmt"
-	"reflect"
-	"strings"
 	"unsafe"
 
+	"fmt"
 	"gvisor.dev/gvisor/third_party/gvsync"
+	"reflect"
+	"strings"
 )
 
 // SeqAtomicLoad returns a copy of *ptr, ensuring that the read does not race
diff --git a/pkg/sentry/platform/ring0/defs_impl.go b/pkg/sentry/platform/ring0/defs_impl.go
index 5032ac56e..ea3f514cd 100755
--- a/pkg/sentry/platform/ring0/defs_impl.go
+++ b/pkg/sentry/platform/ring0/defs_impl.go
@@ -1,13 +1,14 @@
 package ring0
 
 import (
-	"fmt"
 	"gvisor.dev/gvisor/pkg/cpuid"
+	"syscall"
+
+	"fmt"
 	"gvisor.dev/gvisor/pkg/sentry/platform/ring0/pagetables"
 	"gvisor.dev/gvisor/pkg/sentry/usermem"
 	"io"
 	"reflect"
-	"syscall"
 )
 
 var (
diff --git a/pkg/sentry/time/seqatomic_parameters.go b/pkg/sentry/time/seqatomic_parameters.go
index 89792c56d..f6560d0bb 100755
--- a/pkg/sentry/time/seqatomic_parameters.go
+++ b/pkg/sentry/time/seqatomic_parameters.go
@@ -1,12 +1,12 @@
 package time
 
 import (
-	"fmt"
-	"reflect"
-	"strings"
 	"unsafe"
 
+	"fmt"
 	"gvisor.dev/gvisor/third_party/gvsync"
+	"reflect"
+	"strings"
 )
 
 // SeqAtomicLoad returns a copy of *ptr, ensuring that the read does not race