From 9ff0d382d69c53a8fc916cfde844f1e657759f59 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 29 Apr 2021 16:09:49 -0700 Subject: [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 --- pkg/sentry/fsimpl/gofer/filesystem.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) (limited to 'pkg/sentry') 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() { -- cgit v1.2.3