NFS cache consistancy appears to be broken...

[Steve Dickson]

Hey Trond,

The attached patch seems to break cache consistence in a big way....
Doing the following:
1. On server:
$ mkdir ~/t
$ echo Hello > ~/t/tmp

2. On client, wait for a string to appear in this file:
$ until grep -q foo t/tmp ; do echo -n . ; sleep 1 ; done

3. On server, create a *new* file with the same name containing that string:
$ mv ~/t/tmp ~/t/tmp.old; echo foo > ~/t/tmp

will shows how the client will never (and I mean never ;-) ) see
the updated file. I reverted this patch and everything started
work as expected... so it appears using a jiffy-based cache
verifiers may not be such a good idea....

Note: I am using 2.6.15-rc2 kernel.

steved.



Subject:
NFS: Convert cache_change_attribute into a jiffy-based value
From:
Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>
Date:
Fri, 28 Oct 2005 09:07:29 -0700
To:
git-commits-head@xxxxxxxxxxxxxxx

tree 1f3d5db26462d02ecca383794b3061a5eae8d9cc
parent 0e574af1be5f569a5d7f2800333b0bfb358a5e34
author Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> Fri, 28 Oct 2005 06:12:38 -0400
committer Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> Fri, 28 Oct 2005 06:12:38 -0400

NFS: Convert cache_change_attribute into a jiffy-based value

 Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>

 fs/nfs/inode.c         |    8 ++++----
 include/linux/nfs_fs.h |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1135,7 +1135,7 @@ __nfs_revalidate_inode(struct nfs_server
 	 * We may need to keep the attributes marked as invalid if
 	 * we raced with nfs_end_attr_update().
 	 */
-	if (verifier == nfsi->cache_change_attribute)
+	if (time_after_eq(verifier, nfsi->cache_change_attribute))
 		nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME);
 	spin_unlock(&inode->i_lock);
 
@@ -1202,7 +1202,7 @@ void nfs_revalidate_mapping(struct inode
 		if (S_ISDIR(inode->i_mode)) {
 			memset(nfsi->cookieverf, 0, sizeof(nfsi->cookieverf));
 			/* This ensures we revalidate child dentries */
-			nfsi->cache_change_attribute++;
+			nfsi->cache_change_attribute = jiffies;
 		}
 		spin_unlock(&inode->i_lock);
 
@@ -1242,7 +1242,7 @@ void nfs_end_data_update(struct inode *i
 			nfsi->cache_validity |= NFS_INO_INVALID_DATA;
 		spin_unlock(&inode->i_lock);
 	}
-	nfsi->cache_change_attribute ++;
+	nfsi->cache_change_attribute = jiffies;
 	atomic_dec(&nfsi->data_updates);
 }
 
@@ -1391,7 +1391,7 @@ static int nfs_update_inode(struct inode
 		/* Do we perhaps have any outstanding writes? */
 		if (nfsi->npages == 0) {
 			/* No, but did we race with nfs_end_data_update()? */
-			if (verifier  ==  nfsi->cache_change_attribute) {
+			if (time_after_eq(verifier,  nfsi->cache_change_attribute)) {
 				inode->i_size = new_isize;
 				invalid |= NFS_INO_INVALID_DATA;
 			}
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -280,7 +280,7 @@ static inline long nfs_save_change_attri
 static inline int nfs_verify_change_attribute(struct inode *inode, unsigned long chattr)
 {
 	return !nfs_caches_unstable(inode)
-		&& chattr == NFS_I(inode)->cache_change_attribute;
+		&& time_after_eq(chattr, NFS_I(inode)->cache_change_attribute);
 }
 
 /*
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html