On Thu, 2022-02-17 at 17:33 -0500, trondmy@xxxxxxxxxx wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> If a file is being written to, then readdirplus isn't going to help
> with
> retrieving attributes, since we will have to flush out writes anyway
> in
> order to sync the mtime/ctime.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/inode.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 1bef81f5373a..00500c369c5f 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -837,6 +837,7 @@ int nfs_getattr(struct user_namespace
> *mnt_userns, const struct path *path,
> int err = 0;
> bool force_sync = query_flags & AT_STATX_FORCE_SYNC;
> bool do_update = false;
> + bool record_cache = !nfs_have_writebacks(inode);
There is a second case where readdirplus won't help stat() performance:
if the user has specified 'noac' or has otherwise set values for the
acdirmax/acregmax that are too low, then caching breaks down.
Also fixed in the version committed to 'testing'.
>
> trace_nfs_getattr_enter(inode);
>
> @@ -845,7 +846,8 @@ int nfs_getattr(struct user_namespace
> *mnt_userns, const struct path *path,
> STATX_INO | STATX_SIZE | STATX_BLOCKS;
>
> if ((query_flags & AT_STATX_DONT_SYNC) && !force_sync) {
> - nfs_readdirplus_parent_cache_hit(path->dentry);
> + if (record_cache)
> + nfs_readdirplus_parent_cache_hit(path-
> >dentry);
> goto out_no_revalidate;
> }
>
> @@ -894,17 +896,18 @@ int nfs_getattr(struct user_namespace
> *mnt_userns, const struct path *path,
> if (request_mask & STATX_BLOCKS)
> do_update |= cache_validity & NFS_INO_INVALID_BLOCKS;
>
> - if (do_update) {
> + if (record_cache) {
> /* Update the attribute cache */
> - if (!(server->flags & NFS_MOUNT_NOAC))
> + if (do_update && !(server->flags & NFS_MOUNT_NOAC))
> nfs_readdirplus_parent_cache_miss(path-
> >dentry);
> else
> nfs_readdirplus_parent_cache_hit(path-
> >dentry);
> + }
> + if (do_update) {
> err = __nfs_revalidate_inode(server, inode);
> if (err)
> goto out;
> - } else
> - nfs_readdirplus_parent_cache_hit(path->dentry);
> + }
> out_no_revalidate:
> /* Only return attributes that were revalidated. */
> stat->result_mask = nfs_get_valid_attrmask(inode) |
> request_mask;
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
