Re: [PATCH v4 20/21] NFS: Reduce number of RPC calls when doing uncached readdir

Benjamin Coddington
2020-11-09T21:00:02Z

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

> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> If we're doing uncached readdir, allocate multiple pages in order to
> try to avoid duplicate RPC calls for the same getdents() call.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>

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

Ben

> ---
> fs/nfs/dir.c | 79
> ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 46 insertions(+), 33 deletions(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index b6c3501e8f61..238872d116f7 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -694,12 +694,14 @@ void nfs_prime_dcache(struct dentry *parent,
> struct nfs_entry *entry,
> static int nfs_readdir_page_filler(struct nfs_readdir_descriptor
> *desc,
> struct nfs_entry *entry,
> struct page **xdr_pages,
> - struct page *fillme, unsigned int buflen)
> + unsigned int buflen,
> + struct page **arrays,
> + size_t narrays)
> {
> struct address_space *mapping = desc->file->f_mapping;
> struct xdr_stream stream;
> struct xdr_buf buf;
> - struct page *scratch, *new, *page = fillme;
> + struct page *scratch, *new, *page = *arrays;
> int status;
>
> scratch = alloc_page(GFP_KERNEL);
> @@ -725,15 +727,22 @@ static int nfs_readdir_page_filler(struct
> nfs_readdir_descriptor *desc,
> if (status != -ENOSPC)
> continue;
>
> - if (page->mapping != mapping)
> - break;
> - new = nfs_readdir_page_get_next(mapping, page->index + 1,
> - entry->prev_cookie);
> - if (!new)
> - break;
> - if (page != fillme)
> - nfs_readdir_page_unlock_and_put(page);
> - page = new;
> + if (narrays > 1) {
> + narrays--;
> + arrays++;
> + page = *arrays;
> + } else {
> + if (page->mapping != mapping)
> + break;
> + new = nfs_readdir_page_get_next(mapping,
> + page->index + 1,
> + entry->prev_cookie);
> + if (!new)
> + break;
> + if (page != *arrays)
> + nfs_readdir_page_unlock_and_put(page);
> + page = new;
> + }
> status = nfs_readdir_add_to_array(entry, page);
> } while (!status && !entry->eof);
>
> @@ -750,7 +759,7 @@ static int nfs_readdir_page_filler(struct
> nfs_readdir_descriptor *desc,
> break;
> }
>
> - if (page != fillme)
> + if (page != *arrays)
> nfs_readdir_page_unlock_and_put(page);
>
> put_page(scratch);
> @@ -790,10 +799,11 @@ static struct page
> **nfs_readdir_alloc_pages(size_t npages)
> }
>
> static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor
> *desc,
> - struct page *page, __be32 *verf_arg,
> - __be32 *verf_res)
> + __be32 *verf_arg, __be32 *verf_res,
> + struct page **arrays, size_t narrays)
> {
> struct page **pages;
> + struct page *page = *arrays;
> struct nfs_entry *entry;
> size_t array_size;
> struct inode *inode = file_inode(desc->file);
> @@ -835,7 +845,8 @@ static int nfs_readdir_xdr_to_array(struct
> nfs_readdir_descriptor *desc,
> break;
> }
>
> - status = nfs_readdir_page_filler(desc, entry, pages, page, pglen);
> + status = nfs_readdir_page_filler(desc, entry, pages, pglen,
> + arrays, narrays);
> } while (!status && nfs_readdir_page_needs_filling(page));
>
> nfs_readdir_free_pages(pages, array_size);
> @@ -884,8 +895,8 @@ static int find_and_lock_cache_page(struct
> nfs_readdir_descriptor *desc)
> if (!desc->page)
> return -ENOMEM;
> if (nfs_readdir_page_needs_filling(desc->page)) {
> - res = nfs_readdir_xdr_to_array(desc, desc->page,
> - nfsi->cookieverf, verf);
> + res = nfs_readdir_xdr_to_array(desc, nfsi->cookieverf, verf,
> + &desc->page, 1);
> if (res < 0) {
> nfs_readdir_page_unlock_and_put_cached(desc);
> if (res == -EBADCOOKIE || res == -ENOTSYNC) {
> @@ -976,35 +987,37 @@ static void nfs_do_filldir(struct
> nfs_readdir_descriptor *desc)
> */
> static int uncached_readdir(struct nfs_readdir_descriptor *desc)
> {
> - struct page *page = NULL;
> + struct page **arrays;
> + size_t i, sz = 16;
> __be32 verf[NFS_DIR_VERIFIER_SIZE];
> int status;
>
> dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie
> %Lu\n",
> (unsigned long long)desc->dir_cookie);
>
> - page = alloc_page(GFP_HIGHUSER);
> - if (!page) {
> - status = -ENOMEM;
> - goto out;
> - }
> + arrays = nfs_readdir_alloc_pages(sz);
> + if (!arrays)
> + return -ENOMEM;
> + for (i = 0; i < sz; i++)
> + nfs_readdir_page_init_array(arrays[i], desc->dir_cookie);
>
> desc->page_index = 0;
> desc->last_cookie = desc->dir_cookie;
> - desc->page = page;
> desc->duped = 0;
>
> - nfs_readdir_page_init_array(page, desc->dir_cookie);
> - status = nfs_readdir_xdr_to_array(desc, page, desc->verf, verf);
> - if (status < 0)
> - goto out_release;
> + status = nfs_readdir_xdr_to_array(desc, desc->verf, verf, arrays,
> sz);
>
> - nfs_do_filldir(desc);
> + for (i = 0; !desc->eof && i < sz; i++) {
> + desc->page = arrays[i];
> + nfs_do_filldir(desc);
> + }
> + desc->page = NULL;
> +
> +
> + for (i = 0; i < sz; i++)
> + nfs_readdir_clear_array(arrays[i]);
> + nfs_readdir_free_pages(arrays, sz);
>
> - out_release:
> - nfs_readdir_clear_array(desc->page);
> - nfs_readdir_page_put(desc);
> - out:
> dfprintk(DIRCACHE, "NFS: %s: returns %d\n",
> __func__, status);
> return status;
> --
> 2.28.0