Commit 24aa1fe6 authored by Trond Myklebust's avatar Trond Myklebust
Browse files

NFS: Fix a few further cache consistency regressions 



 Steve Dickson writes:
 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 show how the client will never (and I mean never ;-) ) see
 the updated file.

 The problem is that we do not update nfsi->cache_change_attribute when the
 file changes on the server (we only update it when our client makes the
 changes). This again means that functions like nfs_check_verifier() will
 fail to register when the parent directory has changed and should trigger
 a dentry lookup revalidation.

 Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 223db122

fs/nfs/inode.c

+20 −34
Original line number Diff line number Diff line
@@ -54,7 +54,7 @@ 
#define NFS_MAX_READAHEAD	(RPC_DEF_SLOT_TABLE - 1)



static void nfs_invalidate_inode(struct inode *);

static int nfs_update_inode(struct inode *, struct nfs_fattr *, unsigned long);

static int nfs_update_inode(struct inode *, struct nfs_fattr *);



static struct inode *nfs_alloc_inode(struct super_block *sb);

static void nfs_destroy_inode(struct inode *);
@@ -1080,8 +1080,6 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) 
	int		 status = -ESTALE;

	struct nfs_fattr fattr;

	struct nfs_inode *nfsi = NFS_I(inode);

	unsigned long verifier;

	unsigned long cache_validity;



	dfprintk(PAGECACHE, "NFS: revalidating (%s/%Ld)\n",

		inode->i_sb->s_id, (long long)NFS_FILEID(inode));
@@ -1106,8 +1104,6 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) 
		}

	}



	/* Protect against RPC races by saving the change attribute */

	verifier = nfs_save_change_attribute(inode);

	status = NFS_PROTO(inode)->getattr(server, NFS_FH(inode), &fattr);

	if (status != 0) {

		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%s/%Ld) getattr failed, error=%d\n",
@@ -1122,7 +1118,7 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) 
	}



	spin_lock(&inode->i_lock);

	status = nfs_update_inode(inode, &fattr, verifier);

	status = nfs_update_inode(inode, &fattr);

	if (status) {

		spin_unlock(&inode->i_lock);

		dfprintk(PAGECACHE, "nfs_revalidate_inode: (%s/%Ld) refresh failed, error=%d\n",
@@ -1130,20 +1126,11 @@ __nfs_revalidate_inode(struct nfs_server *server, struct inode *inode) 
			 (long long)NFS_FILEID(inode), status);

		goto out;

	}

	cache_validity = nfsi->cache_validity;

	nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE;



	/*

	 * We may need to keep the attributes marked as invalid if

	 * we raced with nfs_end_attr_update().

	 */

	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);



	nfs_revalidate_mapping(inode, inode->i_mapping);



	if (cache_validity & NFS_INO_INVALID_ACL)

	if (nfsi->cache_validity & NFS_INO_INVALID_ACL)

		nfs_zap_acl_cache(inode);



	dfprintk(PAGECACHE, "NFS: (%s/%Ld) revalidation complete\n",
@@ -1346,10 +1333,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) 
		return 0;

	spin_lock(&inode->i_lock);

	nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE;

	if (nfs_verify_change_attribute(inode, fattr->time_start))

		nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME);

	if (time_after(fattr->time_start, nfsi->last_updated))

		status = nfs_update_inode(inode, fattr, fattr->time_start);

		status = nfs_update_inode(inode, fattr);

	else

		status = nfs_check_inode_attributes(inode, fattr);
@@ -1375,10 +1360,7 @@ int nfs_post_op_update_inode(struct inode *inode, struct nfs_fattr *fattr) 
		nfsi->cache_validity |= NFS_INO_INVALID_ATTR | NFS_INO_INVALID_ACCESS;

		goto out;

	}

	status = nfs_update_inode(inode, fattr, fattr->time_start);

	if (time_after_eq(fattr->time_start, nfsi->cache_change_attribute))

		nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME|NFS_INO_REVAL_PAGECACHE);

	nfsi->cache_change_attribute = jiffies;

	status = nfs_update_inode(inode, fattr);

out:

	spin_unlock(&inode->i_lock);

	return status;
@@ -1396,12 +1378,12 @@ out: 
 *

 * A very similar scenario holds for the dir cache.

 */

static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsigned long verifier)

static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)

{

	struct nfs_inode *nfsi = NFS_I(inode);

	loff_t cur_isize, new_isize;

	unsigned int	invalid = 0;

	int data_unstable;

	int data_stable;



	dfprintk(VFS, "NFS: %s(%s/%ld ct=%d info=0x%x)\n",

			__FUNCTION__, inode->i_sb->s_id, inode->i_ino,
@@ -1432,8 +1414,9 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
	nfsi->last_updated = jiffies;



	/* Are we racing with known updates of the metadata on the server? */

	data_unstable = ! (nfs_verify_change_attribute(inode, verifier) ||

		(nfsi->cache_validity & NFS_INO_REVAL_PAGECACHE));

	data_stable = nfs_verify_change_attribute(inode, fattr->time_start);

	if (data_stable)

		nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME);



	/* Check if our cached file size is stale */

 	new_isize = nfs_size_to_loff_t(fattr->size);
@@ -1442,7 +1425,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
		/* Do we perhaps have any outstanding writes? */

		if (nfsi->npages == 0) {

			/* No, but did we race with nfs_end_data_update()? */

			if (time_after_eq(verifier,  nfsi->cache_change_attribute)) {

			if (data_stable) {

				inode->i_size = new_isize;

				invalid |= NFS_INO_INVALID_DATA;

			}
@@ -1451,6 +1434,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
			inode->i_size = new_isize;

			invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;

		}

		nfsi->cache_change_attribute = jiffies;

		dprintk("NFS: isize change on server for file %s/%ld\n",

				inode->i_sb->s_id, inode->i_ino);

	}
@@ -1460,8 +1444,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
		memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime));

		dprintk("NFS: mtime change on server for file %s/%ld\n",

				inode->i_sb->s_id, inode->i_ino);

		if (!data_unstable)

		invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;

		nfsi->cache_change_attribute = jiffies;

	}



	if ((fattr->valid & NFS_ATTR_FATTR_V4)
@@ -1469,15 +1453,15 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
		dprintk("NFS: change_attr change on server for file %s/%ld\n",

		       inode->i_sb->s_id, inode->i_ino);

		nfsi->change_attr = fattr->change_attr;

		if (!data_unstable)

		invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;

		nfsi->cache_change_attribute = jiffies;

	}



	/* If ctime has changed we should definitely clear access+acl caches */

	if (!timespec_equal(&inode->i_ctime, &fattr->ctime)) {

		if (!data_unstable)

		invalid |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;

		memcpy(&inode->i_ctime, &fattr->ctime, sizeof(inode->i_ctime));

		nfsi->cache_change_attribute = jiffies;

	}

	memcpy(&inode->i_atime, &fattr->atime, sizeof(inode->i_atime));
@@ -1515,6 +1499,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign 
	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)

				|| S_ISLNK(inode->i_mode)))

		invalid &= ~NFS_INO_INVALID_DATA;

	if (data_stable)

		invalid &= ~(NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME|NFS_INO_REVAL_PAGECACHE);

	if (!nfs_have_delegation(inode, FMODE_READ))

		nfsi->cache_validity |= invalid;

CRA Git | Maintained and supported by SUSTech CRA and CCSE