On Thu, 06 Feb 2014 17:12:59 -0500 Trond Myklebust
<trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Thu, 2014-02-06 at 13:51 +1100, NeilBrown wrote:
> > On Thu, 6 Feb 2014 13:45:39 +1100 NeilBrown <neilb@xxxxxxx> wrote:
> >
> >
> > > The change to nfs_update_inode fixes an issue that had me stumped for a
> > > while. It was still sending lots of GETATTR requests even after it had
> > > switched to READDIRPLUS instead of using cached info. So that might be a
> > > genuine bug that should be fixed independently of this patch.
> >
> > I managed to post the wrong version of the patch, which didn't have this
> > change. Sorry.
> >
> > Here is the real one.
> >
> > NeilBrown
>
> Hi Neil,
>
> Is there any reason why this patch couldn't just be replaced with the
> following?
Well.... the following doesn't make any change in my test case. :-(
In my test case there is a large directory on the server which doesn't change.
The client sends lots of requests to see if it has changed, but it hasn't.
(Tested with the labeled-NFS-regression fix applied)
Your patch only changes behaviour if nfs_force_lookup_revalidate is called,
and that is only called when NFS_ATTR_FATTR_CHANGE is set, or when an NFSv4
client modifies the directory. Neither of those are happening.
Thanks,
NeilBrown
> (Please note the separate fix for the labeled NFS regression that you
> pointed out.)
>
> Cheers
> Trond
>
> 8<-------------------------------------------------------------------
> >From fda6d0fb7988fd7d541ef6d1023bca92b0e10333 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Date: Thu, 6 Feb 2014 16:45:21 -0500
> Subject: [PATCH] NFS: Use readdirplus in order to efficiently revalidate whole
> directories
>
> When we call nfs_force_lookup_revalidate() in order to force a full
> lookup of all child dentries of a directory, it makes sense to use
> readdirplus to perform that revalidation as efficiently as possible.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/dir.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index be38b573495a..2abcae330ad0 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -437,6 +437,24 @@ void nfs_advise_use_readdirplus(struct inode *dir)
> set_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(dir)->flags);
> }
>
> +/*
> + * Force use of readdirplus to ensure efficient revalidation of
> + * the directory child dentries when we know that we will need
> + * to revalidate the whole directory.
> + *
> + * Caller must hold dir->i_lock.
> + */
> +static
> +void nfs_force_use_readdirplus(struct inode *dir)
> +{
> + if (nfs_server_capable(dir, NFS_CAP_READDIRPLUS)) {
> + struct nfs_inode *nfsi = NFS_I(dir);
> +
> + set_bit(NFS_INO_ADVISE_RDPLUS, &nfsi->flags);
> + nfsi->cache_validity |= NFS_INO_INVALID_DATA;
> + }
> +}
> +
> static
> void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
> {
> @@ -942,12 +960,14 @@ static int nfs_fsync_dir(struct file *filp, loff_t start, loff_t end,
> * This forces the revalidation code in nfs_lookup_revalidate() to do a
> * full lookup on all child dentries of 'dir' whenever a change occurs
> * on the server that might have invalidated our dcache.
> + * Call nfs_force_use_readdirplus for efficiency.
> *
> * The caller should be holding dir->i_lock
> */
> void nfs_force_lookup_revalidate(struct inode *dir)
> {
> NFS_I(dir)->cache_change_attribute++;
> + nfs_force_use_readdirplus(dir);
> }
> EXPORT_SYMBOL_GPL(nfs_force_lookup_revalidate);
>
Attachment:
signature.asc
Description: PGP signature
