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

Benjamin Coddington
2020-11-09T21:41:19Z

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