diff mbox series

NFS: Trigger the "ls -l" readdir heuristic sooner

Message ID 20220920170021.1391560-1-bcodding@redhat.com (mailing list archive)
State New, archived
Headers show
Series NFS: Trigger the "ls -l" readdir heuristic sooner | expand

Commit Message

Benjamin Coddington Sept. 20, 2022, 5 p.m. UTC
Since commit 1a34c8c9a49e ("NFS: Support larger readdir buffers") has
updated dtsize, and with recent improvements to the READDIRPLUS helper
heuristic, the heuristic may not trigger until many dentries are emitted
to userspace.   This will cause many thousands of GETATTR calls for "ls
-l" when the directory's pagecache has already been populated.  This
manifests as poor performance for long directory listings after an
initially fast "ls -l".

Fix this by emitting only 17 entries for any first pass through the NFS
directory's ->iterate_shared(), which allows userpace to prime the
counters for the heuristic.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/dir.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Benjamin Coddington Oct. 21, 2022, 12:43 p.m. UTC | #1
On 20 Sep 2022, at 13:00, Benjamin Coddington wrote:

> Since commit 1a34c8c9a49e ("NFS: Support larger readdir buffers") has
> updated dtsize, and with recent improvements to the READDIRPLUS helper
> heuristic, the heuristic may not trigger until many dentries are emitted
> to userspace.   This will cause many thousands of GETATTR calls for "ls
> -l" when the directory's pagecache has already been populated.  This
> manifests as poor performance for long directory listings after an
> initially fast "ls -l".
>
> Fix this by emitting only 17 entries for any first pass through the NFS
> directory's ->iterate_shared(), which allows userpace to prime the
> counters for the heuristic.
>
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---

Hello Trond and Anna, please consider this patch.  We have folks that are
sad and confused that "ls -l" becomes much slower after running it once.

Olga also tested this fix and reported performance gains here:
https://lore.kernel.org/linux-nfs/CAN-5tyFsJy-D9cEfk8zCH8-0KErWteVW8Zb7BMqjL7==2qtT_A@mail.gmail.com/

Olga, would you be willing to contribute your "Tested-by" for this patch?

Any comments from anyone would also be welcome.

Ben
diff mbox series

Patch

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5d6c2ddc7ea6..3b24b8af5514 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1074,6 +1074,8 @@  static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc)
 	return res;
 }
 
+#define NFS_READDIR_CACHE_MISS_THRESHOLD (16UL)
+
 /*
  * Once we've found the start of the dirent within a page: fill 'er up...
  */
@@ -1083,6 +1085,7 @@  static void nfs_do_filldir(struct nfs_readdir_descriptor *desc,
 	struct file	*file = desc->file;
 	struct nfs_cache_array *array;
 	unsigned int i;
+	bool first_emit = !desc->dir_cookie;
 
 	array = kmap_local_page(desc->page);
 	for (i = desc->cache_entry_index; i < array->size; i++) {
@@ -1106,6 +1109,10 @@  static void nfs_do_filldir(struct nfs_readdir_descriptor *desc,
 			desc->ctx->pos = desc->dir_cookie;
 		else
 			desc->ctx->pos++;
+		if (first_emit && i > NFS_READDIR_CACHE_MISS_THRESHOLD + 1) {
+			desc->eob = true;
+			break;
+		}
 	}
 	if (array->page_is_eof)
 		desc->eof = !desc->eob;
@@ -1187,8 +1194,6 @@  static int uncached_readdir(struct nfs_readdir_descriptor *desc)
 	return status;
 }
 
-#define NFS_READDIR_CACHE_MISS_THRESHOLD (16UL)
-
 static bool nfs_readdir_handle_cache_misses(struct inode *inode,
 					    struct nfs_readdir_descriptor *desc,
 					    unsigned int cache_misses,