summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorAyush Ranjan <ayushranjan@google.com>2021-05-01 12:36:09 -0700
committergVisor bot <gvisor-bot@google.com>2021-05-01 12:37:49 -0700
commit7cafac9f42a8355e7192b57fcc8d41e31d836c53 (patch)
tree8911b4dc0d124eb00a797cf203ba200080293173
parentcde8e8b7a905ce53fe72a1a71b86e9ef352bbc3a (diff)
[perf] Check caching on IncRef'd dentries before the others.
When a child is added to a parent (directory) dentry, both child and parent are queued for checkCachingLocked(). Make sure that the parent is queued first because the parent gained a ref and so could be removed from the LRU cache hence making space for the new child. This could prevent an LRU cache eviction. In practice, this did seem to help. ~800 RPCs were reduced while building //absl/... (ABSL build benchmark). Evictions hurt in 2 ways - create renameMu contention and destroy a possibly useful dentry which will have to be re-walked and re-opened later. Follow up fix for #5859. PiperOrigin-RevId: 371509392
-rw-r--r--pkg/sentry/fsimpl/gofer/filesystem.go22
1 files changed, 14 insertions, 8 deletions
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index c1c9ec008..97ce80853 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -117,6 +117,17 @@ func appendDentry(ds *[]*dentry, d *dentry) *[]*dentry {
return ds
}
+// Precondition: !parent.isSynthetic() && !child.isSynthetic().
+func appendNewChildDentry(ds **[]*dentry, parent *dentry, child *dentry) {
+ // The new child was added to parent and took a ref on the parent (hence
+ // parent can be removed from cache). A new child has 0 refs for now. So
+ // checkCachingLocked() should be called on both. Call it first on the parent
+ // as it may create space in the cache for child to be inserted - hence
+ // avoiding a cache eviction.
+ *ds = appendDentry(*ds, parent)
+ *ds = appendDentry(*ds, child)
+}
+
// Preconditions: ds != nil.
func putDentrySlice(ds *[]*dentry) {
// Allow dentries to be GC'd.
@@ -258,11 +269,7 @@ func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name s
return nil, err
}
parent.cacheNewChildLocked(child, name)
- // For now, child has 0 references, so our caller should call
- // child.checkCachingLocked(). parent gained a ref so we should also call
- // parent.checkCachingLocked() so it can be removed from the cache if needed.
- *ds = appendDentry(*ds, child)
- *ds = appendDentry(*ds, parent)
+ appendNewChildDentry(ds, parent, child)
return child, nil
}
@@ -1114,7 +1121,6 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
}
return nil, err
}
- *ds = appendDentry(*ds, child)
// Incorporate the fid that was opened by lcreate.
useRegularFileFD := child.fileType() == linux.S_IFREG && !d.fs.opts.regularFilesUseSpecialFileFD
if useRegularFileFD {
@@ -1138,7 +1144,7 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
}
// Insert the dentry into the tree.
d.cacheNewChildLocked(child, name)
- *ds = appendDentry(*ds, d)
+ appendNewChildDentry(ds, d, child)
if d.cachedMetadataAuthoritative() {
d.touchCMtime()
d.dirents = nil
@@ -1333,9 +1339,9 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
// parent isn't actually changing.
if oldParent != newParent {
oldParent.decRefNoCaching()
- ds = appendDentry(ds, oldParent)
newParent.IncRef()
ds = appendDentry(ds, newParent)
+ ds = appendDentry(ds, oldParent)
if renamed.isSynthetic() {
oldParent.syntheticChildren--
newParent.syntheticChildren++