Get rid of unnecessary lisafs.Inode allocations.

lisafs.Inode is a heavy struct with linux.Statx in it. However the cost of
copying it on return is lower than that of an allocation.

Additionally unclutter the filesystem.doCreateAt function signature. It already
is quite complex. lisafs had added more complexity earlier. Revert that.

PiperOrigin-RevId: 424972317
This commit is contained in:
Ayush Ranjan
2022-01-28 15:38:05 -08:00
committed by gVisor bot
parent 62665f881d
commit e29fd32d0a
6 changed files with 107 additions and 92 deletions
+4 -4
View File
@@ -72,7 +72,7 @@ type Client struct {
// the server and creates channels for fast IPC. NewClient takes ownership over
// the passed socket. On success, it returns the initialized client along with
// the root Inode.
func NewClient(sock *unet.Socket) (*Client, *Inode, error) {
func NewClient(sock *unet.Socket) (*Client, Inode, error) {
maxChans := maxChannels()
c := &Client{
sockComm: newSockComm(sock),
@@ -99,7 +99,7 @@ func NewClient(sock *unet.Socket) (*Client, *Inode, error) {
c.supported[Mount] = true
var mountResp MountResp
if err := c.SndRcvMessage(Mount, 0, NoopMarshal, mountResp.CheckedUnmarshal, nil); err != nil {
return nil, nil, err
return nil, Inode{}, err
}
// Initialize client.
@@ -142,12 +142,12 @@ func NewClient(sock *unet.Socket) (*Client, *Inode, error) {
for _, channelErr := range channelErrs {
// Return the first non-nil channel creation error.
if channelErr != nil {
return nil, nil, channelErr
return nil, Inode{}, channelErr
}
}
cu.Release()
return c, &mountResp.Root, nil
return c, mountResp.Root, nil
}
func (c *Client) watchdog() {
+15 -15
View File
@@ -212,7 +212,7 @@ func (f *ClientFD) Write(ctx context.Context, src []byte, offset uint64) (uint64
}
// MkdirAt makes the MkdirAt RPC.
func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID) (*Inode, error) {
func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID) (Inode, error) {
var req MkdirAtReq
req.DirFD = f.fd
req.Name = SizedString(name)
@@ -224,11 +224,11 @@ func (f *ClientFD) MkdirAt(ctx context.Context, name string, mode linux.FileMode
ctx.UninterruptibleSleepStart(false)
err := f.client.SndRcvMessage(MkdirAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil)
ctx.UninterruptibleSleepFinish(false)
return &resp.ChildDir, err
return resp.ChildDir, err
}
// SymlinkAt makes the SymlinkAt RPC.
func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID, gid GID) (*Inode, error) {
func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID, gid GID) (Inode, error) {
req := SymlinkAtReq{
DirFD: f.fd,
Name: SizedString(name),
@@ -241,11 +241,11 @@ func (f *ClientFD) SymlinkAt(ctx context.Context, name, target string, uid UID,
ctx.UninterruptibleSleepStart(false)
err := f.client.SndRcvMessage(SymlinkAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil)
ctx.UninterruptibleSleepFinish(false)
return &resp.Symlink, err
return resp.Symlink, err
}
// LinkAt makes the LinkAt RPC.
func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (*Inode, error) {
func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (Inode, error) {
req := LinkAtReq{
DirFD: f.fd,
Target: targetFD,
@@ -256,11 +256,11 @@ func (f *ClientFD) LinkAt(ctx context.Context, targetFD FDID, name string) (*Ino
ctx.UninterruptibleSleepStart(false)
err := f.client.SndRcvMessage(LinkAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil)
ctx.UninterruptibleSleepFinish(false)
return &resp.Link, err
return resp.Link, err
}
// MknodAt makes the MknodAt RPC.
func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID, minor, major uint32) (*Inode, error) {
func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode, uid UID, gid GID, minor, major uint32) (Inode, error) {
var req MknodAtReq
req.DirFD = f.fd
req.Name = SizedString(name)
@@ -274,7 +274,7 @@ func (f *ClientFD) MknodAt(ctx context.Context, name string, mode linux.FileMode
ctx.UninterruptibleSleepStart(false)
err := f.client.SndRcvMessage(MknodAt, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil)
ctx.UninterruptibleSleepFinish(false)
return &resp.Child, err
return resp.Child, err
}
// SetStat makes the SetStat RPC.
@@ -318,7 +318,7 @@ func (f *ClientFD) WalkMultiple(ctx context.Context, names []string) (WalkStatus
}
// Walk makes the Walk RPC with just one path component to walk.
func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) {
func (f *ClientFD) Walk(ctx context.Context, name string) (Inode, error) {
req := WalkReq{
DirFD: f.fd,
Path: []string{name},
@@ -330,15 +330,15 @@ func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) {
err := f.client.SndRcvMessage(Walk, uint32(req.SizeBytes()), req.MarshalBytes, resp.CheckedUnmarshal, nil)
ctx.UninterruptibleSleepFinish(false)
if err != nil {
return nil, err
return Inode{}, err
}
switch resp.Status {
case WalkComponentDoesNotExist:
return nil, unix.ENOENT
return Inode{}, unix.ENOENT
case WalkComponentSymlink:
// f is not a directory which can be walked on.
return nil, unix.ENOTDIR
return Inode{}, unix.ENOTDIR
}
if n := len(resp.Inodes); n > 1 {
@@ -346,12 +346,12 @@ func (f *ClientFD) Walk(ctx context.Context, name string) (*Inode, error) {
f.client.CloseFDBatched(ctx, resp.Inodes[i].ControlFD)
}
log.Warningf("requested to walk one component, but got %d results", n)
return nil, unix.EIO
return Inode{}, unix.EIO
} else if n == 0 {
log.Warningf("walk has success status but no results returned")
return nil, unix.ENOENT
return Inode{}, unix.ENOENT
}
return &inode[0], err
return inode[0], err
}
// WalkStat makes the WalkStat RPC with multiple path components to walk.
+20
View File
@@ -22,6 +22,7 @@ import (
"gvisor.dev/gvisor/pkg/context"
"gvisor.dev/gvisor/pkg/errors/linuxerr"
"gvisor.dev/gvisor/pkg/hostarch"
"gvisor.dev/gvisor/pkg/lisafs"
"gvisor.dev/gvisor/pkg/p9"
"gvisor.dev/gvisor/pkg/refsvfs2"
"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
@@ -35,6 +36,25 @@ func (d *dentry) isDir() bool {
return d.fileType() == linux.S_IFDIR
}
// Preconditions:
// - filesystem.renameMu must be locked.
// - d.dirMu must be locked.
// - d.isDir().
// - child must be a newly-created dentry that has never had a parent.
func (d *dentry) insertCreatedChildLocked(ctx context.Context, childIno *lisafs.Inode, childName string, updateChild func(child *dentry), ds **[]*dentry) error {
child, err := d.fs.newDentryLisa(ctx, childIno)
if err != nil {
d.fs.clientLisa.CloseFDBatched(ctx, childIno.ControlFD)
return err
}
d.cacheNewChildLocked(child, childName)
appendNewChildDentry(ds, d, child)
if updateChild != nil {
updateChild(child)
}
return nil
}
// Preconditions:
// * filesystem.renameMu must be locked.
// * d.dirMu must be locked.
+57 -62
View File
@@ -388,7 +388,7 @@ func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name s
return nil, err
}
// Create a new dentry representing the file.
child, err = fs.newDentryLisa(ctx, childInode)
child, err = fs.newDentryLisa(ctx, &childInode)
if err != nil {
fs.clientLisa.CloseFDBatched(ctx, childInode.ControlFD)
return nil, err
@@ -482,7 +482,7 @@ func (fs *filesystem) resolveLocked(ctx context.Context, rp *vfs.ResolvingPath,
// Preconditions:
// * !rp.Done().
// * For the final path component in rp, !rp.ShouldFollowSymlink().
func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error), createInSyntheticDir func(parent *dentry, name string) error, updateChild func(child *dentry)) error {
func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) error, createInSyntheticDir func(parent *dentry, name string) error) error {
var ds *[]*dentry
fs.renameMu.RLock()
defer fs.renameMuRUnlockAndCheckCaching(ctx, &ds)
@@ -569,26 +569,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
// No cached dentry exists; however, in InteropModeShared there might still be
// an existing file at name. Just attempt the file creation RPC anyways. If a
// file does exist, the RPC will fail with EEXIST like we would have.
lisaInode, err := createInRemoteDir(parent, name, &ds)
if err != nil {
if err := createInRemoteDir(parent, name, &ds); err != nil {
return err
}
// lisafs may aggresively cache newly created inodes. This has helped reduce
// Walk RPCs in practice.
if lisaInode != nil {
child, err := fs.newDentryLisa(ctx, lisaInode)
if err != nil {
fs.clientLisa.CloseFDBatched(ctx, lisaInode.ControlFD)
return err
}
parent.cacheNewChildLocked(child, name)
appendNewChildDentry(&ds, parent, child)
// lisafs may update dentry properties upon successful creation.
if updateChild != nil {
updateChild(child)
}
}
if fs.opts.interop != InteropModeShared {
if child, ok := parent.children[name]; ok && child == nil {
// Delete the now-stale negative dentry.
@@ -833,31 +816,35 @@ func (fs *filesystem) GetParentDentryAt(ctx context.Context, rp *vfs.ResolvingPa
// LinkAt implements vfs.FilesystemImpl.LinkAt.
func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs.VirtualDentry) error {
err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, childName string, ds **[]*dentry) (*lisafs.Inode, error) {
err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, childName string, ds **[]*dentry) error {
if rp.Mount() != vd.Mount() {
return nil, linuxerr.EXDEV
return linuxerr.EXDEV
}
d := vd.Dentry().Impl().(*dentry)
if d.isDir() {
return nil, linuxerr.EPERM
return linuxerr.EPERM
}
gid := auth.KGID(atomic.LoadUint32(&d.gid))
uid := auth.KUID(atomic.LoadUint32(&d.uid))
mode := linux.FileMode(atomic.LoadUint32(&d.mode))
if err := vfs.MayLink(rp.Credentials(), mode, uid, gid); err != nil {
return nil, err
return err
}
if d.nlink == 0 {
return nil, linuxerr.ENOENT
return linuxerr.ENOENT
}
if d.nlink == math.MaxUint32 {
return nil, linuxerr.EMLINK
return linuxerr.EMLINK
}
if fs.opts.lisaEnabled {
return parent.controlFDLisa.LinkAt(ctx, d.controlFDLisa.ID(), childName)
linkInode, err := parent.controlFDLisa.LinkAt(ctx, d.controlFDLisa.ID(), childName)
if err != nil {
return err
}
return parent.insertCreatedChildLocked(ctx, &linkInode, childName, nil, ds)
}
return nil, parent.file.link(ctx, d.file, childName)
}, nil, nil)
return parent.file.link(ctx, d.file, childName)
}, nil)
if err == nil {
// Success!
@@ -869,7 +856,7 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs.
// MkdirAt implements vfs.FilesystemImpl.MkdirAt.
func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MkdirOptions) error {
creds := rp.Credentials()
return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) {
return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) error {
// If the parent is a setgid directory, use the parent's GID
// rather than the caller's and enable setgid.
kgid := creds.EffectiveKGID
@@ -878,12 +865,15 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
kgid = auth.KGID(atomic.LoadUint32(&parent.gid))
mode |= linux.S_ISGID
}
var (
childDirInode *lisafs.Inode
err error
)
var err error
if fs.opts.lisaEnabled {
var childDirInode lisafs.Inode
childDirInode, err = parent.controlFDLisa.MkdirAt(ctx, name, mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(kgid))
if err == nil {
if err = parent.insertCreatedChildLocked(ctx, &childDirInode, name, nil, ds); err != nil {
return err
}
}
} else {
_, err = parent.file.mkdir(ctx, name, p9.FileMode(mode), (p9.UID)(creds.EffectiveKUID), p9.GID(kgid))
}
@@ -891,11 +881,11 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
if fs.opts.interop != InteropModeShared {
parent.incLinks()
}
return childDirInode, nil
return nil
}
if !opts.ForSyntheticMountpoint || linuxerr.Equals(linuxerr.EEXIST, err) {
return nil, err
return err
}
ctx.Infof("Failed to create remote directory %q: %v; falling back to synthetic directory", name, err)
parent.createSyntheticChildLocked(&createSyntheticOpts{
@@ -908,7 +898,7 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
if fs.opts.interop != InteropModeShared {
parent.incLinks()
}
return nil, nil
return nil
}, func(parent *dentry, name string) error {
if !opts.ForSyntheticMountpoint {
// Can't create non-synthetic files in synthetic directories.
@@ -922,26 +912,27 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
})
parent.incLinks()
return nil
}, nil)
})
}
// MknodAt implements vfs.FilesystemImpl.MknodAt.
func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MknodOptions) error {
return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) {
return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error {
creds := rp.Credentials()
var (
childInode *lisafs.Inode
err error
)
var err error
if fs.opts.lisaEnabled {
var childInode lisafs.Inode
childInode, err = parent.controlFDLisa.MknodAt(ctx, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID), opts.DevMinor, opts.DevMajor)
if err == nil {
return parent.insertCreatedChildLocked(ctx, &childInode, name, nil, ds)
}
} else {
_, err = parent.file.mknod(ctx, name, (p9.FileMode)(opts.Mode), opts.DevMajor, opts.DevMinor, (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID))
}
if err == nil {
return childInode, nil
return nil
} else if !linuxerr.Equals(linuxerr.EPERM, err) {
return nil, err
return err
}
// EPERM means that gofer does not allow creating a socket or pipe. Fallback
@@ -952,10 +943,10 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
switch {
case err == nil:
// Step succeeded, another file exists.
return nil, linuxerr.EEXIST
return linuxerr.EEXIST
case !linuxerr.Equals(linuxerr.ENOENT, err):
// Unexpected error.
return nil, err
return err
}
switch opts.Mode.FileType() {
@@ -968,7 +959,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
endpoint: opts.Endpoint,
})
*ds = appendDentry(*ds, parent)
return nil, nil
return nil
case linux.S_IFIFO:
parent.createSyntheticChildLocked(&createSyntheticOpts{
name: name,
@@ -978,11 +969,11 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize),
})
*ds = appendDentry(*ds, parent)
return nil, nil
return nil
}
// Retain error from gofer if synthetic file cannot be created internally.
return nil, linuxerr.EPERM
}, nil, nil)
return linuxerr.EPERM
}, nil)
}
// OpenAt implements vfs.FilesystemImpl.OpenAt.
@@ -1740,21 +1731,25 @@ func (fs *filesystem) StatFSAt(ctx context.Context, rp *vfs.ResolvingPath) (linu
// SymlinkAt implements vfs.FilesystemImpl.SymlinkAt.
func (fs *filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, target string) error {
return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*lisafs.Inode, error) {
return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error {
creds := rp.Credentials()
if fs.opts.lisaEnabled {
return parent.controlFDLisa.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID))
symlinkInode, err := parent.controlFDLisa.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID))
if err != nil {
return err
}
return parent.insertCreatedChildLocked(ctx, &symlinkInode, name, func(child *dentry) {
if fs.opts.interop != InteropModeShared {
// lisafs caches the symlink target on creation. In practice, this
// helps avoid a lot of ReadLink RPCs.
child.haveTarget = true
child.target = target
}
}, ds)
}
_, err := parent.file.symlink(ctx, target, name, (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID))
return nil, err
}, nil, func(child *dentry) {
if fs.opts.interop != InteropModeShared {
// lisafs caches the symlink target on creation. In practice, this
// helps avoid a lot of ReadLink RPCs.
child.haveTarget = true
child.target = target
}
})
return err
}, nil)
}
// UnlinkAt implements vfs.FilesystemImpl.UnlinkAt.
+9 -9
View File
@@ -502,12 +502,12 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt
func (fs *filesystem) initClientAndRoot(ctx context.Context) error {
var err error
if fs.opts.lisaEnabled {
var rootInode *lisafs.Inode
var rootInode lisafs.Inode
rootInode, err = fs.initClientLisa(ctx)
if err != nil {
return err
}
fs.root, err = fs.newDentryLisa(ctx, rootInode)
fs.root, err = fs.newDentryLisa(ctx, &rootInode)
if err != nil {
fs.clientLisa.CloseFDBatched(ctx, rootInode.ControlFD)
}
@@ -524,18 +524,18 @@ func (fs *filesystem) initClientAndRoot(ctx context.Context) error {
return err
}
func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error) {
func (fs *filesystem) initClientLisa(ctx context.Context) (lisafs.Inode, error) {
sock, err := unet.NewSocket(fs.opts.fd)
if err != nil {
return nil, err
return lisafs.Inode{}, err
}
var rootInode *lisafs.Inode
var rootInode lisafs.Inode
ctx.UninterruptibleSleepStart(false)
fs.clientLisa, rootInode, err = lisafs.NewClient(sock)
ctx.UninterruptibleSleepFinish(false)
if err != nil {
return nil, err
return lisafs.Inode{}, err
}
if fs.opts.aname == "/" {
return rootInode, nil
@@ -546,7 +546,7 @@ func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error)
status, inodes, err := rootFD.WalkMultiple(ctx, strings.Split(fs.opts.aname, "/"))
rootFD.CloseBatched(ctx)
if err != nil {
return nil, err
return lisafs.Inode{}, err
}
// Close all intermediate FDs to the attach point.
@@ -558,12 +558,12 @@ func (fs *filesystem) initClientLisa(ctx context.Context) (*lisafs.Inode, error)
switch status {
case lisafs.WalkSuccess:
return &inodes[numInodes-1], nil
return inodes[numInodes-1], nil
default:
last := fs.clientLisa.NewFD(inodes[numInodes-1].ControlFD)
last.CloseBatched(ctx)
log.Warningf("initClientLisa failed because walk to attach point %q failed: lisafs.WalkStatus = %v", fs.opts.aname, status)
return nil, unix.ENOENT
return lisafs.Inode{}, unix.ENOENT
}
}
+2 -2
View File
@@ -195,7 +195,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest
if err != nil {
return err
}
if err := fs.root.restoreFileLisa(ctx, rootInode, &opts); err != nil {
if err := fs.root.restoreFileLisa(ctx, &rootInode, &opts); err != nil {
return err
}
} else {
@@ -381,7 +381,7 @@ func (d *dentry) restoreRecursive(ctx context.Context, opts *vfs.CompleteRestore
if err != nil {
return err
}
if err := d.restoreFileLisa(ctx, inode, opts); err != nil {
if err := d.restoreFileLisa(ctx, &inode, opts); err != nil {
return err
}
} else {