fsgofer optimizations

- Skip chown call in case owner change is not needed
- Skip filepath.Clean() calls when joining paths
- Pass unix.Stat_t by value to reduce runtime.duffcopy calls.
  This change allows for better inlining in localFile.walk().

                                Change            Baseline    Improvement
BenchmarkWalkOne-6           	 2912 ns/op       3082 ns/op     5.5%
BenchmarkCreate-6            	15915 ns/op      19126 ns/op    16.8%
BenchmarkCreateDiffOwner-6	18795 ns/op      19741 ns/op     4.8%

PiperOrigin-RevId: 347667833

3 files changed, 269 insertions, 30 deletions

diff --git a/runsc/fsgofer/BUILD b/runsc/fsgofer/BUILD
index 96c57a426..c56e1d4d0 100644
--- a/runsc/fsgofer/BUILD
+++ b/runsc/fsgofer/BUILD
@@ -29,9 +29,12 @@ go_test(
     srcs = ["fsgofer_test.go"],
     library = ":fsgofer",
     deps = [
+        "//pkg/fd",
         "//pkg/log",
         "//pkg/p9",
         "//pkg/test/testutil",
+        "//runsc/specutils",
+        "@com_github_syndtr_gocapability//capability:go_default_library",
         "@org_golang_x_sys//unix:go_default_library",
     ],
 )
diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index 0b628c8ce..3d94ffeb4 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -49,6 +49,21 @@ const (
 	allowedOpenFlags = unix.O_TRUNC
 )
 
+var (
+	// Remember the process uid/gid to skip chown calls when file owner/group
+	// doesn't need to be changed.
+	processUID = p9.UID(os.Getuid())
+	processGID = p9.GID(os.Getgid())
+)
+
+// join is equivalent to path.Join() but skips path.Clean() which is expensive.
+func join(parent, child string) string {
+	if child == "." || child == ".." {
+		panic(fmt.Sprintf("invalid child path %q", child))
+	}
+	return parent + "/" + child
+}
+
 // Config sets configuration options for each attach point.
 type Config struct {
 	// ROMount is set to true if this is a readonly mount.
@@ -115,7 +130,7 @@ func (a *attachPoint) Attach() (p9.File, error) {
 		return nil, fmt.Errorf("unable to stat %q: %v", a.prefix, err)
 	}
 
-	lf, err := newLocalFile(a, f, a.prefix, readable, stat)
+	lf, err := newLocalFile(a, f, a.prefix, readable, &stat)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create localFile %q: %v", a.prefix, err)
 	}
@@ -124,7 +139,7 @@ func (a *attachPoint) Attach() (p9.File, error) {
 }
 
 // makeQID returns a unique QID for the given stat buffer.
-func (a *attachPoint) makeQID(stat unix.Stat_t) p9.QID {
+func (a *attachPoint) makeQID(stat *unix.Stat_t) p9.QID {
 	a.deviceMu.Lock()
 	defer a.deviceMu.Unlock()
 
@@ -245,7 +260,7 @@ func reopenProcFd(f *fd.FD, mode int) (*fd.FD, error) {
 }
 
 func openAnyFileFromParent(parent *localFile, name string) (*fd.FD, string, bool, error) {
-	pathDebug := path.Join(parent.hostPath, name)
+	pathDebug := join(parent.hostPath, name)
 	f, readable, err := openAnyFile(pathDebug, func(mode int) (*fd.FD, error) {
 		return fd.OpenAt(parent.file, name, openFlags|mode, 0)
 	})
@@ -297,8 +312,8 @@ func openAnyFile(pathDebug string, fn func(mode int) (*fd.FD, error)) (*fd.FD, b
 	return nil, false, extractErrno(err)
 }
 
-func checkSupportedFileType(stat unix.Stat_t, permitSocket bool) error {
-	switch stat.Mode & unix.S_IFMT {
+func checkSupportedFileType(mode uint32, permitSocket bool) error {
+	switch mode & unix.S_IFMT {
 	case unix.S_IFREG, unix.S_IFDIR, unix.S_IFLNK:
 		return nil
 
@@ -313,8 +328,8 @@ func checkSupportedFileType(stat unix.Stat_t, permitSocket bool) error {
 	}
 }
 
-func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat unix.Stat_t) (*localFile, error) {
-	if err := checkSupportedFileType(stat, a.conf.HostUDS); err != nil {
+func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat *unix.Stat_t) (*localFile, error) {
+	if err := checkSupportedFileType(stat.Mode, a.conf.HostUDS); err != nil {
 		return nil, err
 	}
 
@@ -442,8 +457,10 @@ func (l *localFile) Create(name string, p9Flags p9.OpenFlags, perm p9.FileMode,
 	})
 	defer cu.Clean()
 
-	if err := fchown(child.FD(), uid, gid); err != nil {
-		return nil, nil, p9.QID{}, 0, extractErrno(err)
+	if uid != processUID || gid != processGID {
+		if err := fchown(child.FD(), uid, gid); err != nil {
+			return nil, nil, p9.QID{}, 0, extractErrno(err)
+		}
 	}
 	stat, err := fstat(child.FD())
 	if err != nil {
@@ -452,11 +469,11 @@ func (l *localFile) Create(name string, p9Flags p9.OpenFlags, perm p9.FileMode,
 
 	c := &localFile{
 		attachPoint: l.attachPoint,
-		hostPath:    path.Join(l.hostPath, name),
+		hostPath:    join(l.hostPath, name),
 		file:        child,
 		mode:        mode,
 		fileType:    unix.S_IFREG,
-		qid:         l.attachPoint.makeQID(stat),
+		qid:         l.attachPoint.makeQID(&stat),
 	}
 
 	cu.Release()
@@ -488,8 +505,10 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID)
 	}
 	defer f.Close()
 
-	if err := fchown(f.FD(), uid, gid); err != nil {
-		return p9.QID{}, extractErrno(err)
+	if uid != processUID || gid != processGID {
+		if err := fchown(f.FD(), uid, gid); err != nil {
+			return p9.QID{}, extractErrno(err)
+		}
 	}
 	stat, err := fstat(f.FD())
 	if err != nil {
@@ -497,7 +516,7 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID)
 	}
 
 	cu.Release()
-	return l.attachPoint.makeQID(stat), nil
+	return l.attachPoint.makeQID(&stat), nil
 }
 
 // Walk implements p9.File.
@@ -512,7 +531,7 @@ func (l *localFile) WalkGetAttr(names []string) ([]p9.QID, p9.File, p9.AttrMask,
 	if err != nil {
 		return nil, nil, p9.AttrMask{}, p9.Attr{}, err
 	}
-	mask, attr := l.fillAttr(stat)
+	mask, attr := l.fillAttr(&stat)
 	return qids, file, mask, attr, nil
 }
 
@@ -538,13 +557,13 @@ func (l *localFile) walk(names []string) ([]p9.QID, p9.File, unix.Stat_t, error)
 			file:            newFile,
 			mode:            invalidMode,
 			fileType:        l.fileType,
-			qid:             l.attachPoint.makeQID(stat),
+			qid:             l.attachPoint.makeQID(&stat),
 			controlReadable: readable,
 		}
 		return []p9.QID{c.qid}, c, stat, nil
 	}
 
-	var qids []p9.QID
+	qids := make([]p9.QID, 0, len(names))
 	var lastStat unix.Stat_t
 	last := l
 	for _, name := range names {
@@ -560,7 +579,7 @@ func (l *localFile) walk(names []string) ([]p9.QID, p9.File, unix.Stat_t, error)
 			_ = f.Close()
 			return nil, nil, unix.Stat_t{}, extractErrno(err)
 		}
-		c, err := newLocalFile(last.attachPoint, f, path, readable, lastStat)
+		c, err := newLocalFile(last.attachPoint, f, path, readable, &lastStat)
 		if err != nil {
 			_ = f.Close()
 			return nil, nil, unix.Stat_t{}, extractErrno(err)
@@ -609,11 +628,11 @@ func (l *localFile) GetAttr(_ p9.AttrMask) (p9.QID, p9.AttrMask, p9.Attr, error)
 	if err != nil {
 		return p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err)
 	}
-	mask, attr := l.fillAttr(stat)
+	mask, attr := l.fillAttr(&stat)
 	return l.qid, mask, attr, nil
 }
 
-func (l *localFile) fillAttr(stat unix.Stat_t) (p9.AttrMask, p9.Attr) {
+func (l *localFile) fillAttr(stat *unix.Stat_t) (p9.AttrMask, p9.Attr) {
 	attr := p9.Attr{
 		Mode:             p9.FileMode(stat.Mode),
 		UID:              p9.UID(stat.Uid),
@@ -881,8 +900,10 @@ func (l *localFile) Symlink(target, newName string, uid p9.UID, gid p9.GID) (p9.
 	}
 	defer f.Close()
 
-	if err := fchown(f.FD(), uid, gid); err != nil {
-		return p9.QID{}, extractErrno(err)
+	if uid != processUID || gid != processGID {
+		if err := fchown(f.FD(), uid, gid); err != nil {
+			return p9.QID{}, extractErrno(err)
+		}
 	}
 	stat, err := fstat(f.FD())
 	if err != nil {
@@ -890,7 +911,7 @@ func (l *localFile) Symlink(target, newName string, uid p9.UID, gid p9.GID) (p9.
 	}
 
 	cu.Release()
-	return l.attachPoint.makeQID(stat), nil
+	return l.attachPoint.makeQID(&stat), nil
 }
 
 // Link implements p9.File.
@@ -938,8 +959,10 @@ func (l *localFile) Mknod(name string, mode p9.FileMode, _ uint32, _ uint32, uid
 	}
 	defer child.Close()
 
-	if err := fchown(child.FD(), uid, gid); err != nil {
-		return p9.QID{}, extractErrno(err)
+	if uid != processUID || gid != processGID {
+		if err := fchown(child.FD(), uid, gid); err != nil {
+			return p9.QID{}, extractErrno(err)
+		}
 	}
 	stat, err := fstat(child.FD())
 	if err != nil {
@@ -947,7 +970,7 @@ func (l *localFile) Mknod(name string, mode p9.FileMode, _ uint32, _ uint32, uid
 	}
 
 	cu.Release()
-	return l.attachPoint.makeQID(stat), nil
+	return l.attachPoint.makeQID(&stat), nil
 }
 
 // UnlinkAt implements p9.File.
@@ -1045,7 +1068,7 @@ func (l *localFile) readDirent(f int, offset uint64, count uint32, skip uint64)
 				log.Warningf("Readdir is skipping file with failed stat %q, err: %v", l.hostPath, err)
 				continue
 			}
-			qid := l.attachPoint.makeQID(stat)
+			qid := l.attachPoint.makeQID(&stat)
 			offset++
 			dirents = append(dirents, p9.Dirent{
 				QID:    qid,
@@ -1139,7 +1162,7 @@ func (l *localFile) isOpen() bool {
 
 // Renamed implements p9.Renamed.
 func (l *localFile) Renamed(newDir p9.File, newName string) {
-	l.hostPath = path.Join(newDir.(*localFile).hostPath, newName)
+	l.hostPath = join(newDir.(*localFile).hostPath, newName)
 }
 
 // extractErrno tries to determine the errno.
diff --git a/runsc/fsgofer/fsgofer_test.go b/runsc/fsgofer/fsgofer_test.go
index a84206686..c5daebe5e 100644
--- a/runsc/fsgofer/fsgofer_test.go
+++ b/runsc/fsgofer/fsgofer_test.go
@@ -23,10 +23,13 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/syndtr/gocapability/capability"
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/fd"
 	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/p9"
 	"gvisor.dev/gvisor/pkg/test/testutil"
+	"gvisor.dev/gvisor/runsc/specutils"
 )
 
 var allOpenFlags = []p9.OpenFlags{p9.ReadOnly, p9.WriteOnly, p9.ReadWrite}
@@ -197,10 +200,13 @@ func setup(fileType uint32) (string, string, error) {
 	switch fileType {
 	case unix.S_IFREG:
 		name = "file"
-		_, f, _, _, err := root.Create(name, p9.ReadWrite, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid()))
+		fd, f, _, _, err := root.Create(name, p9.ReadWrite, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid()))
 		if err != nil {
 			return "", "", fmt.Errorf("createFile(root, %q) failed, err: %v", "test", err)
 		}
+		if fd != nil {
+			fd.Close()
+		}
 		defer f.Close()
 	case unix.S_IFDIR:
 		name = "dir"
@@ -556,7 +562,28 @@ func TestROMountChecks(t *testing.T) {
 func TestWalkNotFound(t *testing.T) {
 	runCustom(t, []uint32{unix.S_IFDIR}, allConfs, func(t *testing.T, s state) {
 		if _, _, err := s.file.Walk([]string{"nobody-here"}); err != unix.ENOENT {
-			t.Errorf("%v: Walk(%q) should have failed, got: %v, expected: unix.ENOENT", s, "nobody-here", err)
+			t.Errorf("Walk(%q) should have failed, got: %v, expected: unix.ENOENT", "nobody-here", err)
+		}
+		if _, _, err := s.file.Walk([]string{"nobody", "here"}); err != unix.ENOENT {
+			t.Errorf("Walk(%q) should have failed, got: %v, expected: unix.ENOENT", "nobody/here", err)
+		}
+		if !s.conf.ROMount {
+			if _, err := s.file.Mkdir("dir", 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil {
+				t.Fatalf("MkDir(dir) failed, err: %v", err)
+			}
+			if _, _, err := s.file.Walk([]string{"dir", "nobody-here"}); err != unix.ENOENT {
+				t.Errorf("Walk(%q) should have failed, got: %v, expected: unix.ENOENT", "dir/nobody-here", err)
+			}
+		}
+	})
+}
+
+func TestWalkPanic(t *testing.T) {
+	runCustom(t, []uint32{unix.S_IFDIR}, allConfs, func(t *testing.T, s state) {
+		for _, name := range []string{".", ".."} {
+			assertPanic(t, func() {
+				s.file.Walk([]string{name})
+			})
 		}
 	})
 }
@@ -574,6 +601,27 @@ func TestWalkDup(t *testing.T) {
 	})
 }
 
+func TestWalkMultiple(t *testing.T) {
+	runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
+		var names []string
+		var parent p9.File = s.file
+		for i := 0; i < 5; i++ {
+			name := fmt.Sprintf("dir%d", i)
+			names = append(names, name)
+
+			if _, err := parent.Mkdir(name, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil {
+				t.Fatalf("MkDir(%q) failed, err: %v", name, err)
+			}
+
+			var err error
+			_, parent, err = s.file.Walk(names)
+			if err != nil {
+				t.Errorf("Walk(%q): %v", name, err)
+			}
+		}
+	})
+}
+
 func TestReaddir(t *testing.T) {
 	runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
 		name := "dir"
@@ -819,3 +867,168 @@ func TestMknod(t *testing.T) {
 		}
 	})
 }
+
+func BenchmarkWalkOne(b *testing.B) {
+	path, name, err := setup(unix.S_IFDIR)
+	if err != nil {
+		b.Fatalf("%v", err)
+	}
+	defer os.RemoveAll(path)
+
+	a, err := NewAttachPoint(path, Config{})
+	if err != nil {
+		b.Fatalf("NewAttachPoint failed: %v", err)
+	}
+	root, err := a.Attach()
+	if err != nil {
+		b.Fatalf("Attach failed, err: %v", err)
+	}
+	defer root.Close()
+
+	names := []string{name}
+	files := make([]p9.File, 0, 1000)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, file, err := root.Walk(names)
+		if err != nil {
+			b.Fatalf("Walk(%q): %v", name, err)
+		}
+		files = append(files, file)
+
+		// Avoid running out of FDs.
+		if len(files) == cap(files) {
+			b.StopTimer()
+			for _, file := range files {
+				file.Close()
+			}
+			files = files[:0]
+			b.StartTimer()
+		}
+	}
+
+	b.StopTimer()
+	for _, file := range files {
+		file.Close()
+	}
+}
+
+func BenchmarkCreate(b *testing.B) {
+	path, _, err := setup(unix.S_IFDIR)
+	if err != nil {
+		b.Fatalf("%v", err)
+	}
+	defer os.RemoveAll(path)
+
+	a, err := NewAttachPoint(path, Config{})
+	if err != nil {
+		b.Fatalf("NewAttachPoint failed: %v", err)
+	}
+	root, err := a.Attach()
+	if err != nil {
+		b.Fatalf("Attach failed, err: %v", err)
+	}
+	defer root.Close()
+
+	files := make([]p9.File, 0, 500)
+	fds := make([]*fd.FD, 0, 500)
+	uid := p9.UID(os.Getuid())
+	gid := p9.GID(os.Getgid())
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		name := fmt.Sprintf("same-%d", i)
+		fd, file, _, _, err := root.Create(name, p9.ReadOnly, 0777, uid, gid)
+		if err != nil {
+			b.Fatalf("Create(%q): %v", name, err)
+		}
+		files = append(files, file)
+		if fd != nil {
+			fds = append(fds, fd)
+		}
+
+		// Avoid running out of FDs.
+		if len(files) == cap(files) {
+			b.StopTimer()
+			for _, file := range files {
+				file.Close()
+			}
+			files = files[:0]
+			for _, fd := range fds {
+				fd.Close()
+			}
+			fds = fds[:0]
+			b.StartTimer()
+		}
+	}
+
+	b.StopTimer()
+	for _, file := range files {
+		file.Close()
+	}
+	for _, fd := range fds {
+		fd.Close()
+	}
+}
+
+func BenchmarkCreateDiffOwner(b *testing.B) {
+	if !specutils.HasCapabilities(capability.CAP_CHOWN) {
+		b.Skipf("Test requires CAP_CHOWN")
+	}
+
+	path, _, err := setup(unix.S_IFDIR)
+	if err != nil {
+		b.Fatalf("%v", err)
+	}
+	defer os.RemoveAll(path)
+
+	a, err := NewAttachPoint(path, Config{})
+	if err != nil {
+		b.Fatalf("NewAttachPoint failed: %v", err)
+	}
+	root, err := a.Attach()
+	if err != nil {
+		b.Fatalf("Attach failed, err: %v", err)
+	}
+	defer root.Close()
+
+	files := make([]p9.File, 0, 500)
+	fds := make([]*fd.FD, 0, 500)
+	gid := p9.GID(os.Getgid())
+	const nobody = 65534
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		name := fmt.Sprintf("diff-%d", i)
+		fd, file, _, _, err := root.Create(name, p9.ReadOnly, 0777, nobody, gid)
+		if err != nil {
+			b.Fatalf("Create(%q): %v", name, err)
+		}
+		files = append(files, file)
+		if fd != nil {
+			fds = append(fds, fd)
+		}
+
+		// Avoid running out of FDs.
+		if len(files) == cap(files) {
+			b.StopTimer()
+			for _, file := range files {
+				file.Close()
+			}
+			files = files[:0]
+			for _, fd := range fds {
+				fd.Close()
+			}
+			fds = fds[:0]
+			b.StartTimer()
+		}
+	}
+
+	b.StopTimer()
+	for _, file := range files {
+		file.Close()
+	}
+	for _, fd := range fds {
+		fd.Close()
+	}
+}