diff options
author | Ayush Ranjan <ayushranjan@google.com> | 2021-04-29 16:09:49 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-04-29 16:12:08 -0700 |
commit | 9ff0d382d69c53a8fc916cfde844f1e657759f59 (patch) | |
tree | 0f5372d3488bdc1b55b6d8610b6dfb27b73ac55b /pkg | |
parent | eefa00f4aecc7c72a1866357fbd1bbb58aa6fc5e (diff) |
[perf] Remove unnecessary existence checks in doCreateAt().
Originally we were making a WalkGetAttrOne RPC to confirm that a file does not
exist on the remote filesystem - when there was no cached information about the
existence of a dentry at that position.
This change avoids making that RPC and speculatively makes the
mkdir/mknod/linkat/symlink RPC. They will fail with EEXIST if a file exists at
that position as we want.
However the error ordering is important. Existence check comes before
writability check. So we make the existence check when the writability check
fails and give it precedence.
This change saves ~76,000 RPCs while building //absl/... (ABSL build benchmark).
That is 10% of all RPCs made while running that workload.
PiperOrigin-RevId: 371225633
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/sentry/fsimpl/gofer/filesystem.go | 29 |
1 files changed, 24 insertions, 5 deletions
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 40c9243f0..c1c9ec008 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -364,21 +364,40 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir parent.dirMu.Lock() defer parent.dirMu.Unlock() - child, err := fs.getChildLocked(ctx, parent, name, &ds) - switch { - case err != nil && err != syserror.ENOENT: - return err - case child != nil: + if len(name) > maxFilenameLen { + return syserror.ENAMETOOLONG + } + // Check for existence only if caching information is available. Otherwise, + // don't check for existence just yet. We will check for existence if the + // checks for writability fail below. Existence check is done by the creation + // RPCs themselves. + if child, ok := parent.children[name]; ok && child != nil { return syserror.EEXIST } + checkExistence := func() error { + if child, err := fs.getChildLocked(ctx, parent, name, &ds); err != nil && err != syserror.ENOENT { + return err + } else if child != nil { + return syserror.EEXIST + } + return nil + } mnt := rp.Mount() if err := mnt.CheckBeginWrite(); err != nil { + // Existence check takes precedence. + if existenceErr := checkExistence(); existenceErr != nil { + return existenceErr + } return err } defer mnt.EndWrite() if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite); err != nil { + // Existence check takes precedence. + if existenceErr := checkExistence(); existenceErr != nil { + return existenceErr + } return err } if !dir && rp.MustBeDir() { |