2022-02-26 01:27:30

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v8 00/24] Readdir improvements

From: Trond Myklebust <[email protected]>

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 series also implement Ben's page cache filter to ensure that we can
improve the ability to share cached data between processes that are
reading the same directory at the same time, and to avoid live-locks
when the directory is simultaneously changing.

--
v2: Remove reset of dtsize when NFS_INO_FORCE_READDIR is set
v3: Avoid excessive window shrinking in uncached_readdir case
v4: Track 'ls -l' cache hit/miss statistics
Improved algorithm for falling back to uncached readdir
Skip readdirplus when files are being written to
v5: bugfixes
Skip readdirplus when the acdirmax/acregmax values are low
Request a full XDR buffer when doing READDIRPLUS
v6: Add tracing
Don't have lookup request readdirplus when it won't help
v7: Implement Ben's page cache filter
Reduce the use of uncached readdir
Change indexing of the page cache to improve seekdir() performance.
v8: Reduce the page cache overhead by shrinking the cookie hashvalue size
Incorporate other feedback from Anna, Ben and Neil
Fix nfs2/3_decode_dirent return values
Fix the change attribute value set in nfs_readdir_page_get_next()


Trond Myklebust (24):
NFS: Return valid errors from nfs2/3_decode_dirent()
NFS: constify nfs_server_capable() and nfs_have_writebacks()
NFS: Trace lookup revalidation failure
NFS: Initialise the readdir verifier as best we can in nfs_opendir()
NFS: Use kzalloc() to avoid initialising the nfs_open_dir_context
NFS: Calculate page offsets algorithmically
NFS: Store the change attribute in the directory page cache
NFS: If the cookie verifier changes, we must invalidate the page cache
NFS: Don't re-read the entire page cache to find the next cookie
NFS: Adjust the amount of readahead performed by NFS readdir
NFS: Simplify nfs_readdir_xdr_to_array()
NFS: Reduce use of uncached readdir
NFS: Improve heuristic for readdirplus
NFS: Don't ask for readdirplus unless it can help nfs_getattr()
NFSv4: Ask for a full XDR buffer of readdir goodness
NFS: Readdirplus can't help lookup for case insensitive filesystems
NFS: Don't request readdirplus when revalidation was forced
NFS: Add basic readdir tracing
NFS: Trace effects of readdirplus on the dcache
NFS: Trace effects of the readdirplus heuristic
NFS: Convert readdir page cache to use a cookie based index
NFS: Fix up forced readdirplus
NFS: Remove unnecessary cache invalidations for directories
NFS: Cache all entries in the readdirplus reply

fs/nfs/Kconfig | 4 +
fs/nfs/dir.c | 513 ++++++++++++++++++++++++-----------------
fs/nfs/inode.c | 46 ++--
fs/nfs/internal.h | 4 +-
fs/nfs/nfs2xdr.c | 2 +-
fs/nfs/nfs3xdr.c | 28 +--
fs/nfs/nfs4proc.c | 2 -
fs/nfs/nfs4xdr.c | 6 +-
fs/nfs/nfstrace.h | 123 +++++++++-
include/linux/nfs_fs.h | 19 +-
10 files changed, 479 insertions(+), 268 deletions(-)

--
2.35.1


2022-02-26 01:40:37

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v8 01/24] NFS: Return valid errors from nfs2/3_decode_dirent()

From: Trond Myklebust <[email protected]>

Valid return values for decode_dirent() callback functions are:
0: Success
-EBADCOOKIE: End of directory
-EAGAIN: End of xdr_stream

All errors need to map into one of those three values.

Fixes: 573c4e1ef53a ("NFS: Simplify ->decode_dirent() calling sequence")
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs2xdr.c | 2 +-
fs/nfs/nfs3xdr.c | 21 ++++++---------------
2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 7fba7711e6b3..3d5ba43f44bb 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -949,7 +949,7 @@ int nfs2_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,

error = decode_filename_inline(xdr, &entry->name, &entry->len);
if (unlikely(error))
- return error;
+ return -EAGAIN;

/*
* The type (size and byte order) of nfscookie isn't defined in
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 54a1d21cbcc6..7ab60ad98776 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -1967,7 +1967,6 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
bool plus)
{
struct user_namespace *userns = rpc_userns(entry->server->client);
- struct nfs_entry old = *entry;
__be32 *p;
int error;
u64 new_cookie;
@@ -1987,15 +1986,15 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,

error = decode_fileid3(xdr, &entry->ino);
if (unlikely(error))
- return error;
+ return -EAGAIN;

error = decode_inline_filename3(xdr, &entry->name, &entry->len);
if (unlikely(error))
- return error;
+ return -EAGAIN;

error = decode_cookie3(xdr, &new_cookie);
if (unlikely(error))
- return error;
+ return -EAGAIN;

entry->d_type = DT_UNKNOWN;

@@ -2003,7 +2002,7 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
entry->fattr->valid = 0;
error = decode_post_op_attr(xdr, entry->fattr, userns);
if (unlikely(error))
- return error;
+ return -EAGAIN;
if (entry->fattr->valid & NFS_ATTR_FATTR_V3)
entry->d_type = nfs_umode_to_dtype(entry->fattr->mode);

@@ -2018,11 +2017,8 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
return -EAGAIN;
if (*p != xdr_zero) {
error = decode_nfs_fh3(xdr, entry->fh);
- if (unlikely(error)) {
- if (error == -E2BIG)
- goto out_truncated;
- return error;
- }
+ if (unlikely(error))
+ return -EAGAIN;
} else
zero_nfs_fh3(entry->fh);
}
@@ -2031,11 +2027,6 @@ int nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
entry->cookie = new_cookie;

return 0;
-
-out_truncated:
- dprintk("NFS: directory entry contains invalid file handle\n");
- *entry = old;
- return -EAGAIN;
}

/*
--
2.35.1

2022-02-26 02:05:53

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v8 02/24] NFS: constify nfs_server_capable() and nfs_have_writebacks()

From: Trond Myklebust <[email protected]>

Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/nfs_fs.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 72a732a5103c..6e10725887d1 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -363,7 +363,7 @@ static inline void nfs_mark_for_revalidate(struct inode *inode)
spin_unlock(&inode->i_lock);
}

-static inline int nfs_server_capable(struct inode *inode, int cap)
+static inline int nfs_server_capable(const struct inode *inode, int cap)
{
return NFS_SERVER(inode)->caps & cap;
}
@@ -587,12 +587,11 @@ extern struct nfs_commit_data *nfs_commitdata_alloc(bool never_fail);
extern void nfs_commit_free(struct nfs_commit_data *data);
bool nfs_commit_end(struct nfs_mds_commit_info *cinfo);

-static inline int
-nfs_have_writebacks(struct inode *inode)
+static inline bool nfs_have_writebacks(const struct inode *inode)
{
if (S_ISREG(inode->i_mode))
return atomic_long_read(&NFS_I(inode)->nrequests) != 0;
- return 0;
+ return false;
}

/*
--
2.35.1

2022-02-26 02:25:54

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v8 03/24] NFS: Trace lookup revalidation failure

From: Trond Myklebust <[email protected]>

Enable tracing of lookup revalidation failures.

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

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ebddc736eac2..1aa55cac9d9a 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1474,9 +1474,7 @@ nfs_lookup_revalidate_done(struct inode *dir, struct dentry *dentry,
{
switch (error) {
case 1:
- dfprintk(LOOKUPCACHE, "NFS: %s(%pd2) is valid\n",
- __func__, dentry);
- return 1;
+ break;
case 0:
/*
* We can't d_drop the root of a disconnected tree:
@@ -1485,13 +1483,10 @@ nfs_lookup_revalidate_done(struct inode *dir, struct dentry *dentry,
* inodes on unmount and further oopses.
*/
if (inode && IS_ROOT(dentry))
- return 1;
- dfprintk(LOOKUPCACHE, "NFS: %s(%pd2) is invalid\n",
- __func__, dentry);
- return 0;
+ error = 1;
+ break;
}
- dfprintk(LOOKUPCACHE, "NFS: %s(%pd2) lookup returned error %d\n",
- __func__, dentry, error);
+ trace_nfs_lookup_revalidate_exit(dir, dentry, 0, error);
return error;
}

@@ -1623,9 +1618,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
goto out_bad;

trace_nfs_lookup_revalidate_enter(dir, dentry, flags);
- error = nfs_lookup_revalidate_dentry(dir, dentry, inode);
- trace_nfs_lookup_revalidate_exit(dir, dentry, flags, error);
- return error;
+ return nfs_lookup_revalidate_dentry(dir, dentry, inode);
out_valid:
return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
out_bad:
--
2.35.1