nodefs: embed Inode directly in DefaultOperations

This is an implementation detail, but it saves an indirection

nodefs/api.go

@@ -84,7 +84,7 @@ type Operations interface {
 	//
 	// See InodeOf for public API to retrieve an inode from Node.
 	inode() *Inode
-	setInode(*Inode) bool
+	init(ops Operations, attr NodeAttr, bridge *rawBridge, persistent bool)
 
 	// Inode() is a convenience method, and is equivalent to
 	// InodeOf(ops) It is provided by DefaultOperations, and

nodefs/bridge.go

@@ -56,9 +56,20 @@ func (b *rawBridge) newInode(ctx context.Context, ops Operations, id NodeAttr, p
 		log.Panicf("using reserved ID %d for inode number", id.Ino)
 	}
 
+	// This ops already was populated. Just return it.
+	if ops.inode().bridge != nil {
+		return ops.inode()
+	}
+
 	if id.Ino == 0 {
+		for {
 			id.Ino = b.automaticIno
 			b.automaticIno++
+			_, ok := b.nodes[id.Ino]
+			if !ok {
+				break
+			}
+		}
 	}
 
 	// the same node can be looked up through 2 paths in parallel, eg.
@@ -76,6 +87,7 @@ func (b *rawBridge) newInode(ctx context.Context, ops Operations, id NodeAttr, p
 	if old != nil {
 		return old
 	}
+
 	id.Mode = id.Mode &^ 07777
 	if id.Mode == 0 {
 		id.Mode = fuse.S_IFREG
@@ -92,26 +104,10 @@ func (b *rawBridge) newInode(ctx context.Context, ops Operations, id NodeAttr, p
 		log.Panicf("filetype %o unimplemented", id.Mode)
 	}
 
-	inode := &Inode{
-		ops:        ops,
-		nodeAttr:   id,
-		bridge:     b,
-		persistent: persistent,
-		parents:    make(map[parentData]struct{}),
-	}
-	if id.Mode == fuse.S_IFDIR {
-		inode.children = make(map[string]*Inode)
-	}
-
-	b.nodes[id.Ino] = inode
-	ops.setInode(inode)
-	newIno := ops.inode()
-
-	if newIno == inode {
-		newIno.ops.OnAdd(ctx)
-	}
-
-	return newIno
+	b.nodes[id.Ino] = ops.inode()
+	ops.init(ops, id, b, persistent)
+	ops.OnAdd(ctx)
+	return ops.inode()
 }
 
 // addNewChild inserts the child into the tree. Returns file handle if file != nil.
@@ -173,18 +169,16 @@ func NewNodeFS(root DirOperations, opts *Options) fuse.RawFileSystem {
 		bridge.options.AttrTimeout = &oneSec
 	}
 
-	bridge.root = &Inode{
-		lookupCount: 1,
-		children:    make(map[string]*Inode),
-		parents:     nil,
-		ops:         root,
-		bridge:      bridge,
-		nodeAttr: NodeAttr{
+	root.init(root,
+		NodeAttr{
 			Ino:  1,
 			Mode: fuse.S_IFDIR,
 		},
-	}
-	root.setInode(bridge.root)
+		bridge,
+		false,
+	)
+	bridge.root = root.inode()
+	bridge.root.lookupCount = 1
 	bridge.nodes = map[uint64]*Inode{
 		1: bridge.root,
 	}

nodefs/default.go

@@ -6,9 +6,7 @@ package nodefs
 
 import (
 	"context"
-	"sync/atomic"
 	"syscall"
-	"unsafe"
 
 	"github.com/hanwen/go-fuse/fuse"
 	"github.com/hanwen/go-fuse/internal"
@@ -22,43 +20,32 @@ import (
 //
 // It must be embedded in any Operations implementation.
 type DefaultOperations struct {
-	inode_ *Inode
+	inode_ Inode
 }
 
 // check that we have implemented all interface methods
 var _ Operations = &DefaultOperations{}
 
-// set/retrieve inode.
-//
-// node -> inode association, can be simultaneously tried to be set, if for e.g.
-//
-//         root
-//         /  \
-//       dir1 dir2
-//         \  /
-//         file
-//
-// dir1.Lookup("file") and dir2.Lookup("file") are executed simultaneously.
-//
-// If not using NodeAttr, the mapping in rawBridge does not help. So,
-// use atomics so that only one set can win.
-//
-// To read node.inode atomic.LoadPointer is used, however it is not expensive
-// since it translates to regular MOVQ on amd64.
-func (n *DefaultOperations) setInode(inode *Inode) bool {
-	return atomic.CompareAndSwapPointer(
-		(*unsafe.Pointer)(unsafe.Pointer(&n.inode_)),
-		nil, unsafe.Pointer(inode))
+func (n *DefaultOperations) inode() *Inode {
+	return &n.inode_
 }
 
-func (n *DefaultOperations) inode() *Inode {
-	return (*Inode)(atomic.LoadPointer(
-		(*unsafe.Pointer)(unsafe.Pointer(&n.inode_))))
+func (n *DefaultOperations) init(ops Operations, attr NodeAttr, bridge *rawBridge, persistent bool) {
+	n.inode_ = Inode{
+		ops:        ops,
+		nodeAttr:   attr,
+		bridge:     bridge,
+		persistent: persistent,
+		parents:    make(map[parentData]struct{}),
+	}
+	if attr.Mode == fuse.S_IFDIR {
+		n.inode_.children = make(map[string]*Inode)
+	}
 }
 
 // Inode is syntactic sugar for InodeOf(ops).
 func (n *DefaultOperations) Inode() *Inode {
-	return n.inode()
+	return &n.inode_
 }
 
 // StatFs zeroes the out argument and returns OK.  This is because OSX

nodefs/inode.go

@@ -285,17 +285,17 @@ func (n *Inode) ForgetPersistent() {
 	n.removeRef(0, true)
 }
 
-// NewInode returns an inode for the given Operations. The mode should be
-// standard mode argument (eg. S_IFDIR). The opaqueID argument, if
-// non-zero, is used to implement hard-links.  If opaqueID is given,
-// and another node with the same ID is known, that will node will be
+// NewInode returns an inode for the given Operations. The mode should
+// be standard mode argument (eg. S_IFDIR). The inode number in id.Ino
+// argument is used to implement hard-links.  If it is given, and
+// another node with the same ID is known, that will node will be
 // returned, and the passed-in `node` is ignored.
-func (n *Inode) NewInode(ctx context.Context, node Operations, id NodeAttr) *Inode {
-	return n.newInode(ctx, node, id, false)
+func (n *Inode) NewInode(ctx context.Context, ops Operations, id NodeAttr) *Inode {
+	return n.newInode(ctx, ops, id, false)
 }
 
-func (n *Inode) newInode(ctx context.Context, node Operations, id NodeAttr, persistent bool) *Inode {
-	return n.bridge.newInode(ctx, node, id, persistent)
+func (n *Inode) newInode(ctx context.Context, ops Operations, id NodeAttr, persistent bool) *Inode {
+	return n.bridge.newInode(ctx, ops, id, persistent)
 }
 
 // removeRef decreases references. Returns if this operation caused