Hi Trond,
On Wed, Feb 23, 2022 at 8:11 PM <trondmy@xxxxxxxxxx> wrote:
>
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> The current NFS readdir code will always try to maximise the amount of
> readahead it performs on the assumption that we can cache anything that
> isn't immediately read by the process.
> There are several cases where this assumption breaks down, including
> when the 'ls -l' heuristic kicks in to try to force use of readdirplus
> as a batch replacement for lookup/getattr.
>
> This patch therefore tries to tone down the amount of readahead we
> perform, and adjust it to try to match the amount of data being
> requested by user space.
I'm seeing cthon basic tests fail at this patch, but I'm unsure if it
would have started now or in patches 6 or 7 due to the earlier compile
error. The other cthon tests still pass, however:
Thu Feb 24 11:27:44 EST 2022
./server -b -o tcp,v3,sec=sys -m /mnt/nfsv3tcp -p /srv/test/anna/nfsv3tcp server
./server -b -o proto=tcp,sec=sys,v4.0 -m /mnt/nfsv4tcp -p
/srv/test/anna/nfsv4tcp server
./server -b -o proto=tcp,sec=sys,v4.1 -m /mnt/nfsv41tcp -p
/srv/test/anna/nfsv41tcp server
./server -b -o proto=tcp,sec=sys,v4.2 -m /mnt/nfsv42tcp -p
/srv/test/anna/nfsv42tcp server
Waiting for 'b' to finish...
The '-b' test using '-o tcp,v3,sec=sys' args to server: Failed!!
The '-b' test using '-o proto=tcp,sec=sys,v4.0' args to server: Failed!!
The '-b' test using '-o proto=tcp,sec=sys,v4.2' args to server: Failed!!
The '-b' test using '-o proto=tcp,sec=sys,v4.1' args to server: Failed!!
Done: 11:27:46
Anna
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/dir.c | 55 +++++++++++++++++++++++++++++++++++++++++-
> include/linux/nfs_fs.h | 1 +
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 70c0db877815..83933b7018ea 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -69,6 +69,8 @@ const struct address_space_operations nfs_dir_aops = {
> .freepage = nfs_readdir_clear_array,
> };
>
> +#define NFS_INIT_DTSIZE PAGE_SIZE
> +
> static struct nfs_open_dir_context *
> alloc_nfs_open_dir_context(struct inode *dir)
> {
> @@ -78,6 +80,7 @@ alloc_nfs_open_dir_context(struct inode *dir)
> ctx = kzalloc(sizeof(*ctx), GFP_KERNEL_ACCOUNT);
> if (ctx != NULL) {
> ctx->attr_gencount = nfsi->attr_gencount;
> + ctx->dtsize = NFS_INIT_DTSIZE;
> spin_lock(&dir->i_lock);
> if (list_empty(&nfsi->open_files) &&
> (nfsi->cache_validity & NFS_INO_DATA_INVAL_DEFER))
> @@ -153,6 +156,7 @@ struct nfs_readdir_descriptor {
> struct page *page;
> struct dir_context *ctx;
> pgoff_t page_index;
> + pgoff_t page_index_max;
> u64 dir_cookie;
> u64 last_cookie;
> u64 dup_cookie;
> @@ -165,12 +169,36 @@ struct nfs_readdir_descriptor {
> unsigned long gencount;
> unsigned long attr_gencount;
> unsigned int cache_entry_index;
> + unsigned int buffer_fills;
> + unsigned int dtsize;
> signed char duped;
> bool plus;
> bool eob;
> bool eof;
> };
>
> +static void nfs_set_dtsize(struct nfs_readdir_descriptor *desc, unsigned int sz)
> +{
> + struct nfs_server *server = NFS_SERVER(file_inode(desc->file));
> + unsigned int maxsize = server->dtsize;
> +
> + if (sz > maxsize)
> + sz = maxsize;
> + if (sz < NFS_MIN_FILE_IO_SIZE)
> + sz = NFS_MIN_FILE_IO_SIZE;
> + desc->dtsize = sz;
> +}
> +
> +static void nfs_shrink_dtsize(struct nfs_readdir_descriptor *desc)
> +{
> + nfs_set_dtsize(desc, desc->dtsize >> 1);
> +}
> +
> +static void nfs_grow_dtsize(struct nfs_readdir_descriptor *desc)
> +{
> + nfs_set_dtsize(desc, desc->dtsize << 1);
> +}
> +
> static void nfs_readdir_array_init(struct nfs_cache_array *array)
> {
> memset(array, 0, sizeof(struct nfs_cache_array));
> @@ -774,6 +802,7 @@ static int nfs_readdir_page_filler(struct nfs_readdir_descriptor *desc,
> break;
> arrays++;
> *arrays = page = new;
> + desc->page_index_max++;
> } else {
> new = nfs_readdir_page_get_next(mapping,
> page->index + 1,
> @@ -783,6 +812,7 @@ static int nfs_readdir_page_filler(struct nfs_readdir_descriptor *desc,
> if (page != *arrays)
> nfs_readdir_page_unlock_and_put(page);
> page = new;
> + desc->page_index_max = new->index;
> }
> status = nfs_readdir_add_to_array(entry, page);
> } while (!status && !entry->eof);
> @@ -848,7 +878,7 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc,
> struct nfs_entry *entry;
> size_t array_size;
> struct inode *inode = file_inode(desc->file);
> - size_t dtsize = NFS_SERVER(inode)->dtsize;
> + unsigned int dtsize = desc->dtsize;
> int status = -ENOMEM;
>
> entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> @@ -884,6 +914,7 @@ static int nfs_readdir_xdr_to_array(struct nfs_readdir_descriptor *desc,
>
> status = nfs_readdir_page_filler(desc, entry, pages, pglen,
> arrays, narrays);
> + desc->buffer_fills++;
> } while (!status && nfs_readdir_page_needs_filling(page) &&
> page_mapping(page));
>
> @@ -931,6 +962,7 @@ 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)) {
> + desc->page_index_max = desc->page_index;
> res = nfs_readdir_xdr_to_array(desc, nfsi->cookieverf, verf,
> &desc->page, 1);
> if (res < 0) {
> @@ -1067,6 +1099,7 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc)
> desc->cache_entry_index = 0;
> desc->last_cookie = desc->dir_cookie;
> desc->duped = 0;
> + desc->page_index_max = 0;
>
> status = nfs_readdir_xdr_to_array(desc, desc->verf, verf, arrays, sz);
>
> @@ -1076,10 +1109,22 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc)
> }
> desc->page = NULL;
>
> + /*
> + * Grow the dtsize if we have to go back for more pages,
> + * or shrink it if we're reading too many.
> + */
> + if (!desc->eof) {
> + if (!desc->eob)
> + nfs_grow_dtsize(desc);
> + else if (desc->buffer_fills == 1 &&
> + i < (desc->page_index_max >> 1))
> + nfs_shrink_dtsize(desc);
> + }
>
> for (i = 0; i < sz && arrays[i]; i++)
> nfs_readdir_page_array_free(arrays[i]);
> out:
> + desc->page_index_max = -1;
> kfree(arrays);
> dfprintk(DIRCACHE, "NFS: %s: returns %d\n", __func__, status);
> return status;
> @@ -1118,6 +1163,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
> desc->file = file;
> desc->ctx = ctx;
> desc->plus = nfs_use_readdirplus(inode, ctx);
> + desc->page_index_max = -1;
>
> spin_lock(&file->f_lock);
> desc->dir_cookie = dir_ctx->dir_cookie;
> @@ -1128,6 +1174,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
> desc->last_cookie = dir_ctx->last_cookie;
> desc->attr_gencount = dir_ctx->attr_gencount;
> desc->eof = dir_ctx->eof;
> + nfs_set_dtsize(desc, dir_ctx->dtsize);
> memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf));
> spin_unlock(&file->f_lock);
>
> @@ -1169,6 +1216,11 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
>
> nfs_do_filldir(desc, nfsi->cookieverf);
> nfs_readdir_page_unlock_and_put_cached(desc);
> + if (desc->eob || desc->eof)
> + break;
> + /* Grow the dtsize if we have to go back for more pages */
> + if (desc->page_index == desc->page_index_max)
> + nfs_grow_dtsize(desc);
> } while (!desc->eob && !desc->eof);
>
> spin_lock(&file->f_lock);
> @@ -1179,6 +1231,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
> dir_ctx->attr_gencount = desc->attr_gencount;
> dir_ctx->page_index = desc->page_index;
> dir_ctx->eof = desc->eof;
> + dir_ctx->dtsize = desc->dtsize;
> memcpy(dir_ctx->verf, desc->verf, sizeof(dir_ctx->verf));
> spin_unlock(&file->f_lock);
> out_free:
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 1c533f2c1f36..691a27936849 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -107,6 +107,7 @@ struct nfs_open_dir_context {
> __u64 dup_cookie;
> __u64 last_cookie;
> pgoff_t page_index;
> + unsigned int dtsize;
> signed char duped;
> bool eof;
> };
> --
> 2.35.1
>
