Re: [PATCH v4 21/21] NFS: Do uncached readdir when we're seeking a cookie in an empty page cache

["Benjamin Coddington" <bcodding@redhat.com>]

On 7 Nov 2020, at 9:03, trondmy@kernel.org wrote:

> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> If the directory is changing, causing the page cache to get 
> invalidated
> while we are listing the contents, then the NFS client is currently 
> forced
> to read in the entire directory contents from scratch, because it 
> needs
> to perform a linear search for the readdir cookie. While this is not
> an issue for small directories, it does not scale to directories with
> millions of entries.
> In order to be able to deal with large directories that are changing,
> add a heuristic to ensure that if the page cache is empty, and we are
> searching for a cookie that is not the zero cookie, we just default to
> performing uncached readdir.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/dir.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 238872d116f7..d7a9efd31ecd 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -917,11 +917,28 @@ static int find_and_lock_cache_page(struct 
> nfs_readdir_descriptor *desc)
>  	return res;
>  }
>
> +static bool nfs_readdir_dont_search_cache(struct 
> nfs_readdir_descriptor *desc)
> +{
> +	struct address_space *mapping = desc->file->f_mapping;
> +	struct inode *dir = file_inode(desc->file);
> +	unsigned int dtsize = NFS_SERVER(dir)->dtsize;
> +	loff_t size = i_size_read(dir);
> +
> +	/*
> +	 * Default to uncached readdir if the page cache is empty, and
> +	 * we're looking for a non-zero cookie in a large directory.
> +	 */
> +	return desc->dir_cookie != 0 && mapping->nrpages == 0 && size > 
> dtsize;

inode size > dtsize is a little hand-wavy.  We have a lot of customers 
trying to
reverse-engineer nfs_readdir() behavior instead of reading the code, 
this
is sure to drive them crazy.

That said, in the absence of an easy way to make it tunable, I don't 
have
anything better to suggest.

Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Ben


> +}
> +
>  /* Search for desc->dir_cookie from the beginning of the page cache 
> */
>  static int readdir_search_pagecache(struct nfs_readdir_descriptor 
> *desc)
>  {
>  	int res;
>
> +	if (nfs_readdir_dont_search_cache(desc))
> +		return -EBADCOOKIE;
> +
>  	do {
>  		if (desc->page_index == 0) {
>  			desc->current_index = 0;
> -- 
> 2.28.0