If we skip lookup revalidation while holding a delegation, we might miss that the file has changed directories on the server. The directory's change attribute should still be checked against the dentry's d_time to perform a complete revalidation. V2 - Add some commentary as suggested-by J. Bruce Fields. Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx> --- fs/nfs/dir.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 0adfd8840110..8723e82f5c9d 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -1197,12 +1197,20 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry, goto out_bad; } - if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ)) - return nfs_lookup_revalidate_delegated(dir, dentry, inode); - /* Force a full look up iff the parent directory has changed */ if (!(flags & (LOOKUP_EXCL | LOOKUP_REVAL)) && nfs_check_verifier(dir, dentry, flags & LOOKUP_RCU)) { + + /* + * Note that the file can't move while we hold a + * delegation. But this dentry could have been cached + * before we got a delegation. So it's only safe to + * skip revalidation when the parent directory is + * unchanged: + */ + if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ)) + return nfs_lookup_revalidate_delegated(dir, dentry, inode); + error = nfs_lookup_verify_inode(inode, flags); if (error) { if (error == -ESTALE) @@ -1635,9 +1643,6 @@ nfs4_do_lookup_revalidate(struct inode *dir, struct dentry *dentry, if (inode == NULL) goto full_reval; - if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ)) - return nfs_lookup_revalidate_delegated(dir, dentry, inode); - /* NFS only supports OPEN on regular files */ if (!S_ISREG(inode->i_mode)) goto full_reval; -- 2.20.1