On 12 Nov 2020, at 13:26, Trond Myklebust wrote:
> On Thu, 2020-11-12 at 16:51 +0000, Trond Myklebust wrote:
>>
>> I was going to ask you if perhaps reverting Scott's commit
>> 07b5ce8ef2d8
>> ("NFS: Make nfs_readdir revalidate less often") might help here?
>> My thinking is that will trigger more cache invalidations when the
>> directory is changing underneath us, and will now trigger uncached
>> readdir in those situations.
>>
>>>
>
> IOW, the suggestion would be to apply something like the following on
> top of the existing readdir patchset:
I'm all for this approach, but - I'm rarely seeing the mapping->nrpages == 0
since the cache is dropped by a process in nfs_readdir() that immediately
starts filling the cache again.
It would make a lot more sense to me if we could do something like stash
desc->page_index << PAGE_SHIFT in f_pos after each nfs_readdir, then the
hueristic could check f_pos >> PAGE_SHIFT > nrpages.
Yes, f_pos is growing many different meanings in NFS directories, maybe we
can stash it on the directory context.
Ben
>
> ---
> fs/nfs/dir.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 3f70697729d8..384a4663f742 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -956,10 +956,10 @@ static int readdir_search_pagecache(struct
> nfs_readdir_descriptor *desc)
> {
> int res;
>
> - if (nfs_readdir_dont_search_cache(desc))
> - return -EBADCOOKIE;
> -
> do {
> + if (nfs_readdir_dont_search_cache(desc))
> + return -EBADCOOKIE;
> +
> if (desc->page_index == 0) {
> desc->current_index = 0;
> desc->prev_index = 0;
> @@ -1082,11 +1082,9 @@ static int nfs_readdir(struct file *file, struct
> dir_context *ctx)
> * to either find the entry with the appropriate number or
> * revalidate the cookie.
> */
> - if (ctx->pos == 0 || nfs_attribute_cache_expired(inode)) {
> - res = nfs_revalidate_mapping(inode, file->f_mapping);
> - if (res < 0)
> - goto out;
> - }
> + res = nfs_revalidate_mapping(inode, file->f_mapping);
> + if (res < 0)
> + goto out;
>
> res = -ENOMEM;
> desc = kzalloc(sizeof(*desc), GFP_KERNEL);
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
