Merge pull request #8268 from tanyifeng:limit_cache

PiperOrigin-RevId: 495410526

pkg/sentry/fsimpl/gofer/BUILD

@@ -3,6 +3,18 @@ load("//tools/go_generics:defs.bzl", "go_template_instance")
 
 licenses(["notice"])
 
+go_template_instance(
+    name = "string_list",
+    out = "string_list.go",
+    package = "gofer",
+    prefix = "string",
+    template = "//pkg/ilist:generic_list",
+    types = {
+        "Element": "*stringListElem",
+        "Linker": "*stringListElem",
+    },
+)
+
 go_template_instance(
     name = "dentry_list",
     out = "dentry_list.go",
@@ -55,6 +67,7 @@ go_library(
         "socket.go",
         "special_fd_list.go",
         "special_file.go",
+        "string_list.go",
         "symlink.go",
         "time.go",
     ],

pkg/sentry/fsimpl/gofer/directory.go

@@ -70,6 +70,9 @@ func (d *dentry) cacheNewChildLocked(child *dentry, name string) {
 	child.name = name
 	if d.children == nil {
 		d.children = make(map[string]*dentry)
+	} else if c, ok := d.children[name]; ok && c == nil {
+		// This child will not be negative, decrease count of negativeChildren.
+		d.negativeChildren--
 	}
 	d.children[name] = child
 }
@@ -77,6 +80,7 @@ func (d *dentry) cacheNewChildLocked(child *dentry, name string) {
 // Preconditions:
 //   - d.dirMu must be locked.
 //   - d.isDir().
+//   - name is not already a negative entry.
 func (d *dentry) cacheNegativeLookupLocked(name string) {
 	// Don't cache negative lookups if InteropModeShared is in effect (since
 	// this makes remote lookup unavoidable), or if d.isSynthetic() (in which
@@ -90,6 +94,26 @@ func (d *dentry) cacheNegativeLookupLocked(name string) {
 		d.children = make(map[string]*dentry)
 	}
 	d.children[name] = nil
+	d.negativeChildren++
+
+	if !d.negativeChildrenCache.isInited() {
+		// Initializing cache with all negative children name at the first time
+		// that negativeChildren increase upto max.
+		if d.negativeChildren >= maxCachedNegativeChildren {
+			d.negativeChildrenCache.init(maxCachedNegativeChildren)
+			for childName, child := range d.children {
+				if child == nil {
+					d.negativeChildrenCache.add(childName)
+				}
+			}
+		}
+	} else if victim := d.negativeChildrenCache.add(name); victim != "" {
+		// If victim is a negative entry in d.children, delete it.
+		if child, ok := d.children[victim]; ok && child == nil {
+			delete(d.children, victim)
+			d.negativeChildren--
+		}
+	}
 }
 
 type createSyntheticOpts struct {

pkg/sentry/fsimpl/gofer/filesystem.go

@@ -586,6 +586,7 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
 		if child, ok := parent.children[name]; ok && child == nil {
 			// Delete the now-stale negative dentry.
 			delete(parent.children, name)
+			parent.negativeChildren--
 		}
 		parent.touchCMtime()
 		parent.clearDirentsLocked()
@@ -1604,25 +1605,16 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
 		ds = appendDentry(ds, replaced)
 	}
 	oldParent.cacheNegativeLookupLocked(oldName)
-	// We don't use newParent.cacheNewChildLocked() since we don't want to mess
-	// with reference counts and queue oldParent for checkCachingLocked if the
-	// parent isn't actually changing.
+	if renamed.isSynthetic() {
+		oldParent.syntheticChildren--
+		newParent.syntheticChildren++
+	}
+	newParent.cacheNewChildLocked(renamed, newName)
+	oldParent.decRefNoCaching()
 	if oldParent != newParent {
-		oldParent.decRefNoCaching()
-		newParent.IncRef()
 		ds = appendDentry(ds, newParent)
 		ds = appendDentry(ds, oldParent)
-		if renamed.isSynthetic() {
-			oldParent.syntheticChildren--
-			newParent.syntheticChildren++
-		}
-		renamed.parent = newParent
 	}
-	renamed.name = newName
-	if newParent.children == nil {
-		newParent.children = make(map[string]*dentry)
-	}
-	newParent.children[newName] = renamed
 
 	// Update metadata.
 	if renamed.cachedMetadataAuthoritative() {

pkg/sentry/fsimpl/gofer/gofer.go

@@ -95,7 +95,45 @@ const (
 	cacheRemoteRevalidating  = "remote_revalidating"
 )
 
-const defaultMaxCachedDentries = 1000
+const (
+	defaultMaxCachedDentries  = 1000
+	maxCachedNegativeChildren = 1000
+)
+
+// stringFixedCache is a fixed sized cache, once initialized,
+// its size never changes.
+//
+// +stateify savable
+type stringFixedCache struct {
+	// namesList stores negative names with fifo list.
+	// name stored in namesList only means it used to be negative
+	// at the moment you pushed it to the list.
+	namesList stringList
+	size      uint64
+}
+
+func (cache *stringFixedCache) isInited() bool {
+	return cache.size != 0
+}
+
+func (cache *stringFixedCache) init(size uint64) {
+	elements := make([]stringListElem, size)
+	for i := uint64(0); i < size; i++ {
+		cache.namesList.PushFront(&elements[i])
+	}
+	cache.size = size
+}
+
+// Update will push name to the front of the list,
+// and pop the tail value.
+func (cache *stringFixedCache) add(name string) string {
+	tail := cache.namesList.Back()
+	victimName := tail.str
+	tail.str = name
+	cache.namesList.Remove(tail)
+	cache.namesList.PushFront(tail)
+	return victimName
+}
 
 // +stateify savable
 type dentryCache struct {
@@ -879,6 +917,13 @@ type dentry struct {
 	// children is protected by dirMu.
 	children map[string]*dentry
 
+	// If this dentry represents a directory, negativeChildrenCache cache
+	// names of negative children, negativeChildrenCache is protected by dirMu.
+	negativeChildrenCache stringFixedCache
+	// If this dentry represents a directory, negativeChildren is the number
+	// of negative child, negativeChildren is protected by dirMu.
+	negativeChildren int
+
 	// If this dentry represents a directory, syntheticChildren is the number
 	// of child dentries for which dentry.isSynthetic() == true.
 	// syntheticChildren is protected by dirMu.
@@ -1016,6 +1061,13 @@ type dentry struct {
 	watches vfs.Watches
 }
 
+// +stateify savable
+type stringListElem struct {
+	// str is the string that this elem represents.
+	str string
+	stringEntry
+}
+
 // +stateify savable
 type dentryListElem struct {
 	// d is the dentry that this elem represents.

pkg/sentry/fsimpl/gofer/gofer_test.go

@@ -65,3 +65,25 @@ func TestDestroyIdempotent(t *testing.T) {
 	child.checkCachingLocked(ctx, true /* renameMuWriteLocked */)
 	child.checkCachingLocked(ctx, true /* renameMuWriteLocked */)
 }
+
+func TestStringFixedCache(t *testing.T) {
+	names := []string{"a", "b", "c"}
+	cache := stringFixedCache{}
+
+	cache.init(uint64(len(names)))
+	if inited := cache.isInited(); !inited {
+		t.Fatalf("cache.isInited(): %v, want: true", inited)
+	}
+	for _, s := range names {
+		victim := cache.add(s)
+		if victim != "" {
+			t.Fatalf("cache.add(): %v, want: \"\"", victim)
+		}
+	}
+	for _, s := range names {
+		victim := cache.add("something")
+		if victim != s {
+			t.Fatalf("cache.add(): %v, want: %v", victim, s)
+		}
+	}
+}