diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index eaeee86da..6853d48b5 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -426,3 +426,23 @@ func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptio panic("unknown dentry implementation") } } + +// doRevalidation calls into r.start's dentry implementation to perform +// revalidation on all the dentries contained in r. +// +// Preconditions: +// - fs.renameMu must be locked. +// - InteropModeShared is in effect. +func (r *revalidateState) doRevalidation(ctx context.Context, vfsObj *vfs.VirtualFilesystem, ds **[]*dentry) error { + // Skip synthetic dentries because there is no actual implementation that can + // be used to walk the remote filesystem. A start dentry cannot be replaced. + if r.start.isSynthetic() { + return nil + } + switch r.start.impl.(type) { + case *lisafsDentry: + return doRevalidationLisafs(ctx, vfsObj, r, ds) + default: + panic("unknown dentry implementation") + } +} diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index c7d0e88c8..fec896d0e 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -526,3 +526,84 @@ func (d *lisafsDentry) restoreFile(ctx context.Context, inode *lisafs.Inode, opt return nil } + +// doRevalidationLisafs stats all dentries in `state`. It will update or +// invalidate dentries in the cache based on the result. +// +// Preconditions: +// - fs.renameMu must be locked. +// - InteropModeShared is in effect. +func doRevalidationLisafs(ctx context.Context, vfsObj *vfs.VirtualFilesystem, state *revalidateState, ds **[]*dentry) error { + start := state.start.impl.(*lisafsDentry) + + // Populate state.names. + state.names = state.names[:0] // For sanity. + if state.refreshStart { + state.names = append(state.names, "") + } + for _, d := range state.dentries { + state.names = append(state.names, d.name) + } + + // Lock metadata on all dentries *before* getting attributes for them. + if state.refreshStart { + start.metadataMu.Lock() + defer start.metadataMu.Unlock() + } + for _, d := range state.dentries { + d.metadataMu.Lock() + } + // lastUnlockedDentry keeps track of the dentries in state.dentries that have + // already had their metadataMu unlocked. Avoid defer unlock in the loop + // above to avoid heap allocation. + lastUnlockedDentry := -1 + defer func() { + // Advance to the first unevaluated dentry and unlock the remaining + // dentries. + for lastUnlockedDentry++; lastUnlockedDentry < len(state.dentries); lastUnlockedDentry++ { + state.dentries[lastUnlockedDentry].metadataMu.Unlock() + } + }() + + // Make WalkStat RPC. + stats, err := start.controlFD.WalkStat(ctx, state.names) + if err != nil { + return err + } + + if state.refreshStart { + if len(stats) > 0 { + // First dentry is where the search is starting, just update attributes + // since it cannot be replaced. + start.updateMetadataFromStatxLocked(&stats[0]) // +checklocksforce: see above. + stats = stats[1:] + } + } + + for i := 0; i < len(state.dentries); i++ { + d := state.dentries[i] + found := i < len(stats) + // Advance lastUnlockedDentry. It is the responsibility of this for loop + // block to unlock d.metadataMu. + lastUnlockedDentry = i + + // Note that synthetic dentries will always fail this comparison check. + if !found || d.inoKey != inoKeyFromStatx(&stats[i]) { + d.metadataMu.Unlock() + if !found && d.isSynthetic() { + // We have a synthetic file, and no remote file has arisen to replace + // it. + return nil + } + // The file at this path has changed or no longer exists. Mark the + // dentry invalidated. + d.invalidate(ctx, vfsObj, ds) + return nil + } + + // The file at this path hasn't changed. Just update cached metadata. + d.impl.(*lisafsDentry).updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: see above. + d.metadataMu.Unlock() + } + return nil +} diff --git a/pkg/sentry/fsimpl/gofer/revalidate.go b/pkg/sentry/fsimpl/gofer/revalidate.go index 817fbe79e..7f0c42a48 100644 --- a/pkg/sentry/fsimpl/gofer/revalidate.go +++ b/pkg/sentry/fsimpl/gofer/revalidate.go @@ -15,7 +15,6 @@ package gofer import ( - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" @@ -81,6 +80,7 @@ func (fs *filesystem) revalidateParentDir(ctx context.Context, rpOrig *vfs.Resol // // Preconditions: // - fs.renameMu must be locked. +// - parent must have up to date metadata. func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFilesystem, parent *dentry, name string, ds **[]*dentry) error { // Skip revalidation for interop mode different than InteropModeShared or // if the parent is synthetic (child must be synthetic too, but it cannot be @@ -96,11 +96,12 @@ func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFile return nil } - state := makeRevalidateState(parent) + state := makeRevalidateState(parent, false /* refreshStart */) defer state.release() - - state.add(name, child) - return fs.revalidateHelper(ctx, vfsObj, state, ds) + // Note that child can not be nil, because we don't cache negative entries + // when InteropModeShared is in effect. + state.add(child) + return state.doRevalidation(ctx, vfsObj, ds) } // revalidate revalidates path components in rp until done returns true, or @@ -111,15 +112,9 @@ func (fs *filesystem) revalidateOne(ctx context.Context, vfsObj *vfs.VirtualFile // - 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 { - state := makeRevalidateState(start) + state := makeRevalidateState(start, true /* refreshStart */) defer state.release() - // Skip synthetic dentries because the start dentry cannot be replaced in case - // it has been created in the remote file system. - if !start.isSynthetic() { - state.add("", start) - } - done: for cur := start; !done(); { var err error @@ -127,20 +122,13 @@ done: if err != nil { switch err.(type) { case errPartialRevalidation: - if err := fs.revalidateHelper(ctx, rp.VirtualFilesystem(), state, ds); err != nil { + if err := state.doRevalidation(ctx, rp.VirtualFilesystem(), ds); err != nil { return err } // Reset state to release any remaining locks and restart from where // stepping stopped. - state.reset() - state.start = cur - - // Skip synthetic dentries because the start dentry cannot be replaced in - // case it has been created in the remote file system. - if !cur.isSynthetic() { - state.add("", cur) - } + state.reset(cur /* start */, true /* refreshStart */) case errRevalidationStepDone: break done @@ -150,7 +138,7 @@ done: } } } - return fs.revalidateHelper(ctx, rp.VirtualFilesystem(), state, ds) + return state.doRevalidation(ctx, rp.VirtualFilesystem(), ds) } // revalidateStep walks one element of the path and updates revalidationState @@ -209,7 +197,9 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp *vfs.ResolvingPath, return nil, errRevalidationStepDone{} } - state.add(name, child) + // Note that child can not be nil, because we don't cache negative entries + // when InteropModeShared is in effect. + state.add(child) // Symlink must be resolved before continuing with revalidation. if child.isSymlink() { @@ -223,132 +213,46 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp *vfs.ResolvingPath, return d, nil } -// revalidateHelper calls the gofer to stat all dentries in `state`. It will -// update or invalidate dentries in the cache based on the result. -// -// Preconditions: -// - fs.renameMu must be locked. -// - InteropModeShared is in effect. -func (fs *filesystem) revalidateHelper(ctx context.Context, vfsObj *vfs.VirtualFilesystem, state *revalidateState, ds **[]*dentry) error { - if len(state.names) == 0 { - return nil - } - // Lock metadata on all dentries *before* getting attributes for them. - state.lockAllMetadata() +// Precondition: fs.renameMu must be locked. +func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem, ds **[]*dentry) { + // If the dentry is a mountpoint, InvalidateDentry may drop the + // last reference on it, resulting in lock recursion. To avoid + // this, take a dentry reference first, then drop it while + // deferring the call to dentry.checkCachingLocked(). + d.IncRef() + vfsObj.InvalidateDentry(ctx, &d.vfsd) + d.decRefNoCaching() - var stats []linux.Statx - switch dt := state.start.impl.(type) { - case *lisafsDentry: - var err error - stats, err = dt.controlFD.WalkStat(ctx, state.names) - if err != nil { - return err - } - default: - panic("unknown dentry implementation") + // Re-evaluate its caching status (i.e. if it has 0 references, drop it). + // The dentry will be reloaded next time it's accessed. + *ds = appendDentry(*ds, d) + + d.parent.opMu.RLock() + defer d.parent.opMu.RUnlock() + d.parent.childrenMu.Lock() + defer d.parent.childrenMu.Unlock() + + if d.isSynthetic() { + // Normally we don't mark invalidated dentries as deleted since + // they may still exist (but at a different path), and also for + // consistency with Linux. However, synthetic files are guaranteed + // to become unreachable if their dentries are invalidated, so + // treat their invalidation as deletion. + d.setDeleted() + d.decRefNoCaching() + *ds = appendDentry(*ds, d) + + d.parent.syntheticChildren-- + d.parent.clearDirentsLocked() } - i := -1 - for d := state.popFront(); d != nil; d = state.popFront() { - i++ - var found bool - switch state.start.impl.(type) { - case *lisafsDentry: - found = i < len(stats) - default: - panic("unknown dentry implementation") - } - if i == 0 && len(state.names[0]) == 0 { - if found && !d.isSynthetic() { - // First dentry is where the search is starting, just update attributes - // since it cannot be replaced. - switch dt := d.impl.(type) { - case *lisafsDentry: - dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: acquired by lockAllMetadata. - default: - panic("unknown dentry implementation") - } - } - d.metadataMu.Unlock() // +checklocksforce: see above. - continue - } - - var fileChanged bool - if found { - switch d.impl.(type) { - case *lisafsDentry: - fileChanged = d.inoKey != inoKeyFromStatx(&stats[i]) - case nil: - // A remote file was found to replace this synthetic file. - fileChanged = true - default: - panic("unknown dentry implementation") - } - } - // Note that synthetic dentries will always fail this comparison check. - if !found || fileChanged { - d.metadataMu.Unlock() // +checklocksforce: see above. - if !found && d.isSynthetic() { - // We have a synthetic file, and no remote file has arisen to replace - // it. - return nil - } - // The file at this path has changed or no longer exists. Mark the - // dentry invalidated, and re-evaluate its caching status (i.e. if - // it has 0 references, drop it). The dentry will be reloaded next - // time it's accessed. - // - // If the dentry is a mountpoint, InvalidateDentry may drop the - // last reference on it, resulting in lock recursion. To avoid - // this, take a dentry reference first, then drop it while - // deferring the call to dentry.checkCachingLocked(). - d.IncRef() - vfsObj.InvalidateDentry(ctx, &d.vfsd) - d.decRefNoCaching() - *ds = appendDentry(*ds, d) - - name := state.names[i] - d.parent.opMu.RLock() - - d.parent.childrenMu.Lock() - if d.isSynthetic() { - // Normally we don't mark invalidated dentries as deleted since - // they may still exist (but at a different path), and also for - // consistency with Linux. However, synthetic files are guaranteed - // to become unreachable if their dentries are invalidated, so - // treat their invalidation as deletion. - d.setDeleted() - d.decRefNoCaching() - *ds = appendDentry(*ds, d) - - d.parent.syntheticChildren-- - d.parent.clearDirentsLocked() - } - - // Since the opMu was released and reacquired, re-check that the - // parent's child with this name is still the same. Do not touch it if - // it has been replaced with a different one. - if child := d.parent.children[name]; child == d { - // Invalidate dentry so it gets reloaded next time it's accessed. - delete(d.parent.children, name) - } - d.parent.childrenMu.Unlock() - d.parent.opMu.RUnlock() - - return nil - } - - // The file at this path hasn't changed. Just update cached metadata. - switch dt := d.impl.(type) { - case *lisafsDentry: - dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: see above. - default: - panic("unknown dentry implementation") - } - d.metadataMu.Unlock() + // Since the opMu was just reacquired above, re-check that the + // parent's child with this name is still the same. Do not touch it if + // it has been replaced with a different one. + if child := d.parent.children[d.name]; child == d { + // Invalidate dentry so it gets reloaded next time it's accessed. + delete(d.parent.children, d.name) } - - return nil } // revalidateStatePool caches revalidateState instances to save array @@ -364,71 +268,51 @@ var revalidateStatePool = sync.Pool{ // dentries. The list must be in ancestry order, in other words `n` must be // `n-1` child. type revalidateState struct { - // start is the dentry where to start the attributes search. + // start is the dentry where to start the revalidation of dentries. start *dentry - // List of names of entries to refresh attributes. Names length must be the - // same as detries length. They are kept in separate slices because names is - // used to call File.MultiGetAttr(). + // refreshStart indicates whether the attributes of the start dentry should + // be refreshed. + refreshStart bool + + // names is just a slice of names which can be used while making LISAFS RPCs. + // This exists to avoid the cost of repeated string slice allocation to make + // RPCs. names []string - // dentries is the list of dentries that correspond to the names above. - // dentry.metadataMu is acquired as each dentry is added to this list. + // dentries is the list of dentries that need to be revalidated. The first + // dentry is a child of start and each successive dentry is a child of the + // previous. dentries []*dentry - - // locked indicates if metadata lock has been acquired on dentries. - locked bool } -func makeRevalidateState(start *dentry) *revalidateState { +func makeRevalidateState(start *dentry, refreshStart bool) *revalidateState { r := revalidateStatePool.Get().(*revalidateState) r.start = start + r.refreshStart = refreshStart return r } // release must be called after the caller is done with this object. It releases // all metadata locks and resources. func (r *revalidateState) release() { - r.reset() + r.reset(nil /* start */, false /* refreshStart */) revalidateStatePool.Put(r) } // Preconditions: +// - d != nil. // - d is a descendant of all dentries in r.dentries. -func (r *revalidateState) add(name string, d *dentry) { - r.names = append(r.names, name) +func (r *revalidateState) add(d *dentry) { r.dentries = append(r.dentries, d) } -// +checklocksignore -func (r *revalidateState) lockAllMetadata() { - for _, d := range r.dentries { - d.metadataMu.Lock() - } - r.locked = true -} - -func (r *revalidateState) popFront() *dentry { - if len(r.dentries) == 0 { - return nil - } - d := r.dentries[0] - r.dentries = r.dentries[1:] - return d -} - // reset releases all metadata locks and resets all fields to allow this // instance to be reused. // +checklocksignore -func (r *revalidateState) reset() { - if r.locked { - // Unlock any remaining dentries. - for _, d := range r.dentries { - d.metadataMu.Unlock() - } - r.locked = false - } - r.start = nil +func (r *revalidateState) reset(start *dentry, refreshStart bool) { + r.start = start + r.refreshStart = refreshStart r.names = r.names[:0] r.dentries = r.dentries[:0] }