Re: [PATCH v2 00/16] Readdir enhancements

[Trond Myklebust <trondmy@hammerspace.com>]

Hi Ben

Thanks for the review and the testing!

On Wed, 2020-11-04 at 11:14 -0500, Benjamin Coddington wrote:
> Hi Trond, these look great!
> 
> I'm doing some comparison testing before/after this set, and I'm
> getting
> into some memory pressure on a client with 4G ram listing 1.5M
> dentries 
> with
> 12 char filenames.
> 
> It looks like before this set, the readdir code was a bit more
> resilient 
> in
> the face of memory pressure, and I'm wondering if we've dropped a
> call 
> to
> mark_page_accessed().
> 
> * Ben adds:
> 
> @@ -460,7 +461,8 @@ static int nfs_readdir_search_array(struct 
> nfs_readdir_descriptor *desc)
>                  desc->last_cookie = array->last_cookie;
>                  desc->current_index += array->size;
>                  desc->page_index++;
> -       }
> +       } else
> +               mark_page_accessed(desc->page);
>          kunmap_atomic(array);
>          return status;
>   }
> 
> .. no, that's not any better.  I'm still getting evicted pages (or,
> at
> least, low-indexed pages that don't have PageUptodate() set), which 
> makes
> it nearly impossible to finish listing this directory because we just
> keep
> invalidating the mapping.
> 

You're right that I had screwed up the page access marking in the
previous patchsets. I believe this should be fixed in v3 by the
conversion to use grab_cache_page(), which calls find_or_create_page()
and should therefore do the right thing with the FGP_ACCESSED flag.

I believe the reason why your patch above fails to fully correct the
issue is because we always want to mark the page as accessed if we've
scanned it.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com