mount: stop churning the inode table on every readdir (#10606)

* mount: readdir enters a child in the inode table only when it takes a reference

Only readdirplus into the kernel takes a reference on the children it
reports, and only that reference brings a FORGET later to take the entry
back out. Every other listing was inserting all its children anyway.

On WinFsp that meant a listing looked each child up, took a reference,
and immediately gave it back, so a walk of a wide directory paid three
write-lock acquisitions per entry to leave the table exactly as it found
it. On a plain kernel readdir nothing gives the entry back at all, so
listing a directory of 200k files grew both maps by 200k entries that
were never reclaimed.

A dirent's inode number is informational either way: the kernel must
LOOKUP before it can use a nodeid, and the WinFsp adapter re-resolves
every operation by path. So report the number and let the mapping be
built when something actually looks the entry up.

* mount: take the readdirplus reference without a second full lookup

The entry has just been resolved a few lines above, so redoing the whole
lookup only rebuilds the child path and walks both maps again to reach a
counter. Bump it directly, falling back to the full lookup if a Forget
removed the entry in between.

* mount: benchmark a readdir over a 200k directory

Drives doReadDirectory against a meta cache holding 200k entries, one
round of 4096 at a time, for the three front ends that behave
differently: a plain kernel readdir, kernel readdirplus, and a WinFsp
listing that gets attributes but never returns a reference. Reports what
each leaves behind in the inode table alongside the usual metrics.

The sink declares TakesLookupRef as an ordinary method rather than
through the interface, so the same file runs unchanged against an older
tree for comparison.

* mount: stamp an inode on the benchmark's entries

The filer stores one on every entry it writes, so a real listing arrives
with an inode and never derives its own. Leaving it zero made every child
in the benchmark fall through to the MD5 in AsInode, work no filer-backed
mount does, and charged it to both sides of the comparison.

weed/mount/dir_sink.go

@@ -15,6 +15,10 @@ type DirEntrySink interface {
 	// AddEntryPlus is AddEntry for readdirplus, returning the attribute block
 	// to fill in, or nil once the sink is full.
 	AddEntryPlus(entry fuse.DirEntry) *fuse.EntryOut
+
+	// TakesLookupRef reports whether AddEntryPlus hands the sink a reference the
+	// mount must hold until a FORGET returns it.
+	TakesLookupRef() bool
 }
 
 // fuseDirEntryList adapts the kernel reply buffer to DirEntrySink.
@@ -30,6 +34,8 @@ func (l fuseDirEntryList) AddEntryPlus(entry fuse.DirEntry) *fuse.EntryOut {
 	return l.AddDirLookupEntry(entry)
 }
 
+func (l fuseDirEntryList) TakesLookupRef() bool { return true }
+
 // ReadDirectoryInto runs a readdir against sink. ReadDir and ReadDirPlus are
 // this with the kernel reply buffer as the sink.
 func (wfs *WFS) ReadDirectoryInto(input *fuse.ReadIn, sink DirEntrySink, isPlusMode bool) fuse.Status {

weed/mount/inode_to_path.go

@@ -137,6 +137,35 @@ func (i *InodeToPath) Lookup(path util.FullPath, unixTime int64, isDirectory boo
 	return inode
 }
 
+// IncrementNlookup takes one more reference on an inode already in the table,
+// reporting false if it is not there.
+func (i *InodeToPath) IncrementNlookup(inode uint64) bool {
+	i.Lock()
+	defer i.Unlock()
+	entry, found := i.inode2path[inode]
+	if !found {
+		return false
+	}
+	entry.nlookup++
+	return true
+}
+
+// InodeForListing returns the inode number a readdir should report for path
+// without entering it in the table. Nothing is reserved, so the collision probe
+// Lookup does is skipped: the worst case is a repeated st_ino in one listing.
+func (i *InodeToPath) InodeForListing(path util.FullPath, unixTime int64, possibleInode uint64) uint64 {
+	i.RLock()
+	inode, found := i.path2inode[path]
+	i.RUnlock()
+	if found {
+		return inode
+	}
+	if possibleInode != 0 {
+		return possibleInode
+	}
+	return path.AsInode(unixTime)
+}
+
 func (i *InodeToPath) AllocateInode(path util.FullPath, unixTime int64) uint64 {
 	if path == "/" {
 		return 1

weed/mount/weedfs_dir_read.go

@@ -170,12 +170,21 @@ func (wfs *WFS) doReadDirectory(input *fuse.ReadIn, out DirEntrySink, isPlusMode
 	wfs.inodeToPath.TouchDirectory(dirPath)
 
 	var dirEntry fuse.DirEntry
+	// Only a reference makes a child worth entering in the inode table: without
+	// one nothing ever arrives to take the entry back out again.
+	takesLookupRef := isPlusMode && out.TakesLookupRef()
 
 	// index is the position in entryStream, used to calculate the offset for next readdir
 	processEachEntryFn := func(entry *filer.Entry, index int64) bool {
 		dirEntry.Name = entry.Name()
 		dirEntry.Mode = toSyscallMode(entry.Mode)
-		inode := wfs.inodeToPath.Lookup(dirPath.Child(dirEntry.Name), entry.Crtime.Unix(), entry.IsDirectory(), len(entry.HardLinkId) > 0, entry.Inode, false)
+		childPath := dirPath.Child(dirEntry.Name)
+		var inode uint64
+		if takesLookupRef {
+			inode = wfs.inodeToPath.Lookup(childPath, entry.Crtime.Unix(), entry.IsDirectory(), len(entry.HardLinkId) > 0, entry.Inode, false)
+		} else {
+			inode = wfs.inodeToPath.InodeForListing(childPath, entry.Crtime.Unix(), entry.Inode)
+		}
 		dirEntry.Ino = inode
 
 		// Set Off to the next offset so client can resume from correct position
@@ -191,11 +200,15 @@ func (wfs *WFS) doReadDirectory(input *fuse.ReadIn, out DirEntrySink, isPlusMode
 				return false
 			}
 			if fh, found := wfs.fhMap.FindFileHandle(inode); found {
-				glog.V(4).Infof("readdir opened file %s", dirPath.Child(dirEntry.Name))
+				glog.V(4).Infof("readdir opened file %s", childPath)
 				entry = filer.FromPbEntry(string(dirPath), fh.GetEntry().GetEntry())
 			}
 			wfs.outputFilerEntry(entryOut, inode, entry)
-			wfs.inodeToPath.Lookup(dirPath.Child(dirEntry.Name), entry.Crtime.Unix(), entry.IsDirectory(), len(entry.HardLinkId) > 0, entry.Inode, true)
+			// Taken only once the entry is really in the sink, so one that did not
+			// fit leaves no reference behind. The fallback covers a racing Forget.
+			if takesLookupRef && !wfs.inodeToPath.IncrementNlookup(inode) {
+				wfs.inodeToPath.Lookup(childPath, entry.Crtime.Unix(), entry.IsDirectory(), len(entry.HardLinkId) > 0, entry.Inode, true)
+			}
 		}
 		return true
 	}

weed/mount/weedfs_dir_read_bench_test.go

@@ -0,0 +1,222 @@
+package mount
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/go-fuse/v2/fuse"
+	"github.com/seaweedfs/seaweedfs/weed/filer"
+	"github.com/seaweedfs/seaweedfs/weed/mount/meta_cache"
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+)
+
+const benchDirEntryCount = 200000
+
+// benchSink drains a readdir the way a front end does. sinkLimit is how many
+// entries one round accepts, standing in for the kernel reply buffer or the
+// WinFsp adapter's batch.
+type benchSink struct {
+	plus      bool
+	takesRef  bool
+	sinkLimit int
+	count     int
+	inodes    []uint64
+	lastOff   uint64
+	attrs     []fuse.EntryOut
+}
+
+func (s *benchSink) reset() {
+	s.count = 0
+	s.inodes = s.inodes[:0]
+	s.attrs = s.attrs[:0]
+}
+
+func (s *benchSink) AddEntry(entry fuse.DirEntry) bool {
+	if s.count >= s.sinkLimit {
+		return false
+	}
+	s.count++
+	s.lastOff = entry.Off
+	s.inodes = append(s.inodes, entry.Ino)
+	return true
+}
+
+func (s *benchSink) AddEntryPlus(entry fuse.DirEntry) *fuse.EntryOut {
+	if s.count >= s.sinkLimit {
+		return nil
+	}
+	s.count++
+	s.lastOff = entry.Off
+	s.inodes = append(s.inodes, entry.Ino)
+	s.attrs = append(s.attrs, fuse.EntryOut{})
+	return &s.attrs[len(s.attrs)-1]
+}
+
+func (s *benchSink) TakesLookupRef() bool { return s.takesRef }
+
+func inodeTableSize(i *InodeToPath) int {
+	i.RLock()
+	defer i.RUnlock()
+	return len(i.inode2path)
+}
+
+func newBenchWFS(tb testing.TB, dir util.FullPath, n int) *WFS {
+	tb.Helper()
+
+	uidGidMapper, err := meta_cache.NewUidGidMapper("", "")
+	if err != nil {
+		tb.Fatalf("uid/gid mapper: %v", err)
+	}
+
+	root := util.FullPath("/")
+	option := &Option{
+		ChunkSizeLimit:     1024,
+		ConcurrentReaders:  1,
+		VolumeServerAccess: "filerProxy",
+		FilerAddresses:     []pb.ServerAddress{pb.NewServerAddressWithGrpcPort("127.0.0.1:1", 1)},
+		GrpcDialOption:     grpc.WithTransportCredentials(insecure.NewCredentials()),
+		FilerMountRootPath: "/",
+		MountUid:           99,
+		MountGid:           100,
+		MountMode:          0o777,
+		MountMtime:         time.Now(),
+		MountCtime:         time.Now(),
+		UidGidMapper:       uidGidMapper,
+	}
+
+	wfs := &WFS{
+		option:            option,
+		signature:         1,
+		inodeToPath:       NewInodeToPath(root, 0),
+		fhMap:             NewFileHandleToInode(),
+		dhMap:             NewDirectoryHandleToInode(),
+		fhLockTable:       util.NewLockTable[FileHandleId](),
+		hardLinkLockTable: util.NewLockTable[string](),
+	}
+	wfs.metaCache = meta_cache.NewMetaCache(
+		filepath.Join(tb.TempDir(), "meta"),
+		uidGidMapper,
+		root,
+		false,
+		func(path util.FullPath) { wfs.inodeToPath.MarkChildrenCached(path) },
+		func(path util.FullPath) bool { return wfs.inodeToPath.IsChildrenCached(path) },
+		func(meta_cache.EntryInvalidation) {},
+		nil,
+	)
+	tb.Cleanup(wfs.metaCache.Shutdown)
+
+	now := time.Now()
+	ctx := context.Background()
+	if err := wfs.metaCache.InsertEntry(ctx, &filer.Entry{
+		FullPath: dir,
+		Attr:     filer.Attr{Mode: os.ModeDir | 0o755, Mtime: now, Crtime: now, Uid: 99, Gid: 100},
+	}, 0); err != nil {
+		tb.Fatalf("insert dir: %v", err)
+	}
+	for i := 0; i < n; i++ {
+		child := dir.Child(fmt.Sprintf("image-%08d.jpg", i))
+		if err := wfs.metaCache.InsertEntry(ctx, &filer.Entry{
+			FullPath: child,
+			// The filer stamps an inode on every entry it stores, so a listing
+			// arrives with one and never has to derive its own.
+			Attr: filer.Attr{Mode: 0o644, Mtime: now, Crtime: now, Uid: 99, Gid: 100, FileSize: 4096, Inode: child.AsInode(now.Unix())},
+		}, 0); err != nil {
+			tb.Fatalf("insert entry %d: %v", i, err)
+		}
+	}
+
+	// Mark the tree cached so the listing is served from the meta cache and no
+	// filer client is dialled.
+	wfs.inodeToPath.MarkChildrenCached(root)
+	wfs.inodeToPath.Lookup(dir, now.Unix(), true, false, 0, true)
+	wfs.inodeToPath.MarkChildrenCached(dir)
+
+	return wfs
+}
+
+// walkOnce enumerates the whole directory the way a front end does: repeated
+// rounds against one handle until the listing runs dry, returning whatever
+// references the round took.
+func walkOnce(tb testing.TB, wfs *WFS, dirInode uint64, sink *benchSink, forgets bool) int {
+	dhid, _ := wfs.AcquireDirectoryHandle()
+	defer wfs.ReleaseDirectoryHandle(dhid)
+
+	total := 0
+	offset := uint64(0)
+	for {
+		sink.reset()
+		status := wfs.doReadDirectory(&fuse.ReadIn{
+			InHeader: fuse.InHeader{NodeId: dirInode},
+			Fh:       uint64(dhid),
+			Offset:   offset,
+			Size:     1 << 20,
+		}, sink, sink.plus)
+		if status != fuse.OK {
+			tb.Fatalf("readdir: %v", status)
+		}
+		if sink.count == 0 {
+			return total
+		}
+		total += sink.count
+		if forgets {
+			// HasInode keeps this to the entries that really hold a reference,
+			// so a listing that took none is not charged for a bogus Forget.
+			for _, ino := range sink.inodes {
+				if wfs.inodeToPath.HasInode(ino) {
+					wfs.Forget(ino, 1)
+				}
+			}
+		}
+		if sink.lastOff <= offset {
+			return total
+		}
+		offset = sink.lastOff
+	}
+}
+
+var benchCases = []struct {
+	name string
+	plus bool
+	// ref mirrors the sink's TakesLookupRef. The kernel's reports true whatever
+	// the mode, exactly as fuseDirEntryList does; it is the mode that decides
+	// whether a reference is actually granted.
+	ref bool
+	// forgets is what the front end does after a round: the kernel returns one
+	// FORGET per readdirplus entry and none for a plain readdir, and the WinFsp
+	// adapter hands back everything a round gave it.
+	forgets bool
+}{
+	{"kernel_readdir", false, true, false},
+	{"kernel_readdirplus", true, true, true},
+	{"winfsp_readdirplus", true, false, true},
+}
+
+func BenchmarkReadDirectory(b *testing.B) {
+	dir := util.FullPath("/images")
+	for _, tc := range benchCases {
+		b.Run(tc.name, func(b *testing.B) {
+			wfs := newBenchWFS(b, dir, benchDirEntryCount)
+			dirInode, _ := wfs.inodeToPath.GetInode(dir)
+			sink := &benchSink{plus: tc.plus, takesRef: tc.ref, sinkLimit: 4096}
+
+			b.ResetTimer()
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				if got := walkOnce(b, wfs, dirInode, sink, tc.forgets); got != benchDirEntryCount+2 {
+					b.Fatalf("listed %d entries, want %d", got, benchDirEntryCount+2)
+				}
+			}
+			b.StopTimer()
+			// What the listing left in the inode table, over the root and the
+			// directory itself.
+			b.ReportMetric(float64(inodeTableSize(wfs.inodeToPath)), "inodes_left")
+		})
+	}
+}

weed/mount/winfsp/fs_windows.go

@@ -651,6 +651,9 @@ func (s *readdirSink) AddEntryPlus(entry fuse.DirEntry) *fuse.EntryOut {
 	return out
 }
 
+// WinFsp has no FORGET, and every EntryOut is converted before the round ends.
+func (s *readdirSink) TakesLookupRef() bool { return false }
+
 func (w *WinFS) Readdir(path string, fill func(name string, stat *cgofuse.Stat_t, ofst int64) bool, ofst int64, fh uint64) int {
 	inode := w.inodeForHandle(w.dirInodes, fh)
 	if inode == 0 {
@@ -699,12 +702,6 @@ func (w *WinFS) Readdir(path string, fill func(name string, stat *cgofuse.Stat_t
 				filled = false
 			}
 		}
-		// A readdirplus entry carries a reference of its own. Give every one
-		// of them back, including any the fill above stopped short of, or a
-		// single walk of a wide directory strands one reference per child.
-		for _, child := range sink.inodes {
-			w.forget(child)
-		}
 		if !filled {
 			return 0
 		}