From 3d88585eb8c1bc3eefcf267a2e3f82090da683ac Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Fri, 31 Mar 2023 10:43:00 -0700 Subject: [PATCH] Do not infer negative entry using lisafs.WalkComponentDoesNotExist. After 8342c57ae870 ("Split gofer.dentry.dirMu into two mutexes"), the gofer client allows concurrent walks under a directory dentry. Also creation operations (like mkdir, symlink, etc) only write lock dentry.opMu for the parent directory. This means that when performing a WalkMultiple RPC, the status = WalkComponentDoesNotExist information is not useful anymore. Lets take an example, A.WalkMultiple([B, C, D]) => [B inode, C inode] (indicating D does not exist). The RPC is made with A.opMu read locked. So D can be created concurrently because that only requires C.opMu write lock. So we can not create a negative entry for D. D could have been created and evicted from the dentry tree. So we can't know about its presence on the remote filesystem. PiperOrigin-RevId: 520960039 --- pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index 045d7b630..e210658f8 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -284,8 +284,8 @@ func (d *lisafsDentry) getRemoteChild(ctx context.Context, name string) (*dentry // Preconditions: // - fs.renameMu must be locked. -// - parent.opMu must be locked. -// - parent.isDir(). +// - d.opMu must be locked. +// - d.isDir(). // - !rp.Done(). // // Postcondition: The returned dentry is already cached appropriately. @@ -303,11 +303,13 @@ func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp * } names = append(names, name) } - status, inodes, err := d.controlFD.WalkMultiple(ctx, names) + _, inodes, err := d.controlFD.WalkMultiple(ctx, names) if err != nil { return nil, err } if len(inodes) == 0 { + // d.opMu is locked. So a new child could not have appeared concurrently. + // It should be safe to mark this as a negative entry. d.childrenMu.Lock() defer d.childrenMu.Unlock() d.cacheNegativeLookupLocked(names[0]) @@ -371,12 +373,6 @@ func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp * ret = child } } - - if status == lisafs.WalkComponentDoesNotExist && curParent.isDir() { - curParentLock() - curParent.cacheNegativeLookupLocked(names[len(inodes)]) // +checklocksforce: locked via curParentLock(). - curParentUnlock() - } return ret, dentryCreationErr }