Clean up nodeId/lookupCount interactions.

Now, nodeId and lookupCount are exclusively used for kernel
communication, and a nodeId may change during the lifetime of the
Inode if the same node is forgotten and looked up again.

fuse/fsconnector.go

@@ -42,12 +42,12 @@ func NewFileSystemConnector(nodeFs NodeFileSystem, opts *FileSystemOptions) (me
 		opts = NewFileSystemOptions()
 	}
 	me.inodeMap = NewHandleMap(!opts.SkipCheckHandles)
-	me.rootNode = me.newInode(true)
-	me.rootNode.nodeId = FUSE_ROOT_ID
+	me.rootNode = newInode(true, nodeFs.Root())
 
 	// FUSE does not issue a LOOKUP for 1 (obviously), but it does
 	// issue a forget.  This lookupCount is to make the counts match.
-	me.rootNode.addLookupCount(1)
+	me.lookupUpdate(me.rootNode)
+	
 	me.verify()
 	me.MountRoot(nodeFs, opts)
 	return me
@@ -61,50 +61,25 @@ func (me *FileSystemConnector) verify() {
 	root.verify(me.rootNode.mountPoint)
 }
 
-func (me *FileSystemConnector) newInode(isDir bool) *Inode {
-	data := new(Inode)
-	data.nodeId = me.inodeMap.Register(&data.handled, data)
-	data.connector = me
-	if isDir {
-		data.children = make(map[string]*Inode, initDirSize)
-	}
-
-	return data
-}
 
 // createChild() creates a child for given as FsNode as child of 'parent'.  The
 // resulting inode will have its lookupCount incremented.
-func (me *FileSystemConnector) createChild(parent *Inode, name string, fi *os.FileInfo, fsi FsNode) (out *EntryOut, child *Inode) {
-	if fsi.Inode() == nil {
-		child = parent.CreateChild(name, fi.IsDirectory(), fsi)
-	} else {
+func (me *FileSystemConnector) createChild(parent *Inode, name string, fi *os.FileInfo, fsi FsNode) (out *EntryOut) {
 	parent.treeLock.Lock()
 	defer parent.treeLock.Unlock()
-		child = fsi.Inode()
-		child.addLookupCount(1)
+
+	child := fsi.Inode()
+	if child == nil {
+		child = parent.createChild(name, fi.IsDirectory(), fsi)
+	} else {
+		parent.addChild(name, child)
 	}
+	me.lookupUpdate(child)
 	
 	out = parent.mount.fileInfoToEntry(fi)
 	out.Ino = child.nodeId
 	out.NodeId = child.nodeId
-	return out, child
-}
-
-func (me *FileSystemConnector) lookupUpdate(parent *Inode, name string, isDir bool, lookupCount int) *Inode {
-	defer me.verify()
-
-	parent.treeLock.Lock()
-	defer parent.treeLock.Unlock()
-
-	data, ok := parent.children[name]
-	if !ok {
-		data = me.newInode(isDir)
-		parent.addChild(name, data)
-		data.mount = parent.mount
-		data.treeLock = &data.mount.treeLock
-	}
-	data.addLookupCount(lookupCount)
-	return data
+	return out
 }
 
 func (me *FileSystemConnector) findMount(parent *Inode, name string) (mount *fileSystemMount) {
@@ -125,15 +100,28 @@ func (me *FileSystemConnector) toInode(nodeid uint64) *Inode {
 	return i
 }
 
-func (me *FileSystemConnector) forgetUpdate(nodeId uint64, forgetCount int) {
-	defer me.verify()
+// Must run in treeLock.
+func (me *FileSystemConnector) lookupUpdate(node *Inode) {
+	if node.lookupCount == 0 {
+		node.nodeId = me.inodeMap.Register(&node.handled, node)
+	}
+	node.lookupCount += 1
+}
 
-	node := me.toInode(nodeId)
+func (me *FileSystemConnector) forgetUpdate(node *Inode, forgetCount int) {
+	defer me.verify()
 
 	node.treeLock.Lock()
 	defer node.treeLock.Unlock()
 
-	node.addLookupCount(-forgetCount)
+	node.lookupCount -= forgetCount
+	if node.lookupCount == 0 {
+		me.inodeMap.Forget(node.nodeId)
+		node.nodeId = 0
+	} else if node.lookupCount < 0 {
+		panic(fmt.Sprintf("lookupCount underflow: %d: %v", node.lookupCount, me))
+	}
+	
 	me.recursiveConsiderDropInode(node)
 }
 
@@ -158,11 +146,12 @@ func (me *FileSystemConnector) recursiveConsiderDropInode(n *Inode) (drop bool)
 		if ch == nil {
 			panic(fmt.Sprintf("trying to del child %q, but not present", k))
 		}
+		// TODO - change name? This does not really mark the
+		// fuse Forget operation.
 		ch.fsInode.OnForget()
-		me.inodeMap.Forget(ch.nodeId)
 	}
 
-	if len(n.children) > 0 || n.Live() {
+	if len(n.children) > 0 || n.lookupCount > 0 || n.synthetic {
 		return false
 	}
 	if n == me.rootNode || n.mountPoint != nil {
@@ -271,7 +260,7 @@ func (me *FileSystemConnector) Mount(parent *Inode, name string, nodeFs NodeFile
 		return EBUSY
 	}
 
-	node = me.newInode(true)
+	node = newInode(true, nodeFs.Root())
 	if opts == nil {
 		opts = me.rootNode.mountPoint.options
 	}
@@ -335,14 +324,28 @@ func (me *FileSystemConnector) Unmount(node *Inode) Status {
 }
 
 func (me *FileSystemConnector) FileNotify(node *Inode, off int64, length int64) Status {
+	n := node.nodeId
+	if node == me.rootNode {
+		n = FUSE_ROOT_ID
+	}
+	if n == 0 {
+		return OK
+	}
 	out := NotifyInvalInodeOut{
 		Length: length,
 		Off:    off,
-		Ino:    node.nodeId,
+		Ino:    n, 
 	}
 	return me.fsInit.InodeNotify(&out)
 }
 
 func (me *FileSystemConnector) EntryNotify(dir *Inode, name string) Status {
-	return me.fsInit.EntryNotify(dir.nodeId, name)
+	n := dir.nodeId
+	if dir == me.rootNode {
+		n = FUSE_ROOT_ID
+	}
+	if n == 0 {
+		return OK
+	}
+	return me.fsInit.EntryNotify(n, name)
 }

fuse/fsops.go

@@ -32,8 +32,7 @@ func (me *FileSystemConnector) lookupMountUpdate(mount *fileSystemMount) (out *E
 	mount.treeLock.Lock()
 	defer mount.treeLock.Unlock()
 
-	mount.mountInode.addLookupCount(1)
-
+	me.lookupUpdate(mount.mountInode)
 	out = mount.fileInfoToEntry(fi)
 	out.NodeId = mount.mountInode.nodeId
 	out.Ino = out.NodeId
@@ -60,6 +59,7 @@ func (me *FileSystemConnector) internalLookup(parent *Inode, name string, contex
 	return me.postLookup(fi, fsNode, code, getattrNode, lookupNode, name)
 }
 
+
 // Prepare for lookup: we are either looking for getattr of an
 // existing node, or lookup a new one. Here we decide which of those
 func (me *FileSystemConnector) preLookup(parent *Inode, name string) (lookupNode *Inode, attrNode *Inode) {
@@ -69,7 +69,7 @@ func (me *FileSystemConnector) preLookup(parent *Inode, name string) (lookupNode
 	child := parent.children[name]
 	if child != nil {
 		// Make sure the child doesn't die inbetween.
-		child.addLookupCount(1)
+		me.lookupUpdate(child)
 		return nil, child
 	}
 
@@ -89,7 +89,7 @@ func (me *FileSystemConnector) postLookup(fi *os.FileInfo, fsNode FsNode, code S
 		if attrNode != nil {
 			mount.treeLock.Lock()
 			defer mount.treeLock.Unlock()
-			attrNode.addLookupCount(-1)
+			me.forgetUpdate(attrNode, -1)
 		}
 
 		if code == ENOENT && mount.options.NegativeTimeout > 0.0 {
@@ -104,13 +104,14 @@ func (me *FileSystemConnector) postLookup(fi *os.FileInfo, fsNode FsNode, code S
 		out.NodeId = attrNode.nodeId
 		out.Ino = attrNode.nodeId
 	} else if lookupNode != nil {
-		out, _ = me.createChild(lookupNode, name, fi, fsNode)
+		out = me.createChild(lookupNode, name, fi, fsNode)
 	}
 	return out, OK
 }
 
 func (me *FileSystemConnector) Forget(h *InHeader, input *ForgetIn) {
-	me.forgetUpdate(h.NodeId, int(input.Nlookup))
+	node := me.toInode(h.NodeId)
+	me.forgetUpdate(node, int(input.Nlookup))
 }
 
 func (me *FileSystemConnector) GetAttr(header *InHeader, input *GetAttrIn) (out *AttrOut, code Status) {
@@ -228,7 +229,7 @@ func (me *FileSystemConnector) Mknod(header *InHeader, input *MknodIn, name stri
 	parent := me.toInode(header.NodeId)
 	fi, fsNode, code := parent.fsInode.Mknod(name, input.Mode, uint32(input.Rdev), &header.Context)
 	if code.Ok() {
-		out, _ = me.createChild(parent, name, fi, fsNode)
+		out = me.createChild(parent, name, fi, fsNode)
 	}
 	return out, code
 }
@@ -238,7 +239,7 @@ func (me *FileSystemConnector) Mkdir(header *InHeader, input *MkdirIn, name stri
 	fi, fsInode, code := parent.fsInode.Mkdir(name, input.Mode, &header.Context)
 
 	if code.Ok() {
-		out, _ = me.createChild(parent, name, fi, fsInode)
+		out = me.createChild(parent, name, fi, fsInode)
 	}
 	return out, code
 }
@@ -267,7 +268,7 @@ func (me *FileSystemConnector) Symlink(header *InHeader, pointedTo string, linkN
 	parent := me.toInode(header.NodeId)
 	fi, fsNode, code := parent.fsInode.Symlink(linkName, pointedTo, &header.Context)
 	if code.Ok() {
-		out, _ = me.createChild(parent, linkName, fi, fsNode)
+		out = me.createChild(parent, linkName, fi, fsNode)
 	}
 	return out, code
 }
@@ -304,15 +305,7 @@ func (me *FileSystemConnector) Link(header *InHeader, input *LinkIn, name string
 		return nil, code
 	}
 
-	if fsInode.Inode() == nil {
-		out, _ = me.createChild(parent, name, fi, fsInode)
-	} else {
-		fsInode.Inode().addLookupCount(1)
-		parent.addChild(name, fsInode.Inode())
-		out = parent.mount.fileInfoToEntry(fi)
-		out.Ino = fsInode.Inode().nodeId
-		out.NodeId = out.Ino
-	}
+	out = me.createChild(parent, name, fi, fsInode)
 	return out, code
 }
 
@@ -327,8 +320,8 @@ func (me *FileSystemConnector) Create(header *InHeader, input *CreateIn, name st
 	if !code.Ok() {
 		return 0, 0, nil, code
 	}
-	out, child := me.createChild(parent, name, fi, fsNode)
-	handle, opened := parent.mount.registerFileHandle(child, nil, f, input.Flags)
+	out = me.createChild(parent, name, fi, fsNode)
+	handle, opened := parent.mount.registerFileHandle(fsNode.Inode(), nil, f, input.Flags)
 	return opened.FuseFlags, handle, out, code
 }
 

fuse/inode.go

@@ -12,9 +12,6 @@ var _ = log.Println
 type Inode struct {
 	handled Handled
 
-	// Constant during lifetime.
-	nodeId uint64
-
 	// Number of open files and its protection.
 	openFilesMutex sync.Mutex
 	openFiles      []*openedFile
@@ -36,9 +33,19 @@ type Inode struct {
 	// are duplicated in children.
 	mounts map[string]*fileSystemMount
 
-	// Use addLookupCount() to manipulate.
+	// This is exclusively used for managing the lifetime of
+	// nodeId below, and it is ok for a node to have 0 lookupCount
+	// and be in the system if it is synthetic.
 	lookupCount int
 
+	// The nodeId is only used to communicate to the kernel.  If
+	// it is zero, it means the kernel does not know about this
+	// Inode.
+	nodeId uint64
+	
+	// This is to prevent lookupCount==0 node from being dropped.
+	synthetic bool
+	
 	// Non-nil if this is a mountpoint.
 	mountPoint *fileSystemMount
 
@@ -46,8 +53,17 @@ type Inode struct {
 	// during the lifetime, except upon Unmount() when it is set
 	// to nil.
 	mount *fileSystemMount
+}
 
-	connector *FileSystemConnector
+func newInode(isDir bool, fsNode FsNode) *Inode {
+	me := new(Inode)
+	if isDir {
+		me.children = make(map[string]*Inode, initDirSize)
+	}
+	me.fsInode = fsNode
+	me.fsInode.SetInode(me)
+
+	return me
 }
 
 // public methods.
@@ -60,10 +76,6 @@ func (me *Inode) LockTree() func() {
 	return func() { me.treeLock.Unlock() }
 }
 
-func (me *Inode) Live() bool {
-	return me.lookupCount > 0
-}
-
 // Returns any open file, preferably a r/w one.
 func (me *Inode) AnyFile() (file File) {
 	me.openFilesMutex.Lock()
@@ -109,22 +121,26 @@ func (me *Inode) IsDir() bool {
 	return me.children != nil
 }
 
-// Creates an Inode as child.
+
+// CreateChild() creates node for synthetic use
 func (me *Inode) CreateChild(name string, isDir bool, fsi FsNode) *Inode {
 	me.treeLock.Lock()
 	defer me.treeLock.Unlock()
 
+	ch :=  me.createChild(name, isDir, fsi)
+	ch.synthetic = true
+	return ch
+}
+
+// Creates an Inode as child.
+func (me *Inode) createChild(name string, isDir bool, fsi FsNode) *Inode {
 	ch := me.children[name]
 	if ch != nil {
 		panic(fmt.Sprintf("already have a child at %v %q", me.nodeId, name))
 	}
-	ch = me.connector.newInode(isDir)
-	ch.fsInode = fsi
-	fsi.SetInode(ch)
+	ch = newInode(isDir, fsi)
 	ch.mount = me.mount
 	ch.treeLock = me.treeLock
-	ch.addLookupCount(1)
-	ch.connector = me.connector
 
 	me.addChild(name, ch)
 	return ch
@@ -140,13 +156,6 @@ func (me *Inode) GetChild(name string) (child *Inode) {
 ////////////////////////////////////////////////////////////////
 // private
 
-func (me *Inode) addLookupCount(delta int) {
-	me.lookupCount += delta
-	if me.lookupCount < 0 {
-		panic(fmt.Sprintf("lookupCount underflow: %d: %v", me.lookupCount, me))
-	}
-}
-
 // Must be called with treeLock for the mount held.
 func (me *Inode) addChild(name string, child *Inode) {
 	if paranoia {
@@ -182,8 +191,6 @@ func (me *Inode) mountFs(fs NodeFileSystem, opts *FileSystemOptions) {
 	}
 	me.mount = me.mountPoint
 	me.treeLock = &me.mountPoint.treeLock
-	me.fsInode = fs.Root()
-	me.fsInode.SetInode(me)
 }
 
 // Must be called with treeLock held.
@@ -223,6 +230,9 @@ func (me *Inode) verify(cur *fileSystemMount) {
 	if me.lookupCount < 0 {
 		panic(fmt.Sprintf("negative lookup count %d on node %d", me.lookupCount, me.nodeId))
 	}
+	if (me.lookupCount == 0) != (me.nodeId == 0) {
+		panic("kernel registration mismatch")
+	}
 	if me.mountPoint != nil {
 		if me != me.mountPoint.mountInode {
 			panic("mountpoint mismatch")