nodefs: move bridge.mu section under Inode critical section

nodefs/bridge.go

@@ -29,6 +29,8 @@ type rawBridge struct {
 	options Options
 	root    *Inode
 
+	// mu protects the following data.  Locks for inodes must be
+	// taken before rawBridge.mu
 	mu    sync.Mutex
 	nodes []mapEntry
 	free  []uint64
@@ -89,16 +91,14 @@ func (b *rawBridge) Lookup(header *fuse.InHeader, name string, out *fuse.EntryOu
 
 	lockNodes(parent, child)
 	parent.setEntry(name, child)
-	unlockNodes(parent, child)
-
 	b.mu.Lock()
 	if child.nodeID == 0 {
 		b.registerInode(child)
 	}
 	out.NodeId = child.nodeID
-	b.mu.Unlock()
-
 	out.Generation = b.nodes[out.NodeId].generation
+	b.mu.Unlock()
+	unlockNodes(parent, child)
 
 	if b.options.AttrTimeout != nil {
 		out.SetAttrTimeout(*b.options.AttrTimeout)
@@ -110,7 +110,8 @@ func (b *rawBridge) Lookup(header *fuse.InHeader, name string, out *fuse.EntryOu
 	return fuse.OK
 }
 
-// registerInode sets an inode number in the child. Must have bridge.mu
+// registerInode sets an nodeID in the child. Must have bridge.mu and
+// child.mu
 func (b *rawBridge) registerInode(child *Inode) {
 	if l := len(b.free); l > 0 {
 		last := b.free[l-1]
@@ -141,17 +142,15 @@ func (b *rawBridge) Create(input *fuse.CreateIn, name string, out *fuse.CreateOu
 
 	lockNodes(parent, child)
 	parent.setEntry(name, child)
-	unlockNodes(parent, child)
-
 	b.mu.Lock()
 	if child.nodeID == 0 {
 		b.registerInode(child)
 	}
 	out.Fh = b.registerFile(f)
-	b.mu.Unlock()
-
 	out.NodeId = child.nodeID
 	out.Generation = b.nodes[child.nodeID].generation
+	b.mu.Unlock()
+	unlockNodes(parent, child)
 
 	if b.options.AttrTimeout != nil {
 		out.SetAttrTimeout(*b.options.AttrTimeout)
@@ -171,10 +170,12 @@ func (b *rawBridge) Forget(nodeid, nlookup uint64) {
 	n := b.nodes[nodeid].inode
 	b.mu.Unlock()
 
-	if forgotten, _ := n.removeRef(nlookup, false); forgotten {
-		b.free = append(b.free, nodeid)
-		b.nodes[nodeid].inode = nil
-	}
+	n.removeRef(nlookup, false)
+}
+
+func (b *rawBridge) unregisterNode(nodeid uint64) {
+	b.free = append(b.free, nodeid)
+	b.nodes[nodeid].inode = nil
 }
 
 func (b *rawBridge) SetDebug(debug bool) {}

nodefs/files.go

@@ -77,15 +77,15 @@ func (f *loopbackFile) Fsync(ctx context.Context, flags int) (code fuse.Status)
 }
 
 const (
-	F_OFD_GETLK  = 36
-	F_OFD_SETLK  = 37
-	F_OFD_SETLKW = 38
+	_OFD_GETLK  = 36
+	_OFD_SETLK  = 37
+	_OFD_SETLKW = 38
 )
 
 func (f *loopbackFile) GetLk(ctx context.Context, owner uint64, lk *fuse.FileLock, flags uint32, out *fuse.FileLock) (code fuse.Status) {
 	flk := syscall.Flock_t{}
 	lk.ToFlockT(&flk)
-	code = fuse.ToStatus(syscall.FcntlFlock(f.File.Fd(), F_OFD_GETLK, &flk))
+	code = fuse.ToStatus(syscall.FcntlFlock(f.File.Fd(), _OFD_GETLK, &flk))
 	out.FromFlockT(&flk)
 	return
 }
@@ -120,9 +120,9 @@ func (f *loopbackFile) setLock(ctx context.Context, owner uint64, lk *fuse.FileL
 		lk.ToFlockT(&flk)
 		var op int
 		if blocking {
-			op = F_OFD_SETLKW
+			op = _OFD_SETLKW
 		} else {
-			op = F_OFD_SETLK
+			op = _OFD_SETLK
 		}
 		return fuse.ToStatus(syscall.FcntlFlock(f.File.Fd(), op, &flk))
 	}

nodefs/inode.go

@@ -34,7 +34,10 @@ type Inode struct {
 
 	// Following data is mutable.
 
-	// the following fields protected by bridge.mu
+	// mu protects the following mutable fields. When locking
+	// multiple Inodes, locks must be acquired using
+	// lockNodes/unlockNodes
+	mu sync.Mutex
 
 	// ID of the inode; 0 if inode was forgotten.  Forgotten
 	// inodes could be persistent, not yet are unlinked from
@@ -42,11 +45,6 @@ type Inode struct {
 	// from bridge.nodes .
 	nodeID uint64
 
-	// mu protects the following mutable fields. When locking
-	// multiple Inodes, locks must be acquired using
-	// lockNodes/unlockNodes
-	mu sync.Mutex
-
 	// persistent indicates that this node should not be removed
 	// from the tree, even if there are no live references. This
 	// must be set on creation, and can only be changed to false
@@ -272,7 +270,7 @@ func (n *Inode) NewPersistentInode(node Node, mode uint32, opaque uint64) *Inode
 // it has no children, and if the kernel as no references, the nodes
 // gets removed from the tree.
 func (n *Inode) ForgetPersistent() {
-	return n.removeRef(0, true)
+	n.removeRef(0, true)
 }
 
 // NewInode returns an inode for the given Node. The mode should be
@@ -350,6 +348,18 @@ retry:
 		}
 		n.parents = map[parentData]struct{}{}
 		n.changeCounter++
+
+		if n.lookupCount != 0 {
+			panic("lookupCount changed")
+		}
+
+		if n.nodeID != 0 {
+			n.bridge.mu.Lock()
+			n.bridge.unregisterNode(n.nodeID)
+			n.bridge.mu.Unlock()
+			n.nodeID = 0
+		}
+
 		unlockNodes(lockme...)
 		break
 	}