From 9ff0d382d69c53a8fc916cfde844f1e657759f59 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
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(-)

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