go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Reduce walk and open cost in fsgofer 

Implement WalkGetAttr() to reuse the stat that is already
needed for Walk(). In addition, cache file QID, so it
doesn't need to stat the file to compute it.

open(2) time improved by 10%:

Baseline: 6780 ns
Change: 6083 ns

Also fixed file type which was not being set in all places.

PiperOrigin-RevId: 323102560
This commit is contained in:
Fabricio Voznika 2020-07-24 17:18:38 -07:00 committed by gVisor bot
parent ea0342d470
commit bd97206fa1
2 changed files with 107 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

150
runsc/fsgofer/fsgofer.go
View File

@ -48,36 +48,6 @@ const (
openFlags = syscall.O_NOFOLLOW | syscall.O_CLOEXEC
)
type fileType int
const (
regular fileType = iota
directory
symlink
socket
unknown
)
// String implements fmt.Stringer.
func (f fileType) String() string {
switch f {
case regular:
return "regular"
case directory:
return "directory"
case symlink:
return "symlink"
case socket:
return "socket"
}
return "unknown"
}
// ControlSocketAddr generates an abstract unix socket name for the given id.
func ControlSocketAddr(id string) string {
return fmt.Sprintf("\x00runsc-gofer.%s", id)
}
// Config sets configuration options for each attach point.
type Config struct {
// ROMount is set to true if this is a readonly mount.
@ -199,8 +169,6 @@ func (a *attachPoint) makeQID(stat syscall.Stat_t) p9.QID {
// entire file up when it's opened in write mode, and would perform badly when
// multiple files are only being opened for read (esp. startup).
type localFile struct {
p9.DefaultWalkGetAttr
// attachPoint is the attachPoint that serves this localFile.
attachPoint *attachPoint
@ -220,8 +188,11 @@ type localFile struct {
// if localFile isn't opened.
mode p9.OpenFlags
// ft is the fileType for this file.
ft fileType
// fileType for this file. It is equivalent to:
// syscall.Stat_t.Mode & syscall.S_IFMT
fileType uint32
qid p9.QID
// readDirMu protects against concurrent Readdir calls.
readDirMu sync.Mutex
@ -308,29 +279,24 @@ func openAnyFile(path string, fn func(mode int) (*fd.FD, error)) (*fd.FD, bool,
return nil, false, extractErrno(err)
}
func getSupportedFileType(stat syscall.Stat_t, permitSocket bool) (fileType, error) {
var ft fileType
func checkSupportedFileType(stat syscall.Stat_t, permitSocket bool) error {
switch stat.Mode & syscall.S_IFMT {
case syscall.S_IFREG:
ft = regular
case syscall.S_IFDIR:
ft = directory
case syscall.S_IFLNK:
ft = symlink
case syscall.S_IFREG, syscall.S_IFDIR, syscall.S_IFLNK:
return nil
case syscall.S_IFSOCK:
if !permitSocket {
return unknown, syscall.EPERM
return syscall.EPERM
}
ft = socket
return nil
default:
return unknown, syscall.EPERM
return syscall.EPERM
}
return ft, nil
}
func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat syscall.Stat_t) (*localFile, error) {
ft, err := getSupportedFileType(stat, a.conf.HostUDS)
if err != nil {
if err := checkSupportedFileType(stat, a.conf.HostUDS); err != nil {
return nil, err
}
@ -339,7 +305,8 @@ func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat
hostPath: path,
file: file,
mode: invalidMode,
ft: ft,
fileType: stat.Mode & syscall.S_IFMT,
qid: a.makeQID(stat),
controlReadable: readable,
}, nil
}
@ -359,7 +326,7 @@ func newFDMaybe(file *fd.FD) *fd.FD {
// fd is blocking; non-blocking is required.
if err := syscall.SetNonblock(dup.FD(), true); err != nil {
dup.Close()
_ = dup.Close()
return nil
}
return dup
@ -409,16 +376,8 @@ func (l *localFile) Open(flags p9.OpenFlags) (*fd.FD, p9.QID, uint32, error) {
}
}
stat, err := fstat(newFile.FD())
if err != nil {
if newFile != l.file {
newFile.Close()
}
return nil, p9.QID{}, 0, extractErrno(err)
}
var fd *fd.FD
if stat.Mode&syscall.S_IFMT == syscall.S_IFREG {
if l.fileType == syscall.S_IFREG {
// Donate FD for regular files only.
fd = newFDMaybe(newFile)
}
@ -431,7 +390,7 @@ func (l *localFile) Open(flags p9.OpenFlags) (*fd.FD, p9.QID, uint32, error) {
l.file = newFile
}
l.mode = flags & p9.OpenFlagsModeMask
return fd, l.attachPoint.makeQID(stat), 0, nil
return fd, l.qid, 0, nil
}
// Create implements p9.File.
@ -459,7 +418,7 @@ func (l *localFile) Create(name string, mode p9.OpenFlags, perm p9.FileMode, uid
return nil, nil, p9.QID{}, 0, extractErrno(err)
}
cu := cleanup.Make(func() {
child.Close()
_ = child.Close()
// Best effort attempt to remove the file in case of failure.
if err := syscall.Unlinkat(l.file.FD(), name); err != nil {
log.Warningf("error unlinking file %q after failure: %v", path.Join(l.hostPath, name), err)
@ -480,10 +439,12 @@ func (l *localFile) Create(name string, mode p9.OpenFlags, perm p9.FileMode, uid
hostPath: path.Join(l.hostPath, name),
file: child,
mode: mode,
fileType: syscall.S_IFREG,
qid: l.attachPoint.makeQID(stat),
}
cu.Release()
return newFDMaybe(c.file), c, l.attachPoint.makeQID(stat), 0, nil
return newFDMaybe(c.file), c, c.qid, 0, nil
}
// Mkdir implements p9.File.
@ -529,19 +490,34 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID)
// Walk implements p9.File.
func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
qids, file, _, err := l.walk(names)
return qids, file, err
}
// WalkGetAttr implements p9.File.
func (l *localFile) WalkGetAttr(names []string) ([]p9.QID, p9.File, p9.AttrMask, p9.Attr, error) {
qids, file, stat, err := l.walk(names)
if err != nil {
return nil, nil, p9.AttrMask{}, p9.Attr{}, err
}
mask, attr := l.fillAttr(stat)
return qids, file, mask, attr, nil
}
func (l *localFile) walk(names []string) ([]p9.QID, p9.File, syscall.Stat_t, error) {
// Duplicate current file if 'names' is empty.
if len(names) == 0 {
newFile, readable, err := openAnyFile(l.hostPath, func(mode int) (*fd.FD, error) {
return reopenProcFd(l.file, openFlags|mode)
})
if err != nil {
return nil, nil, extractErrno(err)
return nil, nil, syscall.Stat_t{}, extractErrno(err)
}
stat, err := fstat(newFile.FD())
if err != nil {
newFile.Close()
return nil, nil, extractErrno(err)
_ = newFile.Close()
return nil, nil, syscall.Stat_t{}, extractErrno(err)
}
c := &localFile{
@ -549,36 +525,39 @@ func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
hostPath: l.hostPath,
file: newFile,
mode: invalidMode,
fileType: l.fileType,
qid: l.attachPoint.makeQID(stat),
controlReadable: readable,
}
return []p9.QID{l.attachPoint.makeQID(stat)}, c, nil
return []p9.QID{c.qid}, c, stat, nil
}
var qids []p9.QID
var lastStat syscall.Stat_t
last := l
for _, name := range names {
f, path, readable, err := openAnyFileFromParent(last, name)
if last != l {
last.Close()
_ = last.Close()
}
if err != nil {
return nil, nil, extractErrno(err)
return nil, nil, syscall.Stat_t{}, extractErrno(err)
}
stat, err := fstat(f.FD())
lastStat, err = fstat(f.FD())
if err != nil {
f.Close()
return nil, nil, extractErrno(err)
_ = f.Close()
return nil, nil, syscall.Stat_t{}, extractErrno(err)
}
c, err := newLocalFile(last.attachPoint, f, path, readable, stat)
c, err := newLocalFile(last.attachPoint, f, path, readable, lastStat)
if err != nil {
f.Close()
return nil, nil, extractErrno(err)
_ = f.Close()
return nil, nil, syscall.Stat_t{}, extractErrno(err)
}
qids = append(qids, l.attachPoint.makeQID(stat))
qids = append(qids, c.qid)
last = c
}
return qids, last, nil
return qids, last, lastStat, nil
}
// StatFS implements p9.File.
@ -618,12 +597,16 @@ 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)
return l.qid, mask, attr, nil
}
func (l *localFile) fillAttr(stat syscall.Stat_t) (p9.AttrMask, p9.Attr) {
attr := p9.Attr{
Mode: p9.FileMode(stat.Mode),
UID: p9.UID(stat.Uid),
GID: p9.GID(stat.Gid),
NLink: uint64(stat.Nlink),
NLink: stat.Nlink,
RDev: stat.Rdev,
Size: uint64(stat.Size),
BlockSize: uint64(stat.Blksize),
@ -647,8 +630,7 @@ func (l *localFile) GetAttr(_ p9.AttrMask) (p9.QID, p9.AttrMask, p9.Attr, error)
MTime: true,
CTime: true,
}
return l.attachPoint.makeQID(stat), valid, attr, nil
return valid, attr
}
// SetAttr implements p9.File. Due to mismatch in file API, options
@ -689,7 +671,7 @@ func (l *localFile) SetAttr(valid p9.SetAttrMask, attr p9.SetAttr) error {
// Check if it's possible to use cached file, or if another one needs to be
// opened for write.
f := l.file
if l.ft == regular && l.mode != p9.WriteOnly && l.mode != p9.ReadWrite {
if l.fileType == syscall.S_IFREG && l.mode != p9.WriteOnly && l.mode != p9.ReadWrite {
var err error
f, err = reopenProcFd(l.file, openFlags|os.O_WRONLY)
if err != nil {
@ -745,7 +727,7 @@ func (l *localFile) SetAttr(valid p9.SetAttrMask, attr p9.SetAttr) error {
}
}
if l.ft == symlink {
if l.fileType == syscall.S_IFLNK {
// utimensat operates different that other syscalls. To operate on a
// symlink it *requires* AT_SYMLINK_NOFOLLOW with dirFD and a non-empty
// name.
@ -929,7 +911,7 @@ func (l *localFile) Link(target p9.File, newName string) error {
}
// Mknod implements p9.File.
func (l *localFile) Mknod(name string, mode p9.FileMode, _ uint32, _ uint32, uid p9.UID, gid p9.GID) (p9.QID, error) {
func (l *localFile) Mknod(name string, mode p9.FileMode, _ uint32, _ uint32, _ p9.UID, _ p9.GID) (p9.QID, error) {
conf := l.attachPoint.conf
if conf.ROMount {
if conf.PanicOnWrite {
@ -1127,13 +1109,13 @@ func (l *localFile) Connect(flags p9.ConnectFlags) (*fd.FD, error) {
}
if err := syscall.SetNonblock(f, true); err != nil {
syscall.Close(f)
_ = syscall.Close(f)
return nil, err
}
sa := syscall.SockaddrUnix{Name: l.hostPath}
if err := syscall.Connect(f, &sa); err != nil {
syscall.Close(f)
_ = syscall.Close(f)
return nil, err
}

69
runsc/fsgofer/fsgofer_test.go
View File

@ -32,7 +32,7 @@ import (
var allOpenFlags = []p9.OpenFlags{p9.ReadOnly, p9.WriteOnly, p9.ReadWrite}
var (
allTypes = []fileType{regular, directory, symlink}
allTypes = []uint32{syscall.S_IFREG, syscall.S_IFDIR, syscall.S_IFLNK}
// allConfs is set in init().
allConfs []Config
@ -109,24 +109,37 @@ func testReadWrite(f p9.File, flags p9.OpenFlags, content []byte) error {
}
type state struct {
root *localFile
file *localFile
conf Config
ft fileType
root *localFile
file *localFile
conf Config
fileType uint32
}
func (s state) String() string {
return fmt.Sprintf("type(%v)", s.ft)
return fmt.Sprintf("type(%v)", s.fileType)
}
func typeName(fileType uint32) string {
switch fileType {
case syscall.S_IFREG:
return "file"
case syscall.S_IFDIR:
return "directory"
case syscall.S_IFLNK:
return "symlink"
default:
panic(fmt.Sprintf("invalid file type for test: %d", fileType))
}
}
func runAll(t *testing.T, test func(*testing.T, state)) {
runCustom(t, allTypes, allConfs, test)
}
func runCustom(t *testing.T, types []fileType, confs []Config, test func(*testing.T, state)) {
func runCustom(t *testing.T, types []uint32, confs []Config, test func(*testing.T, state)) {
for _, c := range confs {
for _, ft := range types {
name := fmt.Sprintf("%s/%v", configTestName(&c), ft)
name := fmt.Sprintf("%s/%s", configTestName(&c), typeName(ft))
t.Run(name, func(t *testing.T) {
path, name, err := setup(ft)
if err != nil {
@ -150,10 +163,10 @@ func runCustom(t *testing.T, types []fileType, confs []Config, test func(*testin
}
st := state{
root: root.(*localFile),
file: file.(*localFile),
conf: c,
ft: ft,
root: root.(*localFile),
file: file.(*localFile),
conf: c,
fileType: ft,
}
test(t, st)
file.Close()
@ -163,7 +176,7 @@ func runCustom(t *testing.T, types []fileType, confs []Config, test func(*testin
}
}
func setup(ft fileType) (string, string, error) {
func setup(fileType uint32) (string, string, error) {
path, err := ioutil.TempDir(testutil.TmpDir(), "root-")
if err != nil {
return "", "", fmt.Errorf("ioutil.TempDir() failed, err: %v", err)
@ -181,26 +194,26 @@ func setup(ft fileType) (string, string, error) {
defer root.Close()
var name string
switch ft {
case regular:
switch fileType {
case syscall.S_IFREG:
name = "file"
_, 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)
}
defer f.Close()
case directory:
case syscall.S_IFDIR:
name = "dir"
if _, err := root.Mkdir(name, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil {
return "", "", fmt.Errorf("root.MkDir(%q) failed, err: %v", name, err)
}
case symlink:
case syscall.S_IFLNK:
name = "symlink"
if _, err := root.Symlink("/some/target", name, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil {
return "", "", fmt.Errorf("root.Symlink(%q) failed, err: %v", name, err)
}
default:
panic(fmt.Sprintf("unknown file type %v", ft))
panic(fmt.Sprintf("unknown file type %v", fileType))
}
return path, name, nil
}
@ -214,7 +227,7 @@ func createFile(dir *localFile, name string) (*localFile, error) {
}
func TestReadWrite(t *testing.T) {
runCustom(t, []fileType{directory}, rwConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
child, err := createFile(s.file, "test")
if err != nil {
t.Fatalf("%v: createFile() failed, err: %v", s, err)
@ -244,7 +257,7 @@ func TestReadWrite(t *testing.T) {
}
func TestCreate(t *testing.T) {
runCustom(t, []fileType{directory}, rwConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
for i, flags := range allOpenFlags {
_, l, _, _, err := s.file.Create(fmt.Sprintf("test-%d", i), flags, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid()))
if err != nil {
@ -261,7 +274,7 @@ func TestCreate(t *testing.T) {
// TestReadWriteDup tests that a file opened in any mode can be dup'ed and
// reopened in any other mode.
func TestReadWriteDup(t *testing.T) {
runCustom(t, []fileType{directory}, rwConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
child, err := createFile(s.file, "test")
if err != nil {
t.Fatalf("%v: createFile() failed, err: %v", s, err)
@ -303,7 +316,7 @@ func TestReadWriteDup(t *testing.T) {
}
func TestUnopened(t *testing.T) {
runCustom(t, []fileType{regular}, allConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFREG}, allConfs, func(t *testing.T, s state) {
b := []byte("foobar")
if _, err := s.file.WriteAt(b, 0); err != syscall.EBADF {
t.Errorf("%v: WriteAt() should have failed, got: %v, expected: syscall.EBADF", s, err)
@ -325,7 +338,7 @@ func TestUnopened(t *testing.T) {
// was open with O_PATH, but Open() was not checking for it and allowing the
// control file to be reused.
func TestOpenOPath(t *testing.T) {
runCustom(t, []fileType{regular}, rwConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFREG}, rwConfs, func(t *testing.T, s state) {
// Fist remove all permissions on the file.
if err := s.file.SetAttr(p9.SetAttrMask{Permissions: true}, p9.SetAttr{Permissions: p9.FileMode(0)}); err != nil {
t.Fatalf("SetAttr(): %v", err)
@ -362,7 +375,7 @@ func TestSetAttrPerm(t *testing.T) {
valid := p9.SetAttrMask{Permissions: true}
attr := p9.SetAttr{Permissions: 0777}
got, err := SetGetAttr(s.file, valid, attr)
if s.ft == symlink {
if s.fileType == syscall.S_IFLNK {
if err == nil {
t.Fatalf("%v: SetGetAttr(valid, %v) should have failed", s, attr.Permissions)
}
@ -383,7 +396,7 @@ func TestSetAttrSize(t *testing.T) {
valid := p9.SetAttrMask{Size: true}
attr := p9.SetAttr{Size: size}
got, err := SetGetAttr(s.file, valid, attr)
if s.ft == symlink || s.ft == directory {
if s.fileType == syscall.S_IFLNK || s.fileType == syscall.S_IFDIR {
if err == nil {
t.Fatalf("%v: SetGetAttr(valid, %v) should have failed", s, attr.Permissions)
}
@ -465,7 +478,7 @@ func TestLink(t *testing.T) {
}
err = dir.Link(s.file, linkFile)
if s.ft == directory {
if s.fileType == syscall.S_IFDIR {
if err != syscall.EPERM {
t.Errorf("%v: Link(target, %s) should have failed, got: %v, expected: syscall.EPERM", s, linkFile, err)
}
@ -523,7 +536,7 @@ func TestROMountPanics(t *testing.T) {
}
func TestWalkNotFound(t *testing.T) {
runCustom(t, []fileType{directory}, allConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFDIR}, allConfs, func(t *testing.T, s state) {
if _, _, err := s.file.Walk([]string{"nobody-here"}); err != syscall.ENOENT {
t.Errorf("%v: Walk(%q) should have failed, got: %v, expected: syscall.ENOENT", s, "nobody-here", err)
}
@ -544,7 +557,7 @@ func TestWalkDup(t *testing.T) {
}
func TestReaddir(t *testing.T) {
runCustom(t, []fileType{directory}, rwConfs, func(t *testing.T, s state) {
runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) {
name := "dir"
if _, err := s.file.Mkdir(name, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil {
t.Fatalf("%v: MkDir(%s) failed, err: %v", s, name, err)