From 811b7be4f171f1a59a3d3ced386cc88d596a478a Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Wed, 6 Dec 2023 12:41:44 +0300 Subject: [PATCH] Fix a memory leak in inodesByTime (evicted inodes were not removed from the map) --- internal/cluster_fs_grpc.go | 2 ++ internal/goofys.go | 7 +++---- internal/handles.go | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/cluster_fs_grpc.go b/internal/cluster_fs_grpc.go index ac9efb1a..d040c1e8 100644 --- a/internal/cluster_fs_grpc.go +++ b/internal/cluster_fs_grpc.go @@ -464,6 +464,8 @@ func (fs *ClusterFsGrpc) ForgetInode2(ctx context.Context, req *pb.ForgetInode2R inode.resetCache() inode.fs.mu.Lock() + // FIXME: These lines may have a bug (check). Only expired inodes should be forgotten + inode.resetExpireTime() delete(inode.fs.inodes, inode.Id) inode.fs.mu.Unlock() inode.fs.lfru.Forget(inode.Id) diff --git a/internal/goofys.go b/internal/goofys.go index c0f894a4..80997d36 100644 --- a/internal/goofys.go +++ b/internal/goofys.go @@ -834,6 +834,7 @@ func (fs *Goofys) EvictEntry(id fuseops.InodeID) bool { childTmp.SetCacheState(ST_DEAD) // Drop inode fs.mu.Lock() + childTmp.resetExpireTime() delete(fs.inodes, childTmp.Id) fs.forgotCnt += 1 fs.mu.Unlock() @@ -857,7 +858,8 @@ func (fs *Goofys) MetaEvictor() { } // Try to keep the number of cached inodes under control %) fs.mu.RLock() - toEvict := (len(fs.inodes)-fs.flags.EntryLimit)*2 + totalInodes := len(fs.inodes) + toEvict := (totalInodes-fs.flags.EntryLimit)*2 if toEvict < 0 { fs.mu.RUnlock() retry = false @@ -895,10 +897,7 @@ func (fs *Goofys) MetaEvictor() { seen[id] = true } } - fs.mu.RLock() - totalInodes := len(fs.inodes) retry = len(scan) >= toEvict && totalInodes > fs.flags.EntryLimit - fs.mu.RUnlock() atomic.AddInt64(&fs.stats.evicts, int64(evicted)) if len(scan) > 0 { log.Debugf("metadata cache: alive %v, scanned %v, evicted %v", totalInodes, len(scan), evicted) diff --git a/internal/handles.go b/internal/handles.go index 7cd0c52d..58f19234 100644 --- a/internal/handles.go +++ b/internal/handles.go @@ -399,6 +399,7 @@ func (inode *Inode) DeRef(n int64) (stale bool) { if res == 0 && inode.CacheState <= ST_DEAD { inode.resetCache() inode.fs.mu.Lock() + inode.resetExpireTime() delete(inode.fs.inodes, inode.Id) inode.fs.forgotCnt += 1 inode.fs.mu.Unlock() @@ -446,6 +447,20 @@ func (inode *Inode) SetExpireTime(tm time.Time) { inode.fs.mu.Unlock() } +// LOCKS_REQUIRED(inode.mu) +// LOCKS_REQUIRED(inode.fs.mu) +func (inode *Inode) resetExpireTime() { + oldTime := inode.ExpireTime.Unix() + inode.ExpireTime = time.Time{} + oldMap := inode.fs.inodesByTime[oldTime] + if oldMap != nil { + delete(oldMap, inode.Id) + if len(oldMap) == 0 { + delete(inode.fs.inodesByTime, oldTime) + } + } +} + // LOCKS_EXCLUDED(inode.mu) // LOCKS_EXCLUDED(inode.fs.mu) func (inode *Inode) SetExpireLocked(tm time.Time) {