Move lookupCount and nodeId handling into HandleMap.

fuse/fsconnector.go

@@ -115,31 +115,26 @@ func (c *FileSystemConnector) toInode(nodeid uint64) *Inode {
 }
 
 // Must run outside treeLock.  Returns the nodeId.
-func (c *FileSystemConnector) lookupUpdate(node *Inode) uint64 {
-	node.treeLock.Lock()
-	if node.lookupCount == 0 {
-		node.nodeId = c.inodeMap.Register(&node.handled)
-	}
-	node.lookupCount += 1
-	id := node.nodeId
-	node.treeLock.Unlock()
-
+func (c *FileSystemConnector) lookupUpdate(node *Inode) (id uint64) {
+	id = c.inodeMap.Register(&node.handled)
+	c.verify()
 	return id
 }
 
 // Must run outside treeLock.
-func (c *FileSystemConnector) forgetUpdate(node *Inode, forgetCount int) {
-	node.treeLock.Lock()
-	node.lookupCount -= forgetCount
-	if node.lookupCount == 0 {
-		c.inodeMap.Forget(node.nodeId)
-		node.nodeId = 0
-	} else if node.lookupCount < 0 {
-		log.Panicf("lookupCount underflow: %d: %v", node.lookupCount, c)
+func (c *FileSystemConnector) forgetUpdate(nodeID uint64, forgetCount int) {
+	if nodeID == raw.FUSE_ROOT_ID {
+		// We never got a lookup for root, so don't try to forget root.
+		return
 	}
-
-	c.recursiveConsiderDropInode(node)
-	node.treeLock.Unlock()
+	
+	if forgotten, handled := c.inodeMap.Forget(nodeID, forgetCount); forgotten {
+		node := (*Inode)(unsafe.Pointer(handled))
+		node.treeLock.Lock()
+		c.recursiveConsiderDropInode(node)
+		node.treeLock.Unlock()
+	}
+	// TODO - try to drop children even forget was not successful.
 	c.verify()
 }
 
@@ -167,7 +162,7 @@ func (c *FileSystemConnector) recursiveConsiderDropInode(n *Inode) (drop bool) {
 		ch.fsInode.OnForget()
 	}
 
-	if len(n.children) > 0 || n.lookupCount > 0 || !n.FsNode().Deletable() {
+	if len(n.children) > 0 || !n.FsNode().Deletable() {
 		return false
 	}
 	if n == c.rootNode || n.mountPoint != nil {
@@ -281,7 +276,7 @@ func (c *FileSystemConnector) Mount(parent *Inode, name string, nodeFs NodeFileS
 	node.mountPoint.parentInode = parent
 	if c.Debug {
 		log.Println("Mount: ", nodeFs, "on subdir", name,
-			"parent", parent.nodeId)
+			"parent", c.inodeMap.Handle(&parent.handled))
 	}
 	nodeFs.OnMount(c)
 	return OK
@@ -297,7 +292,7 @@ func (c *FileSystemConnector) Mount(parent *Inode, name string, nodeFs NodeFileS
 func (c *FileSystemConnector) Unmount(node *Inode) Status {
 	// TODO - racy.
 	if node.mountPoint == nil {
-		log.Println("not a mountpoint:", node.nodeId)
+		log.Println("not a mountpoint:", c.inodeMap.Handle(&node.handled))
 		return EINVAL
 	}
 
@@ -317,7 +312,8 @@ func (c *FileSystemConnector) Unmount(node *Inode) Status {
 
 	if mount.mountInode != node {
 		log.Panicf("got two different mount inodes %v vs %v",
-			mount.mountInode.nodeId, node.nodeId)
+			c.inodeMap.Handle(&mount.mountInode.handled),
+			c.inodeMap.Handle(&node.handled))
 	}
 
 	if !node.canUnmount() {
@@ -331,69 +327,63 @@ func (c *FileSystemConnector) Unmount(node *Inode) Status {
 	delete(parentNode.children, name)
 	mount.fs.OnUnmount()
 
-	parentId := parentNode.nodeId
+	parentId := c.inodeMap.Handle(&parentNode.handled)
 	if parentNode == c.rootNode {
 		// TODO - test coverage. Currently covered by zipfs/multizip_test.go
 		parentId = raw.FUSE_ROOT_ID
 	}
 
-	c.fsInit.DeleteNotify(parentId, node.nodeId, name)
+	c.fsInit.DeleteNotify(parentId, c.inodeMap.Handle(&node.handled), name)
 	return OK
 }
 
 func (c *FileSystemConnector) FileNotify(node *Inode, off int64, length int64) Status {
-	node.treeLock.RLock()
-	n := node.nodeId
-	node.treeLock.RUnlock()
+	var nId uint64
 	if node == c.rootNode {
-		n = raw.FUSE_ROOT_ID
+		nId = raw.FUSE_ROOT_ID
+	} else {
+		nId = c.inodeMap.Handle(&node.handled)
 	}
-	if n == 0 {
+		
+	if nId == 0 {
 		return OK
 	}
 	out := raw.NotifyInvalInodeOut{
 		Length: length,
 		Off:    off,
-		Ino:    n,
+		Ino:    nId,
 	}
 	return c.fsInit.InodeNotify(&out)
 }
 
-func (c *FileSystemConnector) EntryNotify(dir *Inode, name string) Status {
-	dir.treeLock.RLock()
-	n := dir.nodeId
-	dir.treeLock.RUnlock()
-	if dir == c.rootNode {
-		n = raw.FUSE_ROOT_ID
+func (c *FileSystemConnector) EntryNotify(node *Inode, name string) Status {
+	var nId uint64 
+	if node == c.rootNode {
+		nId = raw.FUSE_ROOT_ID
+	} else {
+		nId = c.inodeMap.Handle(&node.handled)
 	}
-	if n == 0 {
+
+	if nId == 0 {
 		return OK
 	}
-	return c.fsInit.EntryNotify(n, name)
+	return c.fsInit.EntryNotify(nId, name)
 }
 
 func (c *FileSystemConnector) DeleteNotify(dir *Inode, child *Inode, name string) Status {
 	var nId uint64
-	var chId uint64
 
-	dir.treeLock.RLock()
 	if dir == c.rootNode {
 		nId = raw.FUSE_ROOT_ID
 	} else {
-		nId = dir.nodeId
+		nId = c.inodeMap.Handle(&dir.handled)
 	}
 
-	if child.treeLock != dir.treeLock {
-		child.treeLock.RLock()
-		chId = child.nodeId
-		child.treeLock.RUnlock()
-	} else {
-		chId = child.nodeId
-	}
-	dir.treeLock.RUnlock()
-
 	if nId == 0 {
 		return OK
 	}
+
+	chId := c.inodeMap.Handle(&child.handled)
+	
 	return c.fsInit.DeleteNotify(nId, chId, name)
 }

fuse/fsmount.go

@@ -85,7 +85,7 @@ func (m *fileSystemMount) getOpenedFile(h uint64) *openedFile {
 }
 
 func (m *fileSystemMount) unregisterFileHandle(handle uint64, node *Inode) *openedFile {
-	obj := m.openFiles.Forget(handle)
+	_, obj := m.openFiles.Forget(handle, 1)
 	opened := (*openedFile)(unsafe.Pointer(obj))
 	node.openFilesMutex.Lock()
 	idx := -1

fuse/fsops.go

@@ -99,8 +99,7 @@ func (c *FileSystemConnector) Lookup(out *raw.EntryOut, header *raw.InHeader, na
 }
 
 func (c *FileSystemConnector) Forget(nodeID, nlookup uint64) {
-	node := c.toInode(nodeID)
-	c.forgetUpdate(node, int(nlookup))
+	c.forgetUpdate(nodeID, int(nlookup))
 }
 
 func (c *FileSystemConnector) GetAttr(out *raw.AttrOut, header *raw.InHeader, input *raw.GetAttrIn) (code Status) {

fuse/handle.go

@@ -23,12 +23,24 @@ type HandleMap interface {
 	Register(obj *Handled) uint64
 	Count() int
 	Decode(uint64) *Handled
-	Forget(uint64) *Handled
+	Forget(handle uint64, count int) (bool, *Handled)
+	Handle(obj* Handled) uint64
 	Has(uint64) bool
 }
 
 type Handled struct {
 	check  uint32
+	handle uint64
+	count  int
+}
+
+func (h *Handled) verify() {
+	if h.count < 0 {
+		log.Panicf("negative lookup count %d", h.count)
+	}
+	if (h.count == 0) != (h.handle == 0) {
+		log.Panicf("registration mismatch: lookup %d id %d", h.count, h.handle)
+	}
 }
 
 const _ALREADY_MSG = "Object already has a handle"
@@ -44,24 +56,41 @@ type portableHandleMap struct {
 }
 
 func (m *portableHandleMap) Register(obj *Handled) (handle uint64) {
-	if obj.check != 0 {
-		panic(_ALREADY_MSG)
-	}
 	m.Lock()
+	if obj.count == 0 {
+		if obj.check != 0 {
+			panic(_ALREADY_MSG)
+		}
 
-	if len(m.freeIds) == 0 {
-		handle = uint64(len(m.handles))
-		m.handles = append(m.handles, obj)
+		if len(m.freeIds) == 0 {
+			handle = uint64(len(m.handles))
+			m.handles = append(m.handles, obj)
+		} else {
+			handle = m.freeIds[len(m.freeIds)-1]
+			m.freeIds = m.freeIds[:len(m.freeIds)-1]
+			m.handles[handle] = obj
+		}
+		m.used++
+		obj.handle = handle
 	} else {
-		handle = m.freeIds[len(m.freeIds)-1]
-		m.freeIds = m.freeIds[:len(m.freeIds)-1]
-		m.handles[handle] = obj
+		handle = obj.handle
 	}
-	m.used++
+	obj.count++
 	m.Unlock()
 	return handle
 }
 
+func (m *portableHandleMap) Handle(obj *Handled) (h uint64) {
+	m.RLock()
+	if obj.count == 0 {
+		h = 0
+	} else {
+		h = obj.handle
+	}
+	m.RUnlock()
+	return h
+}
+
 func (m *portableHandleMap) Count() int {
 	m.RLock()
 	c := m.used
@@ -76,14 +105,21 @@ func (m *portableHandleMap) Decode(h uint64) *Handled {
 	return v
 }
 
-func (m *portableHandleMap) Forget(h uint64) *Handled {
+func (m *portableHandleMap) Forget(h uint64, count int) (forgotten bool, obj *Handled) {
 	m.Lock()
-	v := m.handles[h]
-	m.handles[h] = nil
-	m.freeIds = append(m.freeIds, h)
-	m.used--
+	obj = m.handles[h]
+	obj.count -= count
+	if obj.count < 0 {
+		panic("underflow")
+	} else if obj.count == 0 {
+		m.handles[h] = nil
+		m.freeIds = append(m.freeIds, h)
+		m.used--
+		forgotten = true
+		obj.handle = 0
+	} 
 	m.Unlock()
-	return v
+	return forgotten, obj
 }
 
 func (m *portableHandleMap) Has(h uint64) bool {
@@ -99,10 +135,15 @@ type int32HandleMap struct {
 	handles map[uint32]*Handled
 }
 
-func (m *int32HandleMap) Register(obj *Handled) uint64 {
+func (m *int32HandleMap) Register(obj *Handled) (handle uint64) {
 	m.mutex.Lock()
-	handle := uint32(uintptr(unsafe.Pointer(obj)))
-	m.handles[handle] = obj
+	h := uint32(uintptr(unsafe.Pointer(obj)))
+	if obj.count == 0 {
+		m.handles[h] = obj
+		obj.handle = uint64(h)
+	}
+	handle = uint64(h)
+	obj.count++
 	m.mutex.Unlock()
 	return uint64(handle)
 }
@@ -114,6 +155,15 @@ func (m *int32HandleMap) Has(h uint64) bool {
 	return ok
 }
 
+func (m *int32HandleMap) Handle(obj *Handled) uint64 {
+	if obj.count == 0 {
+		return 0
+	}
+	
+	h := uint32(uintptr(unsafe.Pointer(obj)))
+	return uint64(h)
+}
+
 func (m *int32HandleMap) Count() int {
 	m.mutex.Lock()
 	c := len(m.handles)
@@ -121,14 +171,21 @@ func (m *int32HandleMap) Count() int {
 	return c
 }
 
-func (m *int32HandleMap) Forget(handle uint64) *Handled {
-	val := m.Decode(handle)
+func (m *int32HandleMap) Forget(handle uint64, count int) (forgotten bool, obj *Handled) {
+	obj = m.Decode(handle)
 
 	m.mutex.Lock()
-	val.check = 0
-	delete(m.handles, uint32(handle))
+	obj.count -= count
+	if obj.count == 0 {
+		obj.check = 0
+		delete(m.handles, uint32(handle))
+		forgotten = true
+	} else if obj.count < 0 {
+		panic("underflow")
+	}
+	obj.handle = 0
 	m.mutex.Unlock()
-	return val
+	return forgotten, obj
 }
 
 func (m *int32HandleMap) Decode(handle uint64) *Handled {
@@ -196,42 +253,66 @@ func (m *int64HandleMap) Register(obj *Handled) (handle uint64) {
 	defer m.verify()
 
 	m.mutex.Lock()
-	defer m.mutex.Unlock()
+	if obj.count == 0 {
+		handle = uint64(uintptr(unsafe.Pointer(obj)))
 
-	handle = uint64(uintptr(unsafe.Pointer(obj)))
+		rest := (handle &^ (1<<48 - 1))
+		if rest != 0 {
+			panic("more than 48 bits in address")
+		}
+		if handle&0x7 != 0 {
+			panic("unaligned ptr")
+		}
+		handle >>= 3
 
-	rest := (handle &^ (1<<48 - 1))
-	if rest != 0 {
-		panic("more than 48 bits in address")
-	}
-	if handle&0x7 != 0 {
-		panic("unaligned ptr")
+		check := m.nextFree
+		m.nextFree++
+		m.nextFree = m.nextFree & (1<<(64-48+3) - 1)
+
+		handle |= uint64(check) << (48 - 3)
+		if obj.check != 0 {
+			panic(_ALREADY_MSG)
+		}
+		obj.check = check
+		m.handles[handle] = obj
+	} else {
+		handle = m.Handle(obj)
 	}
-	handle >>= 3
+	obj.count ++
+	m.mutex.Unlock()
 
-	check := m.nextFree
-	m.nextFree++
-	m.nextFree = m.nextFree & (1<<(64-48+3) - 1)
+	return handle
+}
 
-	handle |= uint64(check) << (48 - 3)
-	if obj.check != 0 {
-		panic(_ALREADY_MSG)
+func (m *int64HandleMap) Handle(obj *Handled) (handle uint64) {
+	if obj.count == 0 {
+		return 0
 	}
-	obj.check = check
-	m.handles[handle] = obj
+	
+	handle = uint64(uintptr(unsafe.Pointer(obj)))
+	handle >>= 3
+	handle |= uint64(obj.check) << (48 - 3)
 	return handle
 }
 
-func (m *int64HandleMap) Forget(handle uint64) (val *Handled) {
+
+func (m *int64HandleMap) Forget(handle uint64, count int) (forgotten bool, obj *Handled) {
 	defer m.verify()
 
-	val = m.Decode(handle)
+	obj = m.Decode(handle)
 
 	m.mutex.Lock()
-	delete(m.handles, handle)
-	val.check = 0
+	obj.count -= count
+	if obj.count == 0 {
+		delete(m.handles, handle)
+		obj.check = 0
+		obj.handle = 0
+		forgotten = true
+	} else if obj.count < 0 {
+		panic("underflow")
+	}
 	m.mutex.Unlock()
-	return val
+	return forgotten, obj
 }
 
 func (m *int64HandleMap) Has(handle uint64) bool {

fuse/handle_test.go

@@ -68,6 +68,7 @@ func TestHandleMapPointerLayout(t *testing.T) {
 
 func TestHandleMapBasic(t *testing.T) {
 	for _, portable := range []bool{true, false} {
+		t.Log("portable:", portable)
 		v := new(Handled)
 		hm := NewHandleMap(portable)
 		h := hm.Register(v)
@@ -75,13 +76,16 @@ func TestHandleMapBasic(t *testing.T) {
 		if !hm.Has(h) {
 			t.Fatal("Does not have handle")
 		}
+		if hm.Handle(v) != h {
+			t.Fatalf("handle mismatch, got %x want %x", hm.Handle(v), h)
+		}
 		if hm.Decode(h) != v {
 			t.Fatal("address mismatch")
 		}
 		if hm.Count() != 1 {
 			t.Fatal("count error")
 		}
-		hm.Forget(h)
+		hm.Forget(h, 1)
 		if hm.Count() != 0 {
 			t.Fatal("count error")
 		}

fuse/inode.go

@@ -39,22 +39,6 @@ type Inode struct {
 	// All data below is protected by treeLock.
 	children map[string]*Inode
 
-	// The nodeId is only used to communicate to the kernel.  If
-	// it is zero, it means the kernel does not know about this
-	// Inode.  Only forget/lookup/notify methods should nodeId
-	// directly.
-	nodeId uint64
-
-	// lookupCount registers how often the kernel got this inode
-	// back for a Lookup operation. This number is a reference
-	// count, and the Forget operation lists how many references to drop.
-	//
-	// The lookupCount is exclusively used for managing the
-	// lifetime of nodeId variable.  It is ok for a node to have 0
-	// == lookupCount.  This can happen if the inode return false
-	// for Deletable().
-	lookupCount int
-
 	// Non-nil if this inode is a mountpoint, ie. the Root of a
 	// NodeFileSystem.
 	mountPoint *fileSystemMount
@@ -236,12 +220,7 @@ func (n *Inode) getMountDirEntries() (out []DirEntry) {
 const initDirSize = 20
 
 func (n *Inode) verify(cur *fileSystemMount) {
-	if n.lookupCount < 0 {
-		log.Panicf("negative lookup count %d on node %d", n.lookupCount, n.nodeId)
-	}
-	if (n.lookupCount == 0) != (n.nodeId == 0) {
-		log.Panicf("kernel registration mismatch: lookup %d id %d", n.lookupCount, n.nodeId)
-	}
+	n.handled.verify()
 	if n.mountPoint != nil {
 		if n != n.mountPoint.mountInode {
 			log.Panicf("mountpoint mismatch %v %v", n, n.mountPoint.mountInode)

fuse/pathfs.go

@@ -262,7 +262,7 @@ func (n *pathInode) GetPath() string {
 
 	path := string(pathBytes)
 	if n.pathFs.Debug {
-		log.Printf("Inode %d = %q (%s)", n.Inode().nodeId, path, n.fs.String())
+		log.Printf("Inode %d = %q (%s)", n.Inode().handled.handle, path, n.fs.String())
 	}
 
 	return path