Add intent to vfs.ResolvingPath in gofer client.

In some cases, like walkParentDirLocked(), we do not want to resolve the entire
path held in rp immediately. However, currently the gofer client still tries to
make an RPC to walk the entire path. This can be wasteful as callers of
walkParentDirLocked() are creation operations and the last path component does
not exist. So the gofer has to attempt to walk a non-existing file.

Add a non-exported `resolvingPath` type to gofer client which just wraps
*vfs.ResolvingPath. It can additionally hold such information.

This change also updates lisafsDentry to not attempt to walk the last path
component for such an rp.

PiperOrigin-RevId: 522151184
This commit is contained in:
Ayush Ranjan
2023-04-05 14:17:04 -07:00
committed by gVisor bot
parent ebda75f504
commit f0b8875509
6 changed files with 67 additions and 42 deletions
+1 -1
View File
@@ -248,7 +248,7 @@ func (d *dentry) getRemoteChild(ctx context.Context, name string) (*dentry, erro
// Postcondition: The returned dentry is already cached appropriately.
//
// +checklocksread:d.opMu
func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp resolvingPath, ds **[]*dentry) (*dentry, error) {
switch dt := d.impl.(type) {
case *lisafsDentry:
return dt.getRemoteChildAndWalkPathLocked(ctx, rp, ds)
+14 -12
View File
@@ -177,7 +177,7 @@ func (fs *filesystem) renameMuUnlockAndCheckCaching(ctx context.Context, ds **[]
// part of rp must have been revalidated.
//
// +checklocksread:d.opMu
func (fs *filesystem) stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, mayFollowSymlinks bool, ds **[]*dentry) (*dentry, bool, error) {
func (fs *filesystem) stepLocked(ctx context.Context, rp resolvingPath, d *dentry, mayFollowSymlinks bool, ds **[]*dentry) (*dentry, bool, error) {
if !d.isDir() {
return nil, false, linuxerr.ENOTDIR
}
@@ -293,7 +293,7 @@ func (fs *filesystem) getRemoteChildLocked(ctx context.Context, parent *dentry,
// may prefetch the entire path represented by rp.
//
// +checklocksread:parent.opMu
func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp resolvingPath, ds **[]*dentry) (*dentry, error) {
if child, err := parent.getCachedChildLocked(rp.Component()); child != nil || err != nil {
return child, err
}
@@ -344,11 +344,12 @@ func (d *dentry) getCachedChildLocked(name string) (*dentry, error) {
// - !rp.Done().
// - If !d.cachedMetadataAuthoritative(), then d's cached metadata must be up
// to date.
func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, ds **[]*dentry) (*dentry, error) {
if err := fs.revalidateParentDir(ctx, rp, d, ds); err != nil {
func (fs *filesystem) walkParentDirLocked(ctx context.Context, vfsRP *vfs.ResolvingPath, d *dentry, ds **[]*dentry) (*dentry, error) {
rp := resolvingPathParent(vfsRP)
if err := fs.revalidatePath(ctx, rp, d, ds); err != nil {
return nil, err
}
for !rp.Final() {
for !rp.done() {
d.opMu.RLock()
next, followedSymlink, err := fs.stepLocked(ctx, rp, d, true /* mayFollowSymlinks */, ds)
d.opMu.RUnlock()
@@ -357,7 +358,7 @@ func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.Resolving
}
d = next
if followedSymlink {
if err := fs.revalidateParentDir(ctx, rp, d, ds); err != nil {
if err := fs.revalidatePath(ctx, rp, d, ds); err != nil {
return nil, err
}
}
@@ -371,12 +372,13 @@ func (fs *filesystem) walkParentDirLocked(ctx context.Context, rp *vfs.Resolving
// resolveLocked resolves rp to an existing file.
//
// Preconditions: fs.renameMu must be locked.
func (fs *filesystem) resolveLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
func (fs *filesystem) resolveLocked(ctx context.Context, vfsRP *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
rp := resolvingPathFull(vfsRP)
d := rp.Start().Impl().(*dentry)
if err := fs.revalidatePath(ctx, rp, d, ds); err != nil {
return nil, err
}
for !rp.Done() {
for !rp.done() {
d.opMu.RLock()
next, followedSymlink, err := fs.stepLocked(ctx, rp, d, true /* mayFollowSymlinks */, ds)
d.opMu.RUnlock()
@@ -599,7 +601,7 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b
return linuxerr.ENOENT
}
} else {
child, _, err = fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, &ds)
child, _, err = fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, &ds)
if err != nil {
return err
}
@@ -872,7 +874,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v
// to creating a synthetic one, i.e. one that is kept entirely in memory.
// Check that we're not overriding an existing file with a synthetic one.
_, _, err := fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, ds) // +checklocksforce: parent.opMu taken by doCreateAt.
_, _, err := fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, ds) // +checklocksforce: parent.opMu taken by doCreateAt.
switch {
case err == nil:
// Step succeeded, another file exists.
@@ -972,7 +974,7 @@ afterTrailingSymlink:
// serializing OpenAt calls in the same directory in the common case
// that the file exists.
parent.opMu.RLock()
child, followedSymlink, err := fs.stepLocked(ctx, rp, parent, true /* mayFollowSymlinks */, &ds)
child, followedSymlink, err := fs.stepLocked(ctx, resolvingPathFull(rp), parent, true /* mayFollowSymlinks */, &ds)
parent.opMu.RUnlock()
if followedSymlink {
if mustCreate {
@@ -1012,7 +1014,7 @@ afterTrailingSymlink:
// Step to the file again. Since we still hold opMu for
// writing, there can't be a race here.
child, _, err = fs.stepLocked(ctx, rp, parent, false /* mayFollowSymlinks */, &ds)
child, _, err = fs.stepLocked(ctx, resolvingPathFull(rp), parent, false /* mayFollowSymlinks */, &ds)
parent.opMu.Unlock()
}
if err != nil {
+37
View File
@@ -2198,3 +2198,40 @@ func (fd *fileDescription) LockPOSIX(ctx context.Context, uid fslock.UniqueID, o
func (fd *fileDescription) UnlockPOSIX(ctx context.Context, uid fslock.UniqueID, r fslock.LockRange) error {
return fd.Locks().UnlockPOSIX(ctx, uid, r)
}
// resolvingPath is just a wrapper around *vfs.ResolvingPath. It additionally
// holds some information around the intent behind resolving the path.
type resolvingPath struct {
*vfs.ResolvingPath
// excludeLast indicates whether the intent is to resolve until the last path
// component. If true, the last path component should remain unresolved.
excludeLast bool
}
func resolvingPathFull(rp *vfs.ResolvingPath) resolvingPath {
return resolvingPath{ResolvingPath: rp, excludeLast: false}
}
func resolvingPathParent(rp *vfs.ResolvingPath) resolvingPath {
return resolvingPath{ResolvingPath: rp, excludeLast: true}
}
func (rp *resolvingPath) done() bool {
if rp.excludeLast {
return rp.Final()
}
return rp.Done()
}
func (rp *resolvingPath) copy() resolvingPath {
return resolvingPath{
ResolvingPath: rp.ResolvingPath.Copy(),
excludeLast: rp.excludeLast,
}
}
// Precondition: !rp.done() && rp.Component() is not "." or "..".
func (rp *resolvingPath) getComponents(emit func(string) bool) {
rp.GetComponents(rp.excludeLast, emit)
}
+3 -3
View File
@@ -286,14 +286,14 @@ 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.Component() is not "." or "..".
// - !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) {
func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp resolvingPath, ds **[]*dentry) (*dentry, error) {
// 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 {
rp.getComponents(func(name string) bool {
if name == "." {
return true
}
+6 -24
View File
@@ -44,7 +44,7 @@ func (errRevalidationStepDone) Error() string {
//
// Preconditions:
// - fs.renameMu must be locked.
func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig *vfs.ResolvingPath, start *dentry, ds **[]*dentry) error {
func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig resolvingPath, start *dentry, ds **[]*dentry) error {
// Revalidation is done even if start is synthetic in case the path is
// something like: ../non_synthetic_file.
if fs.opts.interop != InteropModeShared {
@@ -52,26 +52,8 @@ func (fs *filesystem) revalidatePath(ctx context.Context, rpOrig *vfs.ResolvingP
}
// Copy resolving path to walk the path for revalidation.
rp := rpOrig.Copy()
err := fs.revalidate(ctx, rp, start, rp.Done, ds)
rp.Release(ctx)
return err
}
// revalidateParentDir does the same as revalidatePath, but stops at the parent.
//
// Preconditions:
// - fs.renameMu must be locked.
func (fs *filesystem) revalidateParentDir(ctx context.Context, rpOrig *vfs.ResolvingPath, start *dentry, ds **[]*dentry) error {
// Revalidation is done even if start is synthetic in case the path is
// something like: ../non_synthetic_file and parent is non synthetic.
if fs.opts.interop != InteropModeShared {
return nil
}
// Copy resolving path to walk the path for revalidation.
rp := rpOrig.Copy()
err := fs.revalidate(ctx, rp, start, rp.Final, ds)
rp := rpOrig.copy()
err := fs.revalidate(ctx, rp, start, ds)
rp.Release(ctx)
return err
}
@@ -111,12 +93,12 @@ func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFile
// Preconditions:
// - fs.renameMu must be locked.
// - InteropModeShared is in effect.
func (fs *filesystem) revalidate(ctx context.Context, rp *vfs.ResolvingPath, start *dentry, done func() bool, ds **[]*dentry) error {
func (fs *filesystem) revalidate(ctx context.Context, rp resolvingPath, start *dentry, ds **[]*dentry) error {
state := makeRevalidateState(start, true /* refreshStart */)
defer state.release()
done:
for cur := start; !done(); {
for cur := start; !rp.done(); {
var err error
cur, err = fs.revalidateStep(ctx, rp, cur, state)
if err != nil {
@@ -161,7 +143,7 @@ done:
// - fs.renameMu must be locked.
// - !rp.Done().
// - InteropModeShared is in effect (assumes no negative dentries).
func (fs *filesystem) revalidateStep(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, state *revalidateState) (*dentry, error) {
func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *dentry, state *revalidateState) (*dentry, error) {
switch name := rp.Component(); name {
case ".":
// Do nothing.
+6 -2
View File
@@ -255,12 +255,16 @@ 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) {
// update rp state. It halts if emit() returns false. If excludeLast is true,
// then the last path component is not emitted.
func (rp *ResolvingPath) GetComponents(excludeLast bool, emit func(string) bool) {
// Copy rp state.
cur := rp.pit
curPart := rp.curPart
for cur.Ok() {
if excludeLast && curPart == 0 && !cur.NextOk() {
break
}
if !emit(cur.String()) {
break
}