2022-02-24 00:48:09

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v7 06/21] NFS: If the cookie verifier changes, we must invalidate the page cache

From: Trond Myklebust <[email protected]>

Ensure that if the cookie verifier changes when we use the zero-valued
cookie, then we invalidate any cached pages.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5d9367d9b651..7932d474ce00 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -945,9 +945,14 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc)
/*
* Set the cookie verifier if the page cache was empty
*/
- if (desc->page_index == 0)
+ if (desc->last_cookie == 0 &&
+ 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,
+ -1);
+ }
}
res = nfs_readdir_search_array(desc);
if (res == 0)
--
2.35.1


2022-02-24 01:39:06

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v7 07/21] NFS: Don't re-read the entire page cache to find the next cookie

From: Trond Myklebust <[email protected]>

If the page cache entry that was last read gets invalidated for some
reason, then make sure we can re-create it on the next call to readdir.
This, combined with the cache page validation, allows us to reuse the
cached value of page-index on successive calls to nfs_readdir.

Credit is due to Benjamin Coddington for showing that the concept works,
and that it allows for improved cache sharing between processes even in
the case where pages are lost due to LRU or active invalidation.

Suggested-by: Benjamin Coddington <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 10 +++++++---
include/linux/nfs_fs.h | 1 +
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 7932d474ce00..70c0db877815 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1124,6 +1124,8 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
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;
desc->attr_gencount = dir_ctx->attr_gencount;
desc->eof = dir_ctx->eof;
memcpy(desc->verf, dir_ctx->verf, sizeof(desc->verf));
@@ -1172,6 +1174,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;
@@ -1213,10 +1216,11 @@ static loff_t nfs_llseek_dir(struct file *filp, loff_t offset, int whence)
}
if (offset != filp->f_pos) {
filp->f_pos = offset;
- if (nfs_readdir_use_cookie(filp))
- dir_ctx->dir_cookie = offset;
- else
+ if (!nfs_readdir_use_cookie(filp)) {
dir_ctx->dir_cookie = 0;
+ dir_ctx->page_index = 0;
+ } else
+ dir_ctx->dir_cookie = offset;
if (offset == 0)
memset(dir_ctx->verf, 0, sizeof(dir_ctx->verf));
dir_ctx->duped = 0;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 6e10725887d1..1c533f2c1f36 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -105,6 +105,7 @@ struct nfs_open_dir_context {
__be32 verf[NFS_DIR_VERIFIER_SIZE];
__u64 dir_cookie;
__u64 dup_cookie;
+ __u64 last_cookie;
pgoff_t page_index;
signed char duped;
bool eof;
--
2.35.1

2022-02-24 16:58:28

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH v7 06/21] NFS: If the cookie verifier changes, we must invalidate the page cache

Hi Trond,

On Wed, Feb 23, 2022 at 7:48 PM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> Ensure that if the cookie verifier changes when we use the zero-valued
> cookie, then we invalidate any cached pages.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/dir.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 5d9367d9b651..7932d474ce00 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -945,9 +945,14 @@ static int find_and_lock_cache_page(struct nfs_readdir_descriptor *desc)
> /*
> * Set the cookie verifier if the page cache was empty
> */
> - if (desc->page_index == 0)
> + if (desc->last_cookie == 0 &&
> + 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,

I'm getting this when I try to compile this patch:

fs/nfs/dir.c: In function ‘find_and_lock_cache_page’:
fs/nfs/dir.c:953:61: error: ‘struct nfs_readdir_descriptor’ has no
member named ‘page_index_max’; did you mean ‘page_index’?
953 |
desc->page_index_max + 1,
|
^~~~~~~~~~~~~~
| page_index
make[2]: *** [scripts/Makefile.build:288: fs/nfs/dir.o] Error 1
make[1]: *** [scripts/Makefile.build:550: fs/nfs] Error 2
make: *** [Makefile:1831: fs] Error 2
make: *** Waiting for unfinished jobs....

It looks like the "page_index_max" field is added in patch 8.

Anna


Anna
> + -1);
> + }
> }
> res = nfs_readdir_search_array(desc);
> if (res == 0)
> --
> 2.35.1
>