From a102e7e0fa467be956cc89cd828b9543f48521ea Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Tue, 4 Apr 2023 23:20:43 -0700 Subject: [PATCH] Use a vfs.ResolvingPath to perform prefetch walk in LISAFS. Earlier we were using `fspath.Iterator`. However, that is not indicative of the entire path being walked. `vfs.ResolvingPath` holds an array of iterators. Multiple iterators are used as symlinks are resolved during path traversal. We do not use `vfs.ResolvingPath.Copy()` because this is on a hot path. Instead we have added a const function GetComponents() which emits all the unresolved path components in `vfs.ResolvingPath` without changing its state. This way we should be able to prefetch more path components in one RPC. PiperOrigin-RevId: 521964461 --- pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 19 ++++++++++--------- pkg/sentry/vfs/resolving_path.go | 24 ++++++++++++++++++------ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index e210658f8..578f429b2 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -286,23 +286,24 @@ func (d *lisafsDentry) getRemoteChild(ctx context.Context, name string) (*dentry // - fs.renameMu must be locked. // - d.opMu must be locked. // - d.isDir(). -// - !rp.Done(). +// - !rp.Done() && rp.Component() is not "." or "..". // // Postcondition: The returned dentry is already cached appropriately. func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { - // Walk as much of the path as possible in 1 RPC. - // Note that pit is a copy of the iterator that does not affect rp. - var names []string - for pit := rp.Pit(); pit.Ok(); pit = pit.Next() { - name := pit.String() + // Collect as many path components as possible to walk. + var namesArr [16]string // arbitrarily sized array to help avoid slice allocation. + names := namesArr[:0] + rp.GetComponents(func(name string) bool { if name == "." { - continue + return true } if name == ".." { - break + return false } names = append(names, name) - } + return true + }) + // Walk as much of the path as possible in 1 RPC. _, inodes, err := d.controlFD.WalkMultiple(ctx, names) if err != nil { return nil, err diff --git a/pkg/sentry/vfs/resolving_path.go b/pkg/sentry/vfs/resolving_path.go index 710e87a93..ad059a6f7 100644 --- a/pkg/sentry/vfs/resolving_path.go +++ b/pkg/sentry/vfs/resolving_path.go @@ -223,12 +223,6 @@ func (rp *ResolvingPath) Final() bool { return rp.curPart == 0 && !rp.pit.NextOk() } -// Pit returns a copy of rp's current path iterator. Modifying the iterator -// does not change rp. -func (rp *ResolvingPath) Pit() fspath.Iterator { - return rp.pit -} - // Component returns the current path component in the stream represented by // rp. // @@ -260,6 +254,24 @@ func (rp *ResolvingPath) Advance() { } } +// GetComponents emits all the remaining path components in rp. It does *not* +// update rp state. It halts if emit() returns false. +func (rp *ResolvingPath) GetComponents(emit func(string) bool) { + // Copy rp state. + cur := rp.pit + curPart := rp.curPart + for cur.Ok() { + if !emit(cur.String()) { + break + } + cur = cur.Next() + if !cur.Ok() && curPart > 0 { + curPart-- + cur = rp.parts[curPart] + } + } +} + // CheckRoot is called before resolving the parent of the Dentry d. If the // Dentry is contextually a VFS root, such that path resolution should treat // d's parent as itself, CheckRoot returns (true, nil). If the Dentry is the