nodefs: garbage collection of unreferenced nodes.

Nodes can become unused due to following reasons:

* no more children (see new RmChild function) 
* no kernel references (Forget)
* stop being persistent

If this happens, drop the node from the tree. Removals of nodes
cascade from leaves up to parents recursively

lookupCount is now protected by under Inode.mu

nodeID remains under bridge.mu

nodefs/bridge.go

@@ -87,19 +87,18 @@ func (b *rawBridge) Lookup(header *fuse.InHeader, name string, out *fuse.EntryOu
 		return code
 	}
 
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
 	lockNodes(parent, child)
 	parent.setEntry(name, child)
 	unlockNodes(parent, child)
 
+	b.mu.Lock()
 	if child.nodeID == 0 {
 		b.registerInode(child)
 	}
-
 	out.NodeId = child.nodeID
-	out.Generation = b.nodes[child.nodeID].generation
+	b.mu.Unlock()
+
+	out.Generation = b.nodes[out.NodeId].generation
 
 	if b.options.AttrTimeout != nil {
 		out.SetAttrTimeout(*b.options.AttrTimeout)
@@ -111,6 +110,7 @@ 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
 func (b *rawBridge) registerInode(child *Inode) {
 	if l := len(b.free); l > 0 {
 		last := b.free[l-1]
@@ -139,15 +139,17 @@ func (b *rawBridge) Create(input *fuse.CreateIn, name string, out *fuse.CreateOu
 		return code
 	}
 
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	b.registerInode(child)
-
 	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
 
@@ -158,7 +160,6 @@ func (b *rawBridge) Create(input *fuse.CreateIn, name string, out *fuse.CreateOu
 		out.SetEntryTimeout(*b.options.EntryTimeout)
 	}
 
-	out.Fh = b.registerFile(f)
 	out.OpenFlags = flags
 
 	f.GetAttr(ctx, &out.Attr)
@@ -167,17 +168,13 @@ func (b *rawBridge) Create(input *fuse.CreateIn, name string, out *fuse.CreateOu
 
 func (b *rawBridge) Forget(nodeid, nlookup uint64) {
 	b.mu.Lock()
-	defer b.mu.Unlock()
 	n := b.nodes[nodeid].inode
-	n.lookupCount -= nlookup
-	if n.lookupCount == 0 {
-		n.clearChildren()
-		n.clearParents()
+	b.mu.Unlock()
 
+	if forgotten, _ := n.removeRef(nlookup, false); forgotten {
 		b.free = append(b.free, nodeid)
 		b.nodes[nodeid].inode = nil
 	}
-
 }
 
 func (b *rawBridge) SetDebug(debug bool) {}
@@ -204,7 +201,6 @@ func (b *rawBridge) GetAttr(input *fuse.GetAttrIn, out *fuse.AttrOut) (code fuse
 }
 
 func (b *rawBridge) SetAttr(input *fuse.SetAttrIn, out *fuse.AttrOut) (code fuse.Status) {
-
 	ctx := context.TODO()
 
 	n, fEntry := b.inode(input.NodeId, input.Fh)
@@ -340,12 +336,15 @@ func (b *rawBridge) Open(input *fuse.OpenIn, out *fuse.OpenOut) (status fuse.Sta
 
 	b.mu.Lock()
 	defer b.mu.Unlock()
-
 	out.Fh = b.registerFile(f)
 	out.OpenFlags = flags
 	return fuse.OK
 }
 
+// registerFile hands out a file handle. Must have bridge.mu
+//
+// XXX is it allowed to return the same Fh from two different Open
+// calls on the same inode?
 func (b *rawBridge) registerFile(f File) uint64 {
 	var fh uint64
 	if len(b.freeFiles) > 0 {

nodefs/inode.go

@@ -36,17 +36,23 @@ type Inode struct {
 
 	// the following fields protected by bridge.mu
 
-	// ID of the inode; 0 if inode was forgotten.
-	// forgotten inodes are unlinked from parent and children, but could be
-	// still not yet removed from bridge.nodes .
-	lookupCount uint64
-	nodeID      uint64
+	// ID of the inode; 0 if inode was forgotten.  Forgotten
+	// inodes could be persistent, not yet are unlinked from
+	// parent and children, but could be still not yet removed
+	// 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
+	// by calling removeRef.
+	persistent bool
+
 	// changeCounter increments every time the below mutable state
 	// (lookupCount, nodeID, children, parents) is modified.
 	//
@@ -55,6 +61,9 @@ type Inode struct {
 	// did not changed, and if it changed - retry the operation.
 	changeCounter uint32
 
+	// Number of kernel refs to this node.
+	lookupCount uint64
+
 	children map[string]*Inode
 	parents  map[parentData]struct{}
 }
@@ -132,7 +141,7 @@ func unlockNodes(ns ...*Inode) {
 func (n *Inode) Forgotten() bool {
 	n.bridge.mu.Lock()
 	defer n.bridge.mu.Unlock()
-	return n.lookupCount == 0
+	return n.nodeID == 0
 }
 
 // Node returns the Node object implementing the file system operations.
@@ -217,8 +226,6 @@ func (n *Inode) FindChildByOpaqueID(name string, opaqueID uint64) *Inode {
 // This, for example could be satisfied if both iparent and ichild are locked,
 // but it could be also valid if only iparent is locked and ichild was just
 // created and only one goroutine keeps referencing it.
-//
-// XXX also ichild.lookupCount++ ?
 func (iparent *Inode) setEntry(name string, ichild *Inode) {
 	ichild.parents[parentData{name, iparent}] = struct{}{}
 	iparent.children[name] = ichild
@@ -255,79 +262,155 @@ func (n *Inode) clearParents() {
 	}
 }
 
-func (n *Inode) clearChildren() {
-	if n.mode != fuse.S_IFDIR {
-		return
+// NewPersistentInode returns an Inode whose lifetime is not in
+// control of the kernel.
+func (n *Inode) NewPersistentInode(node Node, mode uint32, opaque uint64) *Inode {
+	return n.newInode(node, mode, opaque, true)
+}
+
+// ForgetPersistent manually marks the node as no longer important. If
+// 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)
+}
+
+// NewInode returns an inode for the given Node. The mode should be
+// standard mode argument (eg. S_IFDIR). The opaqueID argument can be
+// used to signal changes in the tree structure during lookup (see
+// FindChildByOpaqueID). For a loopback file system, the inode number
+// of the underlying file is a good candidate.
+func (n *Inode) NewInode(node Node, mode uint32, opaqueID uint64) *Inode {
+	return n.newInode(node, mode, opaqueID, false)
+}
+
+func (n *Inode) newInode(node Node, mode uint32, opaqueID uint64, persistent bool) *Inode {
+	ch := &Inode{
+		mode:       mode ^ 07777,
+		node:       node,
+		bridge:     n.bridge,
+		persistent: persistent,
+		parents:    make(map[parentData]struct{}),
+	}
+	if mode&fuse.S_IFDIR != 0 {
+		ch.children = make(map[string]*Inode)
 	}
+	if node.setInode(ch) {
+		return ch
+	}
+
+	return node.inode()
+}
 
+// removeRef decreases references. Returns if this operation caused
+// the node to be forgotten (for kernel references), and whether it is
+// live (ie. was not dropped from the tree)
+func (n *Inode) removeRef(nlookup uint64, dropPersistence bool) (forgotten bool, live bool) {
 	var lockme []*Inode
+	var parents []parentData
+
+	n.mu.Lock()
+	if nlookup > 0 && dropPersistence {
+		panic("only one allowed")
+	} else if nlookup > 0 {
+		n.lookupCount -= nlookup
+		n.changeCounter++
+	} else if dropPersistence && n.persistent {
+		n.persistent = false
+		n.changeCounter++
+	}
+
+retry:
 	for {
 		lockme = append(lockme[:0], n)
-
-		n.mu.Lock()
-		ts := n.changeCounter
-		for _, ch := range n.children {
-			lockme = append(lockme, ch)
+		parents = parents[:0]
+		nChange := n.changeCounter
+		live = n.lookupCount > 0 || len(n.children) > 0 || n.persistent
+		forgotten = n.lookupCount == 0
+		for p := range n.parents {
+			parents = append(parents, p)
+			lockme = append(lockme, p.parent)
 		}
 		n.mu.Unlock()
 
+		if live {
+			return forgotten, live
+		}
+
 		lockNodes(lockme...)
-		success := false
-		if ts == n.changeCounter {
-			for nm, ch := range n.children {
-				delete(ch.parents, parentData{nm, n})
-				ch.changeCounter++
-			}
-			n.children = map[string]*Inode{}
-			n.changeCounter++
-			success = true
+		if n.changeCounter != nChange {
+			unlockNodes(lockme...)
+			n.mu.Lock() // TODO could avoid unlocking and relocking n here.
+			continue retry
 		}
-		unlockNodes(lockme...)
 
-		if success {
-			break
+		for _, p := range parents {
+			delete(p.parent.children, p.name)
+			p.parent.changeCounter++
 		}
+		n.parents = map[parentData]struct{}{}
+		n.changeCounter++
+		unlockNodes(lockme...)
+		break
 	}
 
-	// XXX not right - we cannot fully clear our children, because they can
-	// be also children of another directory.
-	//
-	// XXX also not right - the kernel can send FORGET(idir) but keep
-	// references to children inodes.
-	for _, ch := range lockme {
-		if ch != n {
-			ch.clearChildren()
+	for _, p := range lockme {
+		if p != n {
+			p.removeRef(0, false)
 		}
 	}
+	return forgotten, false
 }
 
-// NewPersistentInode returns an Inode with a LookupCount == 1, ie. the
-// node will only get garbage collected if the kernel issues a forget
-// on any of its parents.
-func (n *Inode) NewPersistentInode(node Node, mode uint32, opaque uint64) *Inode {
-	ch := n.NewInode(node, mode, opaque)
-	ch.lookupCount++
-	return ch
-}
+// RmChild removes multiple children.  Returns whether the removal
+// succeeded and whether the node is still live afterward. The removal
+// is transactional: it only succeeds if all names are children, and
+// if they all were removed successfully.  If the removal was
+// successful, and there are no children left, the node may be removed
+// from the FS tree. In that case, RmChild returns live==false.
+func (n *Inode) RmChild(names ...string) (success, live bool) {
+	var lockme []*Inode
 
-// NewInode returns an inode for the given Node. The mode should be
-// standard mode argument (eg. S_IFDIR). The opaqueID argument can be
-// used to signal changes in the tree structure during lookup (see
-// FindChildByOpaqueID). For a loopback file system, the inode number
-// of the underlying file is a good candidate.
-func (n *Inode) NewInode(node Node, mode uint32, opaqueID uint64) *Inode {
-	ch := &Inode{
-		mode:    mode ^ 07777,
-		node:    node,
-		bridge:  n.bridge,
-		parents: make(map[parentData]struct{}),
-	}
-	if mode&fuse.S_IFDIR != 0 {
-		ch.children = make(map[string]*Inode)
+retry:
+	for {
+		n.mu.Lock()
+		lockme = append(lockme[:0], n)
+		nChange := n.changeCounter
+		for _, nm := range names {
+			ch := n.children[nm]
+			if ch == nil {
+				n.mu.Unlock()
+				return false, true
+			}
+			lockme = append(lockme, ch)
+		}
+		n.mu.Unlock()
+
+		lockNodes(lockme...)
+		if n.changeCounter != nChange {
+			unlockNodes(lockme...)
+			n.mu.Lock() // TODO could avoid unlocking and relocking n here.
+			continue retry
+		}
+
+		for _, nm := range names {
+			ch := n.children[nm]
+			delete(n.children, nm)
+			ch.changeCounter++
+		}
+		n.changeCounter++
+
+		live = n.lookupCount > 0 || len(n.children) > 0 || n.persistent
+		unlockNodes(lockme...)
+
+		// removal successful
+		break
 	}
-	if node.setInode(ch) {
-		return ch
+
+	if !live {
+		_, live := n.removeRef(0, false)
+		return true, live
 	}
 
-	return node.inode()
+	return true, true
 }