Linux NFS development
 help / color / mirror / Atom feed
From: trondmy@kernel.org
To: linux-nfs@vger.kernel.org
Subject: [PATCH v7 19/21] NFS: Convert readdir page cache to use a cookie based index
Date: Wed, 23 Feb 2022 16:13:03 -0500	[thread overview]
Message-ID: <20220223211305.296816-20-trondmy@kernel.org> (raw)
In-Reply-To: <20220223211305.296816-19-trondmy@kernel.org>

From: Trond Myklebust <trond.myklebust@hammerspace.com>

Instead of using a linear index to address the pages, use the cookie of
the first entry, since that is what we use to match the page anyway.

This allows us to avoid re-reading the entire cache on a seekdir() type
of operation. The latter is very common when re-exporting NFS, and is a
major performance drain.

The change does affect our duplicate cookie detection, since we can no
longer rely on the page index as a linear offset for detecting whether
we looped backwards. However since we no longer do a linear search
through all the pages on each call to nfs_readdir(), this is less of a
concern than it was previously.
The other downside is that invalidate_mapping_pages() no longer can use
the page index to avoid clearing pages that have been read. A subsequent
patch will restore the functionality this provides to the 'ls -l'
heuristic.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/dir.c           | 99 +++++++++++++++---------------------------
 include/linux/nfs_fs.h |  2 -
 2 files changed, 34 insertions(+), 67 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 06bd612296d5..2007eebfb5cf 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -39,6 +39,7 @@
 #include <linux/sched.h>
 #include <linux/kmemleak.h>
 #include <linux/xattr.h>
+#include <linux/xxhash.h>
 
 #include "delegation.h"
 #include "iostat.h"
@@ -158,9 +159,7 @@ struct nfs_readdir_descriptor {
 	pgoff_t		page_index_max;
 	u64		dir_cookie;
 	u64		last_cookie;
-	u64		dup_cookie;
 	loff_t		current_index;
-	loff_t		prev_index;
 
 	__be32		verf[NFS_DIR_VERIFIER_SIZE];
 	unsigned long	dir_verifier;
@@ -170,7 +169,6 @@ struct nfs_readdir_descriptor {
 	unsigned int	cache_entry_index;
 	unsigned int	buffer_fills;
 	unsigned int	dtsize;
-	signed char duped;
 	bool plus;
 	bool eob;
 	bool eof;
@@ -333,6 +331,13 @@ int nfs_readdir_add_to_array(struct nfs_entry *entry, struct page *page)
 	return ret;
 }
 
+static pgoff_t nfs_readdir_page_cookie_hash(u64 cookie)
+{
+	if (cookie == 0)
+		return 0;
+	return xxhash(&cookie, sizeof(cookie), 0);
+}
+
 static bool nfs_readdir_page_cookie_match(struct page *page, u64 last_cookie,
 					  u64 change_attr)
 {
@@ -354,8 +359,9 @@ static void nfs_readdir_page_unlock_and_put(struct page *page)
 }
 
 static struct page *nfs_readdir_page_get_locked(struct address_space *mapping,
-						pgoff_t index, u64 last_cookie)
+						u64 last_cookie)
 {
+	pgoff_t index = nfs_readdir_page_cookie_hash(last_cookie);
 	struct page *page;
 	u64 change_attr;
 
@@ -374,11 +380,6 @@ static struct page *nfs_readdir_page_get_locked(struct address_space *mapping,
 	return page;
 }
 
-static loff_t nfs_readdir_page_offset(struct page *page)
-{
-	return (loff_t)page->index * (loff_t)nfs_readdir_array_maxentries();
-}
-
 static u64 nfs_readdir_page_last_cookie(struct page *page)
 {
 	struct nfs_cache_array *array;
@@ -411,11 +412,11 @@ static void nfs_readdir_page_set_eof(struct page *page)
 }
 
 static struct page *nfs_readdir_page_get_next(struct address_space *mapping,
-					      pgoff_t index, u64 cookie)
+					      u64 cookie)
 {
 	struct page *page;
 
-	page = nfs_readdir_page_get_locked(mapping, index, cookie);
+	page = nfs_readdir_page_get_locked(mapping, cookie);
 	if (page) {
 		if (nfs_readdir_page_last_cookie(page) == cookie)
 			return page;
@@ -443,6 +444,13 @@ bool nfs_readdir_use_cookie(const struct file *filp)
 	return true;
 }
 
+static void nfs_readdir_rewind_search(struct nfs_readdir_descriptor *desc)
+{
+	desc->current_index = 0;
+	desc->last_cookie = 0;
+	desc->page_index = 0;
+}
+
 static int nfs_readdir_search_for_pos(struct nfs_cache_array *array,
 				      struct nfs_readdir_descriptor *desc)
 {
@@ -491,32 +499,11 @@ static int nfs_readdir_search_for_cookie(struct nfs_cache_array *array,
 
 	for (i = 0; i < array->size; i++) {
 		if (array->array[i].cookie == desc->dir_cookie) {
-			struct nfs_inode *nfsi = NFS_I(file_inode(desc->file));
-
-			new_pos = nfs_readdir_page_offset(desc->page) + i;
-			if (desc->attr_gencount != nfsi->attr_gencount) {
-				desc->duped = 0;
-				desc->attr_gencount = nfsi->attr_gencount;
-			} else if (new_pos < desc->prev_index) {
-				if (desc->duped > 0
-				    && desc->dup_cookie == desc->dir_cookie) {
-					if (printk_ratelimit()) {
-						pr_notice("NFS: directory %pD2 contains a readdir loop."
-								"Please contact your server vendor.  "
-								"The file: %s has duplicate cookie %llu\n",
-								desc->file, array->array[i].name, desc->dir_cookie);
-					}
-					status = -ELOOP;
-					goto out;
-				}
-				desc->dup_cookie = desc->dir_cookie;
-				desc->duped = -1;
-			}
+			new_pos = desc->current_index + i;
 			if (nfs_readdir_use_cookie(desc->file))
 				desc->ctx->pos = desc->dir_cookie;
 			else
 				desc->ctx->pos = new_pos;
-			desc->prev_index = new_pos;
 			desc->cache_entry_index = i;
 			return 0;
 		}
@@ -527,7 +514,6 @@ static int nfs_readdir_search_for_cookie(struct nfs_cache_array *array,
 		if (desc->dir_cookie == array->last_cookie)
 			desc->eof = true;
 	}
-out:
 	return status;
 }
 
@@ -820,18 +806,16 @@ 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,
 							entry->prev_cookie);
 			if (!new)
 				break;
 			if (page != *arrays)
 				nfs_readdir_page_unlock_and_put(page);
 			page = new;
-			desc->page_index_max = new->index;
 		}
+		desc->page_index_max++;
 		status = nfs_readdir_add_to_array(entry, page);
 	} while (!status && !entry->eof);
 
@@ -954,7 +938,6 @@ static struct page *
 nfs_readdir_page_get_cached(struct nfs_readdir_descriptor *desc)
 {
 	return nfs_readdir_page_get_locked(desc->file->f_mapping,
-					   desc->page_index,
 					   desc->last_cookie);
 }
 
@@ -984,7 +967,7 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc)
 			trace_nfs_readdir_cache_fill_done(inode, res);
 			if (res == -EBADCOOKIE || res == -ENOTSYNC) {
 				invalidate_inode_pages2(desc->file->f_mapping);
-				desc->page_index = 0;
+				nfs_readdir_rewind_search(desc);
 				trace_nfs_readdir_invalidate_cache_range(
 					inode, 0, MAX_LFS_FILESIZE);
 				return -EAGAIN;
@@ -998,12 +981,10 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc)
 		    memcmp(nfsi->cookieverf, verf, sizeof(nfsi->cookieverf))) {
 			memcpy(nfsi->cookieverf, verf,
 			       sizeof(nfsi->cookieverf));
-			invalidate_inode_pages2_range(desc->file->f_mapping,
-						      desc->page_index_max + 1,
+			invalidate_inode_pages2_range(desc->file->f_mapping, 1,
 						      -1);
 			trace_nfs_readdir_invalidate_cache_range(
-				inode, desc->page_index_max + 1,
-				MAX_LFS_FILESIZE);
+				inode, 1, MAX_LFS_FILESIZE);
 		}
 	}
 	res = nfs_readdir_search_array(desc);
@@ -1019,11 +1000,6 @@ static int readdir_search_pagecache(struct nfs_readdir_descriptor *desc)
 	int res;
 
 	do {
-		if (desc->page_index == 0) {
-			desc->current_index = 0;
-			desc->prev_index = 0;
-			desc->last_cookie = 0;
-		}
 		res = find_and_lock_cache_page(desc);
 	} while (res == -EAGAIN);
 	return res;
@@ -1058,8 +1034,6 @@ static void nfs_do_filldir(struct nfs_readdir_descriptor *desc,
 			desc->ctx->pos = desc->dir_cookie;
 		else
 			desc->ctx->pos++;
-		if (desc->duped != 0)
-			desc->duped = 1;
 	}
 	if (array->page_is_eof)
 		desc->eof = !desc->eob;
@@ -1101,7 +1075,6 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc)
 	desc->page_index = 0;
 	desc->cache_entry_index = 0;
 	desc->last_cookie = desc->dir_cookie;
-	desc->duped = 0;
 	desc->page_index_max = 0;
 
 	trace_nfs_readdir_uncached(desc->file, desc->verf, desc->last_cookie,
@@ -1134,6 +1107,8 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc)
 	for (i = 0; i < sz && arrays[i]; i++)
 		nfs_readdir_page_array_free(arrays[i]);
 out:
+	if (!nfs_readdir_use_cookie(desc->file))
+		nfs_readdir_rewind_search(desc);
 	desc->page_index_max = -1;
 	kfree(arrays);
 	dfprintk(DIRCACHE, "NFS: %s: returns %d\n", __func__, status);
@@ -1144,17 +1119,14 @@ static int uncached_readdir(struct nfs_readdir_descriptor *desc)
 
 static void nfs_readdir_handle_cache_misses(struct inode *inode,
 					    struct nfs_readdir_descriptor *desc,
-					    pgoff_t page_index,
 					    unsigned int cache_misses)
 {
 	if (desc->ctx->pos == 0 ||
 	    cache_misses <= NFS_READDIR_CACHE_MISS_THRESHOLD)
 		return;
-	if (invalidate_mapping_pages(inode->i_mapping, page_index + 1, -1) == 0)
+	if (invalidate_mapping_pages(inode->i_mapping, 0, -1) == 0)
 		return;
-	trace_nfs_readdir_invalidate_cache_range(
-		inode, (loff_t)(page_index + 1) << PAGE_SHIFT,
-		MAX_LFS_FILESIZE);
+	trace_nfs_readdir_invalidate_cache_range(inode, 0, MAX_LFS_FILESIZE);
 }
 
 /* The file offset position represents the dirent entry number.  A
@@ -1194,8 +1166,6 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 
 	spin_lock(&file->f_lock);
 	desc->dir_cookie = dir_ctx->dir_cookie;
-	desc->dup_cookie = dir_ctx->dup_cookie;
-	desc->duped = dir_ctx->duped;
 	page_index = dir_ctx->page_index;
 	desc->page_index = page_index;
 	desc->last_cookie = dir_ctx->last_cookie;
@@ -1213,7 +1183,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 	}
 
 	desc->plus = nfs_use_readdirplus(inode, ctx, cache_hits, cache_misses);
-	nfs_readdir_handle_cache_misses(inode, desc, page_index, cache_misses);
+	nfs_readdir_handle_cache_misses(inode, desc, cache_misses);
 
 	do {
 		res = readdir_search_pagecache(desc);
@@ -1233,7 +1203,6 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 		}
 		if (res == -ETOOSMALL && desc->plus) {
 			nfs_zap_caches(inode);
-			desc->page_index = 0;
 			desc->plus = false;
 			desc->eof = false;
 			continue;
@@ -1252,9 +1221,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 
 	spin_lock(&file->f_lock);
 	dir_ctx->dir_cookie = desc->dir_cookie;
-	dir_ctx->dup_cookie = desc->dup_cookie;
 	dir_ctx->last_cookie = desc->last_cookie;
-	dir_ctx->duped = desc->duped;
 	dir_ctx->attr_gencount = desc->attr_gencount;
 	dir_ctx->page_index = desc->page_index;
 	dir_ctx->eof = desc->eof;
@@ -1297,13 +1264,15 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence)
 	if (offset != filp->f_pos) {
 		filp->f_pos = offset;
 		dir_ctx->page_index = 0;
-		if (!nfs_readdir_use_cookie(filp))
+		if (!nfs_readdir_use_cookie(filp)) {
 			dir_ctx->dir_cookie = 0;
-		else
+			dir_ctx->last_cookie = 0;
+		} else {
 			dir_ctx->dir_cookie = offset;
+			dir_ctx->last_cookie = offset;
+		}
 		if (offset == 0)
 			memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf));
-		dir_ctx->duped = 0;
 		dir_ctx->eof = false;
 	}
 	spin_unlock(&filp->f_lock);
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 20a4cf0acad2..42aad886d3c0 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -106,11 +106,9 @@ struct nfs_open_dir_context {
 	unsigned long attr_gencount;
 	__be32	verf[NFS_DIR_VERIFIER_SIZE];
 	__u64 dir_cookie;
-	__u64 dup_cookie;
 	__u64 last_cookie;
 	pgoff_t page_index;
 	unsigned int dtsize;
-	signed char duped;
 	bool eof;
 	struct rcu_head rcu_head;
 };
-- 
2.35.1


  reply	other threads:[~2022-02-23 21:21 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-23 21:12 [PATCH v7 00/21] Readdir improvements trondmy
2022-02-23 21:12 ` [PATCH v7 01/21] NFS: constify nfs_server_capable() and nfs_have_writebacks() trondmy
2022-02-23 21:12   ` [PATCH v7 02/21] NFS: Trace lookup revalidation failure trondmy
2022-02-23 21:12     ` [PATCH v7 03/21] NFS: Use kzalloc() to avoid initialising the nfs_open_dir_context trondmy
2022-02-23 21:12       ` [PATCH v7 04/21] NFS: Calculate page offsets algorithmically trondmy
2022-02-23 21:12         ` [PATCH v7 05/21] NFS: Store the change attribute in the directory page cache trondmy
2022-02-23 21:12           ` [PATCH v7 06/21] NFS: If the cookie verifier changes, we must invalidate the " trondmy
2022-02-23 21:12             ` [PATCH v7 07/21] NFS: Don't re-read the entire page cache to find the next cookie trondmy
2022-02-23 21:12               ` [PATCH v7 08/21] NFS: Adjust the amount of readahead performed by NFS readdir trondmy
2022-02-23 21:12                 ` [PATCH v7 09/21] NFS: Simplify nfs_readdir_xdr_to_array() trondmy
2022-02-23 21:12                   ` [PATCH v7 10/21] NFS: Reduce use of uncached readdir trondmy
2022-02-23 21:12                     ` [PATCH v7 11/21] NFS: Improve heuristic for readdirplus trondmy
2022-02-23 21:12                       ` [PATCH v7 12/21] NFS: Don't ask for readdirplus unless it can help nfs_getattr() trondmy
2022-02-23 21:12                         ` [PATCH v7 13/21] NFSv4: Ask for a full XDR buffer of readdir goodness trondmy
2022-02-23 21:12                           ` [PATCH v7 14/21] NFS: Readdirplus can't help lookup for case insensitive filesystems trondmy
2022-02-23 21:12                             ` [PATCH v7 15/21] NFS: Don't request readdirplus when revalidation was forced trondmy
2022-02-23 21:13                               ` [PATCH v7 16/21] NFS: Add basic readdir tracing trondmy
2022-02-23 21:13                                 ` [PATCH v7 17/21] NFS: Trace effects of readdirplus on the dcache trondmy
2022-02-23 21:13                                   ` [PATCH v7 18/21] NFS: Trace effects of the readdirplus heuristic trondmy
2022-02-23 21:13                                     ` trondmy [this message]
2022-02-23 21:13                                       ` [PATCH v7 20/21] NFS: Fix up forced readdirplus trondmy
2022-02-23 21:13                                         ` [PATCH v7 21/21] NFS: Remove unnecessary cache invalidations for directories trondmy
2022-02-24 17:31                                       ` [PATCH v7 19/21] NFS: Convert readdir page cache to use a cookie based index Benjamin Coddington
2022-02-25  2:33                                         ` Trond Myklebust
2022-02-25  3:17                                           ` NeilBrown
2022-02-25  4:25                                             ` Trond Myklebust
2022-02-25 12:33                                               ` Benjamin Coddington
2022-02-25 13:11                                                 ` Trond Myklebust
2022-02-24 15:53                                 ` [PATCH v7 16/21] NFS: Add basic readdir tracing Benjamin Coddington
2022-02-25  2:35                                   ` Trond Myklebust
2022-02-24 16:55                     ` [PATCH v7 10/21] NFS: Reduce use of uncached readdir Anna Schumaker
2022-02-25  4:07                       ` Trond Myklebust
2022-02-24 16:30                 ` [PATCH v7 08/21] NFS: Adjust the amount of readahead performed by NFS readdir Anna Schumaker
2022-02-24 16:18             ` [PATCH v7 06/21] NFS: If the cookie verifier changes, we must invalidate the page cache Anna Schumaker
2022-02-24 14:53           ` [PATCH v7 05/21] NFS: Store the change attribute in the directory " Benjamin Coddington
2022-02-25  2:26             ` Trond Myklebust
2022-02-25  3:51               ` Trond Myklebust
2022-02-25 11:38                 ` Benjamin Coddington
2022-02-25 13:10                   ` Trond Myklebust
2022-02-25 13:26                     ` Trond Myklebust
2022-02-25 14:44                     ` Benjamin Coddington
2022-02-25 15:18                       ` Trond Myklebust
2022-02-25 15:34                         ` Benjamin Coddington
2022-02-25 20:23                           ` Benjamin Coddington
2022-02-25 20:28                             ` Benjamin Coddington
2022-02-25 20:41                             ` Trond Myklebust
2022-02-25 22:04                               ` Benjamin Coddington
2022-02-25 22:29                               ` Trond Myklebust
2022-02-24 14:15         ` [PATCH v7 04/21] NFS: Calculate page offsets algorithmically Benjamin Coddington
2022-02-25  2:11           ` Trond Myklebust
2022-02-25 11:28             ` Benjamin Coddington
2022-02-25 12:44               ` Trond Myklebust
2022-02-24 14:14     ` [PATCH v7 02/21] NFS: Trace lookup revalidation failure Benjamin Coddington
2022-02-25  2:09       ` Trond Myklebust
2022-02-24 12:25 ` [PATCH v7 00/21] Readdir improvements David Wysochanski
2022-02-25  4:00   ` Trond Myklebust
2022-02-24 15:07 ` David Wysochanski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220223211305.296816-20-trondmy@kernel.org \
    --to=trondmy@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox