On Tue, Mar 02, 2021 at 11:28:35PM -0500, trondmy@xxxxxxxxxx wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> There should be no reason to expect the directory permissions to change
> just because the directory contents changed or a negative lookup timed
> out. So let's avoid doing a full call to nfs_mark_for_revalidate() in
> that case.
> Furthermore, if this is a negative dentry, and we haven't actually done
> a new lookup, then we have no reason yet to believe the directory has
> changed at all. So let's remove the gratuitous directory inode
> invalidation altogether when called from
> nfs_lookup_revalidate_negative().
Thanks! I tested this patch and 2/2 from this series, and I can confirm
that it addresses the issue that we were seeing.
Tested-by: Geert Jansen <gerardu@xxxxxxxxxx>
> Reported-by: Geert Jansen <gerardu@xxxxxxxxxx>
> Fixes: 5ceb9d7fdaaf ("NFS: Refactor nfs_lookup_revalidate()")
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/dir.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 19a9f434442f..6350873cb8bd 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1401,6 +1401,15 @@ int nfs_lookup_verify_inode(struct inode *inode, unsigned int flags)
> goto out;
> }
>
> +static void nfs_mark_dir_for_revalidate(struct inode *inode)
> +{
> + struct nfs_inode *nfsi = NFS_I(inode);
> +
> + spin_lock(&inode->i_lock);
> + nfsi->cache_validity |= NFS_INO_REVAL_PAGECACHE;
> + spin_unlock(&inode->i_lock);
> +}
> +
> /*
> * We judge how long we want to trust negative
> * dentries by looking at the parent inode mtime.
> @@ -1435,7 +1444,6 @@ nfs_lookup_revalidate_done(struct inode *dir, struct dentry *dentry,
> __func__, dentry);
> return 1;
> case 0:
> - nfs_mark_for_revalidate(dir);
> if (inode && S_ISDIR(inode->i_mode)) {
> /* Purge readdir caches. */
> nfs_zap_caches(inode);
> @@ -1525,6 +1533,8 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
> nfs_free_fattr(fattr);
> nfs_free_fhandle(fhandle);
> nfs4_label_free(label);
> + if (!ret)
> + nfs_mark_dir_for_revalidate(dir);
> return nfs_lookup_revalidate_done(dir, dentry, inode, ret);
> }
>
> @@ -1567,7 +1577,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
> error = nfs_lookup_verify_inode(inode, flags);
> if (error) {
> if (error == -ESTALE)
> - nfs_zap_caches(dir);
> + nfs_mark_dir_for_revalidate(dir);
> goto out_bad;
> }
> nfs_advise_use_readdirplus(dir);
> @@ -2064,7 +2074,7 @@ nfs_add_or_obtain(struct dentry *dentry, struct nfs_fh *fhandle,
> dput(parent);
> return d;
> out_error:
> - nfs_mark_for_revalidate(dir);
> + nfs_mark_dir_for_revalidate(dir);
> d = ERR_PTR(error);
> goto out;
> }
> --
> 2.29.2
>
