.. this one breaks things again:
On 19 Nov 2016, at 11:54, Trond Myklebust wrote:
There is little point in setting NFS_INO_ADVISE_RDPLUS in nfs_lookup
and
nfs_lookup_revalidate() unless a process is actually doing readdir on
the
parent directory.
Furthermore, there is little point in using readdirplus if we're
trying
to revalidate a negative dentry.
Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
---
fs/nfs/dir.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 53e02b8bd9bd..5befd382be7d 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -455,14 +455,23 @@ bool nfs_use_readdirplus(struct inode *dir,
struct dir_context *ctx)
}
/*
- * This function is called by the lookup code to request the use of
- * readdirplus to accelerate any future lookups in the same
+ * This function is called by the lookup and getattr code to request
the
+ * use of readdirplus to accelerate any future lookups in the same
* directory.
+ * Do this by checking if there is an active file descriptor
+ * and calling nfs_advise_use_readdirplus, then forcing a
+ * cache flush.
*/
static
void nfs_advise_use_readdirplus(struct inode *dir)
{
- set_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(dir)->flags);
+ struct nfs_inode *nfsi = NFS_I(dir);
+
+ if (nfs_server_capable(dir, NFS_CAP_READDIRPLUS) &&
+ !list_empty(&nfsi->open_files)) {
+ set_bit(NFS_INO_ADVISE_RDPLUS, &nfsi->flags);
+ invalidate_mapping_pages(dir->i_mapping, 0, -1);
+ }
}
So every time advise_use_readdirplus it drops the mapping.. but what
about
subsequent calls into nfs_readdir() to get the next batch of entries?
For
the ls -l case, we want to keep setting NFS_INO_ADVISE_RDPLUS, but we
shouldn't start over filling the mapping from the beginning again.
/*
@@ -475,10 +484,7 @@ void nfs_advise_use_readdirplus(struct inode
*dir)
*/
void nfs_force_use_readdirplus(struct inode *dir)
{
- if (!list_empty(&NFS_I(dir)->open_files)) {
- nfs_advise_use_readdirplus(dir);
- invalidate_mapping_pages(dir->i_mapping, 0, -1);
- }
+ nfs_advise_use_readdirplus(dir);
}
static
@@ -1150,7 +1156,7 @@ static int nfs_lookup_revalidate(struct dentry
*dentry, unsigned int flags)
return -ECHILD;
goto out_bad;
}
- goto out_valid_noent;
+ goto out_valid;
}
if (is_bad_inode(inode)) {
@@ -1192,6 +1198,9 @@ static int nfs_lookup_revalidate(struct dentry
*dentry, unsigned int flags)
if (IS_ERR(label))
goto out_error;
+ /* We need to force a revalidation: set a readdirplus hint */
+ nfs_advise_use_readdirplus(dir);
+
trace_nfs_lookup_revalidate_enter(dir, dentry, flags);
error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, fhandle, fattr,
label);
trace_nfs_lookup_revalidate_exit(dir, dentry, flags, error);
@@ -1211,9 +1220,6 @@ static int nfs_lookup_revalidate(struct dentry
*dentry, unsigned int flags)
out_set_verifier:
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
out_valid:
- /* Success: notify readdir to use READDIRPLUS */
- nfs_advise_use_readdirplus(dir);
- out_valid_noent:
if (flags & LOOKUP_RCU) {
if (parent != ACCESS_ONCE(dentry->d_parent))
return -ECHILD;
Now when listing with `ls -l`: the second call into nfs_readdir() to
get
the next batch of entries will not have NFS_INO_ADVISE_RDPLUS.
I think this removes nfs_advise_use_readdirplus(dir) from the common
"goto
out_valid" path through nfs_lookup_revalidate() (the block with the
'iff'
typo).
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
