Skip to content

Commit

Permalink
Fix a memory leak in inodesByTime (evicted inodes were not removed fr…
Browse files Browse the repository at this point in the history
…om the map)
  • Loading branch information
vitalif committed Dec 6, 2023
1 parent 950ddf5 commit 811b7be
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
2 changes: 2 additions & 0 deletions internal/cluster_fs_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions internal/goofys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions internal/handles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 811b7be

Please sign in to comment.