diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD index 182317e26..c3d6e886d 100644 --- a/pkg/sentry/fsimpl/gofer/BUILD +++ b/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", ], diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index 056da2048..123dc07aa 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/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 { diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 0085eed75..932d74283 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/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() { diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index d72f8b38c..dc23e4e53 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/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. diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index 58a06e9fa..8f5b551ad 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/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) + } + } +}