NFS: Fix the resolution problem with nfs_inode_attrs_need_update()

It appears that 'jiffies' timestamps do not have high enough resolution for
nfs_inode_attrs_need_update(). One problem is that a GETATTR can be
launched within < 1 jiffy of the last operation that updated the attribute.
Another problem is that RPC calls can take < 1 jiffy to execute.

We can fix this by switching the variables to use a simple global counter
that gets incremented every time we start another GETATTR call.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 49d5654128276f0d7ad5ab56394f021c64c9440a..4807074ada8c7564c7020f2a091ec5ca8e9f4614 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -156,6 +156,7 @@ typedef struct {
        decode_dirent_t decode;
        int             plus;
        unsigned long   timestamp;
+       unsigned long   gencount;
        int             timestamp_valid;
 } nfs_readdir_descriptor_t;
 
@@ -177,7 +178,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page)
        struct file     *file = desc->file;
        struct inode    *inode = file->f_path.dentry->d_inode;
        struct rpc_cred *cred = nfs_file_cred(file);
-       unsigned long   timestamp;
+       unsigned long   timestamp, gencount;
        int             error;
 
        dfprintk(DIRCACHE, "NFS: %s: reading cookie %Lu into page %lu\n",
@@ -186,6 +187,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page)
 
  again:
        timestamp = jiffies;
+       gencount = nfs_inc_attr_generation_counter();
        error = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred, desc->entry->cookie, page,
                                          NFS_SERVER(inode)->dtsize, desc->plus);
        if (error < 0) {
@@ -199,6 +201,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page *page)
                goto error;
        }
        desc->timestamp = timestamp;
+       desc->gencount = gencount;
        desc->timestamp_valid = 1;
        SetPageUptodate(page);
        /* Ensure consistent page alignment of the data.
@@ -224,9 +227,10 @@ int dir_decode(nfs_readdir_descriptor_t *desc)
        if (IS_ERR(p))
                return PTR_ERR(p);
        desc->ptr = p;
-       if (desc->timestamp_valid)
+       if (desc->timestamp_valid) {
                desc->entry->fattr->time_start = desc->timestamp;
-       else
+               desc->entry->fattr->gencount = desc->gencount;
+       } else
                desc->entry->fattr->valid &= ~NFS_ATTR_FATTR;
        return 0;
 }
@@ -471,7 +475,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
        struct rpc_cred *cred = nfs_file_cred(file);
        struct page     *page = NULL;
        int             status;
-       unsigned long   timestamp;
+       unsigned long   timestamp, gencount;
 
        dfprintk(DIRCACHE, "NFS: uncached_readdir() searching for cookie %Lu\n",
                        (unsigned long long)*desc->dir_cookie);
@@ -482,6 +486,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
                goto out;
        }
        timestamp = jiffies;
+       gencount = nfs_inc_attr_generation_counter();
        status = NFS_PROTO(inode)->readdir(file->f_path.dentry, cred,
                                                *desc->dir_cookie, page,
                                                NFS_SERVER(inode)->dtsize,
@@ -490,6 +495,7 @@ int uncached_readdir(nfs_readdir_descriptor_t *desc, void *dirent,
        desc->ptr = kmap(page);         /* matching kunmap in nfs_do_filldir */
        if (status >= 0) {
                desc->timestamp = timestamp;
+               desc->gencount = gencount;
                desc->timestamp_valid = 1;
                if ((status = dir_decode(desc)) == 0)
                        desc->entry->prev_cookie = *desc->dir_cookie;

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index de3f11e6234e2e591874f787edf9bd90f6ebf4df..116a3bd2bc9b7e08428bddb5ad1da62ebf83b54b 100644 (file)
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -305,7 +305,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
                        init_special_inode(inode, inode->i_mode, fattr->rdev);
 
                nfsi->read_cache_jiffies = fattr->time_start;
-               nfsi->last_updated = now;
+               nfsi->attr_gencount = fattr->gencount;
                nfsi->cache_change_attribute = now;
                inode->i_atime = fattr->atime;
                inode->i_mtime = fattr->mtime;
@@ -909,6 +909,30 @@ static int nfs_size_need_update(const struct inode *inode, const struct nfs_fatt
        return nfs_size_to_loff_t(fattr->size) > i_size_read(inode);
 }
 
+static unsigned long nfs_attr_generation_counter;
+
+static unsigned long nfs_read_attr_generation_counter(void)
+{
+       smp_rmb();
+       return nfs_attr_generation_counter;
+}
+
+unsigned long nfs_inc_attr_generation_counter(void)
+{
+       unsigned long ret;
+       smp_rmb();
+       ret = ++nfs_attr_generation_counter;
+       smp_wmb();
+       return ret;
+}
+
+void nfs_fattr_init(struct nfs_fattr *fattr)
+{
+       fattr->valid = 0;
+       fattr->time_start = jiffies;
+       fattr->gencount = nfs_inc_attr_generation_counter();
+}
+
 /**
  * nfs_inode_attrs_need_update - check if the inode attributes need updating
  * @inode - pointer to inode
@@ -922,8 +946,7 @@ static int nfs_size_need_update(const struct inode *inode, const struct nfs_fatt
  * catch the case where ctime either didn't change, or went backwards
  * (if someone reset the clock on the server) by looking at whether
  * or not this RPC call was started after the inode was last updated.
- * Note also the check for jiffy wraparound if the last_updated timestamp
- * is later than 'jiffies'.
+ * Note also the check for wraparound of 'attr_gencount'
  *
  * The function returns 'true' if it thinks the attributes in 'fattr' are
  * more recent than the ones cached in the inode.
@@ -933,10 +956,10 @@ static int nfs_inode_attrs_need_update(const struct inode *inode, const struct n
 {
        const struct nfs_inode *nfsi = NFS_I(inode);
 
-       return time_after(fattr->time_start, nfsi->last_updated) ||
+       return ((long)fattr->gencount - (long)nfsi->attr_gencount) > 0 ||
                nfs_ctime_need_update(inode, fattr) ||
                nfs_size_need_update(inode, fattr) ||
-               time_after(nfsi->last_updated, jiffies);
+               ((long)nfsi->attr_gencount - (long)nfs_read_attr_generation_counter() > 0);
 }
 
 static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr)
@@ -1107,7 +1130,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
                }
                /* If ctime has changed we should definitely clear access+acl caches */
                if (!timespec_equal(&inode->i_ctime, &fattr->ctime))
-                       invalid |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
+                       invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
        } else if (nfsi->change_attr != fattr->change_attr) {
                dprintk("NFS: change_attr change on server for file %s/%ld\n",
                                inode->i_sb->s_id, inode->i_ino);
@@ -1163,7 +1186,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
                nfs_inc_stats(inode, NFSIOS_ATTRINVALIDATE);
                nfsi->attrtimeo = NFS_MINATTRTIMEO(inode);
                nfsi->attrtimeo_timestamp = now;
-               nfsi->last_updated = now;
+               nfsi->attr_gencount = nfs_inc_attr_generation_counter();
        } else {
                if (!time_in_range(now, nfsi->attrtimeo_timestamp, nfsi->attrtimeo_timestamp + nfsi->attrtimeo)) {
                        if ((nfsi->attrtimeo <<= 1) > NFS_MAXATTRTIMEO(inode))

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index ca563ee13e32a3c93e2619b66250a1aa622eda6d..ac8d0233b05c00650498f92750a1b0b8db994239 100644 (file)
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -137,7 +137,7 @@ struct nfs_inode {
        unsigned long           attrtimeo_timestamp;
        __u64                   change_attr;            /* v4 only */
 
-       unsigned long           last_updated;
+       unsigned long           attr_gencount;
        /* "Generation counter" for the attribute cache. This is
         * bumped whenever we update the metadata on the
         * server.
@@ -344,15 +344,11 @@ extern struct nfs_open_context *get_nfs_open_context(struct nfs_open_context *ct
 extern void put_nfs_open_context(struct nfs_open_context *ctx);
 extern struct nfs_open_context *nfs_find_open_context(struct inode *inode, struct rpc_cred *cred, int mode);
 extern u64 nfs_compat_user_ino64(u64 fileid);
+extern void nfs_fattr_init(struct nfs_fattr *fattr);
 
 /* linux/net/ipv4/ipconfig.c: trims ip addr off front of name, too. */
 extern __be32 root_nfs_parse_addr(char *name); /*__init*/
-
-static inline void nfs_fattr_init(struct nfs_fattr *fattr)
-{
-       fattr->valid = 0;
-       fattr->time_start = jiffies;
-}
+extern unsigned long nfs_inc_attr_generation_counter(void);
 
 /*
  * linux/fs/nfs/file.c

diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 6ee6ae3f095ceeeca48f0151c5ca11d33ab257cf..c1c31acb8a2b97608f55902de126fbdf0b0ddd64 100644 (file)
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -56,6 +56,7 @@ struct nfs_fattr {
        __u64                   change_attr;    /* NFSv4 change attribute */
        __u64                   pre_change_attr;/* pre-op NFSv4 change attribute */
        unsigned long           time_start;
+       unsigned long           gencount;
 };
 
 #define NFS_ATTR_WCC           0x0001          /* pre-op WCC data    */