netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

fsgofer optimizations

Browse Source 
- 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
This commit is contained in:
Fabricio Voznika
2020-12-15 12:23:55 -08:00
committed by gVisor bot
parent 5843a5007c
commit 12ac31ed04
3 changed files with 269 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runsc/fsgofer/BUILD
+3
View File
@@ -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",
],
)
runsc/fsgofer/fsgofer.go
+51 -28
View File
@@ -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.
runsc/fsgofer/fsgofer_test.go
+215 -2
View File
@@ -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()
}
}