NFS: Fix a performance regression in readdir
authorTrond Myklebust <trond.myklebust@primarydata.com>
Sat, 19 Nov 2016 15:54:55 +0000 (10:54 -0500)
committerTrond Myklebust <trond.myklebust@primarydata.com>
Fri, 2 Dec 2016 16:42:50 +0000 (11:42 -0500)
Ben Coddington reports that commit 311324ad1713, by adding the function
nfs_dir_mapping_need_revalidate() that checks page cache validity on
each call to nfs_readdir() causes a performance regression when
the directory is being modified.

If the directory is changing while we're iterating through the directory,
POSIX does not require us to invalidate the page cache unless the user
calls rewinddir(). However, we still do want to ensure that we use
readdirplus in order to avoid a load of stat() calls when the user
is doing an 'ls -l' workload.

The fix should be to invalidate the page cache immediately when we're
setting the NFS_INO_ADVISE_RDPLUS bit.

Reported-by: Benjamin Coddington <bcodding@redhat.com>
Fixes: 311324ad1713 ("NFS: Be more aggressive in using readdirplus...")
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
Tested-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
fs/nfs/dir.c

index f1ecfcc632eb8627ede5a61f8743f9745ea191d6..9220bc7b9cd778c1978d9317b72096e890551f6e 100644 (file)
@@ -477,7 +477,7 @@ void nfs_force_use_readdirplus(struct inode *dir)
 {
        if (!list_empty(&NFS_I(dir)->open_files)) {
                nfs_advise_use_readdirplus(dir);
-               nfs_zap_mapping(dir, dir->i_mapping);
+               invalidate_mapping_pages(dir->i_mapping, 0, -1);
        }
 }
 
@@ -886,17 +886,6 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc)
        goto out;
 }
 
-static bool nfs_dir_mapping_need_revalidate(struct inode *dir)
-{
-       struct nfs_inode *nfsi = NFS_I(dir);
-
-       if (nfs_attribute_cache_expired(dir))
-               return true;
-       if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
-               return true;
-       return false;
-}
-
 /* The file offset position represents the dirent entry number.  A
    last cookie cache takes care of the common case of reading the
    whole directory.
@@ -928,7 +917,7 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
        desc->decode = NFS_PROTO(inode)->decode_dirent;
        desc->plus = nfs_use_readdirplus(inode, ctx) ? 1 : 0;
 
-       if (ctx->pos == 0 || nfs_dir_mapping_need_revalidate(inode))
+       if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
                res = nfs_revalidate_mapping(inode, file->f_mapping);
        if (res < 0)
                goto out;