Commit 951a143b authored by Trond Myklebust's avatar Trond Myklebust

[PATCH] NFS: Fix the file size revalidation

 Instead of looking at whether or not the file is open for writes before
 we accept to update the length using the server value, we should rather
 be looking at whether or not we are currently caching any writes.

 Failure to do so means in particular that we're not updating the file
 length correctly after obtaining a POSIX or BSD lock.
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 08e9eac4
...@@ -517,7 +517,7 @@ retry: ...@@ -517,7 +517,7 @@ retry:
result = tot_bytes; result = tot_bytes;
out: out:
nfs_end_data_update_defer(inode); nfs_end_data_update(inode);
nfs_writedata_free(wdata); nfs_writedata_free(wdata);
return result; return result;
......
...@@ -1147,27 +1147,6 @@ void nfs_end_data_update(struct inode *inode) ...@@ -1147,27 +1147,6 @@ void nfs_end_data_update(struct inode *inode)
atomic_dec(&nfsi->data_updates); atomic_dec(&nfsi->data_updates);
} }
/**
* nfs_end_data_update_defer
* @inode - pointer to inode
* Declare end of the operations that will update file data
* This will defer marking the inode as needing revalidation
* unless there are no other pending updates.
*/
void nfs_end_data_update_defer(struct inode *inode)
{
struct nfs_inode *nfsi = NFS_I(inode);
if (atomic_dec_and_test(&nfsi->data_updates)) {
/* Mark the attribute cache for revalidation */
nfsi->flags |= NFS_INO_INVALID_ATTR;
/* Directories and symlinks: invalidate page cache too */
if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
nfsi->flags |= NFS_INO_INVALID_DATA;
nfsi->cache_change_attribute ++;
}
}
/** /**
* nfs_refresh_inode - verify consistency of the inode attribute cache * nfs_refresh_inode - verify consistency of the inode attribute cache
* @inode - pointer to inode * @inode - pointer to inode
...@@ -1222,8 +1201,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) ...@@ -1222,8 +1201,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
if (!timespec_equal(&inode->i_mtime, &fattr->mtime) if (!timespec_equal(&inode->i_mtime, &fattr->mtime)
|| cur_size != new_isize) || cur_size != new_isize)
nfsi->flags |= NFS_INO_INVALID_ATTR; nfsi->flags |= NFS_INO_INVALID_ATTR;
} else if (S_ISREG(inode->i_mode) && new_isize > cur_size) } else if (new_isize != cur_size && nfsi->npages == 0)
nfsi->flags |= NFS_INO_INVALID_ATTR; nfsi->flags |= NFS_INO_INVALID_ATTR;
/* Have any file permissions changed? */ /* Have any file permissions changed? */
if ((inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO) if ((inode->i_mode & S_IALLUGO) != (fattr->mode & S_IALLUGO)
...@@ -1257,10 +1236,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr) ...@@ -1257,10 +1236,8 @@ int nfs_refresh_inode(struct inode *inode, struct nfs_fattr *fattr)
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, unsigned long verifier)
{ {
struct nfs_inode *nfsi = NFS_I(inode); struct nfs_inode *nfsi = NFS_I(inode);
__u64 new_size; loff_t cur_isize, new_isize;
loff_t new_isize;
unsigned int invalid = 0; unsigned int invalid = 0;
loff_t cur_isize;
int data_unstable; int data_unstable;
dfprintk(VFS, "NFS: %s(%s/%ld ct=%d info=0x%x)\n", dfprintk(VFS, "NFS: %s(%s/%ld ct=%d info=0x%x)\n",
...@@ -1293,49 +1270,39 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign ...@@ -1293,49 +1270,39 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr, unsign
/* Are we racing with known updates of the metadata on the server? */ /* Are we racing with known updates of the metadata on the server? */
data_unstable = ! nfs_verify_change_attribute(inode, verifier); data_unstable = ! nfs_verify_change_attribute(inode, verifier);
/* Check if the file size agrees */ /* Check if our cached file size is stale */
new_size = fattr->size;
new_isize = nfs_size_to_loff_t(fattr->size); new_isize = nfs_size_to_loff_t(fattr->size);
cur_isize = i_size_read(inode); cur_isize = i_size_read(inode);
if (cur_isize != new_size) { if (new_isize != cur_isize) {
#ifdef NFS_DEBUG_VERBOSE /* Do we perhaps have any outstanding writes? */
printk(KERN_DEBUG "NFS: isize change on %s/%ld\n", inode->i_sb->s_id, inode->i_ino); if (nfsi->npages == 0) {
#endif /* No, but did we race with nfs_end_data_update()? */
/* if (verifier == nfsi->cache_change_attribute) {
* If we have pending writebacks, things can get
* messy.
*/
if (S_ISREG(inode->i_mode) && data_unstable) {
if (new_isize > cur_isize) {
inode->i_size = new_isize; inode->i_size = new_isize;
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; invalid |= NFS_INO_INVALID_DATA;
} }
} else { invalid |= NFS_INO_INVALID_ATTR;
} else if (new_isize > cur_isize) {
inode->i_size = new_isize; inode->i_size = new_isize;
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
} }
dprintk("NFS: isize change on server for file %s/%ld\n",
inode->i_sb->s_id, inode->i_ino);
} }
/* /* Check if the mtime agrees */
* Note: we don't check inode->i_mtime since pipes etc.
* can change this value in VFS without requiring a
* cache revalidation.
*/
if (!timespec_equal(&inode->i_mtime, &fattr->mtime)) { if (!timespec_equal(&inode->i_mtime, &fattr->mtime)) {
memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime)); memcpy(&inode->i_mtime, &fattr->mtime, sizeof(inode->i_mtime));
#ifdef NFS_DEBUG_VERBOSE dprintk("NFS: mtime change on server for file %s/%ld\n",
printk(KERN_DEBUG "NFS: mtime change on %s/%ld\n", inode->i_sb->s_id, inode->i_ino); inode->i_sb->s_id, inode->i_ino);
#endif
if (!data_unstable) if (!data_unstable)
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA; invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA;
} }
if ((fattr->valid & NFS_ATTR_FATTR_V4) if ((fattr->valid & NFS_ATTR_FATTR_V4)
&& nfsi->change_attr != fattr->change_attr) { && nfsi->change_attr != fattr->change_attr) {
#ifdef NFS_DEBUG_VERBOSE dprintk("NFS: change_attr change on server for file %s/%ld\n",
printk(KERN_DEBUG "NFS: change_attr change on %s/%ld\n",
inode->i_sb->s_id, inode->i_ino); inode->i_sb->s_id, inode->i_ino);
#endif
nfsi->change_attr = fattr->change_attr; nfsi->change_attr = fattr->change_attr;
if (!data_unstable) if (!data_unstable)
invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL; invalid |= NFS_INO_INVALID_ATTR|NFS_INO_INVALID_DATA|NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
......
...@@ -220,7 +220,7 @@ static int nfs_writepage_sync(struct nfs_open_context *ctx, struct inode *inode, ...@@ -220,7 +220,7 @@ static int nfs_writepage_sync(struct nfs_open_context *ctx, struct inode *inode,
ClearPageError(page); ClearPageError(page);
io_error: io_error:
nfs_end_data_update_defer(inode); nfs_end_data_update(inode);
nfs_writedata_free(wdata); nfs_writedata_free(wdata);
return written ? written : result; return written ? written : result;
} }
...@@ -401,7 +401,7 @@ static void nfs_inode_remove_request(struct nfs_page *req) ...@@ -401,7 +401,7 @@ static void nfs_inode_remove_request(struct nfs_page *req)
nfsi->npages--; nfsi->npages--;
if (!nfsi->npages) { if (!nfsi->npages) {
spin_unlock(&nfsi->req_lock); spin_unlock(&nfsi->req_lock);
nfs_end_data_update_defer(inode); nfs_end_data_update(inode);
iput(inode); iput(inode);
} else } else
spin_unlock(&nfsi->req_lock); spin_unlock(&nfsi->req_lock);
......
...@@ -294,7 +294,6 @@ extern void nfs_begin_attr_update(struct inode *); ...@@ -294,7 +294,6 @@ extern void nfs_begin_attr_update(struct inode *);
extern void nfs_end_attr_update(struct inode *); extern void nfs_end_attr_update(struct inode *);
extern void nfs_begin_data_update(struct inode *); extern void nfs_begin_data_update(struct inode *);
extern void nfs_end_data_update(struct inode *); extern void nfs_end_data_update(struct inode *);
extern void nfs_end_data_update_defer(struct inode *);
extern struct nfs_open_context *alloc_nfs_open_context(struct dentry *dentry, struct rpc_cred *cred); extern struct nfs_open_context *alloc_nfs_open_context(struct dentry *dentry, struct rpc_cred *cred);
extern struct nfs_open_context *get_nfs_open_context(struct nfs_open_context *ctx); extern struct nfs_open_context *get_nfs_open_context(struct nfs_open_context *ctx);
extern void put_nfs_open_context(struct nfs_open_context *ctx); extern void put_nfs_open_context(struct nfs_open_context *ctx);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment