Commit f044636d authored by Trond Myklebust's avatar Trond Myklebust

NFS: Add attribute update barriers to nfs_setattr_update_inode()

Ensure that other operations which raced with our setattr RPC call
cannot revert the file attribute changes that were made on the server.
To do so, we artificially bump the attribute generation counter on
the inode so that all calls to nfs_fattr_init() that precede ours
will be dropped.

The motivation for the patch came from Chuck Lever's reports of readaheads
racing with truncate operations and causing the file size to be reverted.
Reported-by: default avatarChuck Lever <chuck.lever@oracle.com>
Signed-off-by: default avatarTrond Myklebust <trond.myklebust@primarydata.com>
Tested-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 140e049c
...@@ -556,6 +556,7 @@ EXPORT_SYMBOL_GPL(nfs_setattr); ...@@ -556,6 +556,7 @@ EXPORT_SYMBOL_GPL(nfs_setattr);
* This is a copy of the common vmtruncate, but with the locking * This is a copy of the common vmtruncate, but with the locking
* corrected to take into account the fact that NFS requires * corrected to take into account the fact that NFS requires
* inode->i_size to be updated under the inode->i_lock. * inode->i_size to be updated under the inode->i_lock.
* Note: must be called with inode->i_lock held!
*/ */
static int nfs_vmtruncate(struct inode * inode, loff_t offset) static int nfs_vmtruncate(struct inode * inode, loff_t offset)
{ {
...@@ -565,14 +566,14 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) ...@@ -565,14 +566,14 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset)
if (err) if (err)
goto out; goto out;
spin_lock(&inode->i_lock);
i_size_write(inode, offset); i_size_write(inode, offset);
/* Optimisation */ /* Optimisation */
if (offset == 0) if (offset == 0)
NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_DATA; NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_DATA;
spin_unlock(&inode->i_lock);
spin_unlock(&inode->i_lock);
truncate_pagecache(inode, offset); truncate_pagecache(inode, offset);
spin_lock(&inode->i_lock);
out: out:
return err; return err;
} }
...@@ -585,10 +586,15 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset) ...@@ -585,10 +586,15 @@ static int nfs_vmtruncate(struct inode * inode, loff_t offset)
* Note: we do this in the *proc.c in order to ensure that * Note: we do this in the *proc.c in order to ensure that
* it works for things like exclusive creates too. * it works for things like exclusive creates too.
*/ */
void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr) void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr,
struct nfs_fattr *fattr)
{ {
if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) { /* Barrier: bump the attribute generation count. */
nfs_fattr_set_barrier(fattr);
spin_lock(&inode->i_lock); spin_lock(&inode->i_lock);
NFS_I(inode)->attr_gencount = fattr->gencount;
if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) {
if ((attr->ia_valid & ATTR_MODE) != 0) { if ((attr->ia_valid & ATTR_MODE) != 0) {
int mode = attr->ia_mode & S_IALLUGO; int mode = attr->ia_mode & S_IALLUGO;
mode |= inode->i_mode & ~S_IALLUGO; mode |= inode->i_mode & ~S_IALLUGO;
...@@ -600,12 +606,13 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr) ...@@ -600,12 +606,13 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
inode->i_gid = attr->ia_gid; inode->i_gid = attr->ia_gid;
nfs_set_cache_invalid(inode, NFS_INO_INVALID_ACCESS nfs_set_cache_invalid(inode, NFS_INO_INVALID_ACCESS
| NFS_INO_INVALID_ACL); | NFS_INO_INVALID_ACL);
spin_unlock(&inode->i_lock);
} }
if ((attr->ia_valid & ATTR_SIZE) != 0) { if ((attr->ia_valid & ATTR_SIZE) != 0) {
nfs_inc_stats(inode, NFSIOS_SETATTRTRUNC); nfs_inc_stats(inode, NFSIOS_SETATTRTRUNC);
nfs_vmtruncate(inode, attr->ia_size); nfs_vmtruncate(inode, attr->ia_size);
} }
nfs_update_inode(inode, fattr);
spin_unlock(&inode->i_lock);
} }
EXPORT_SYMBOL_GPL(nfs_setattr_update_inode); EXPORT_SYMBOL_GPL(nfs_setattr_update_inode);
......
...@@ -138,7 +138,7 @@ nfs3_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr, ...@@ -138,7 +138,7 @@ nfs3_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
nfs_fattr_init(fattr); nfs_fattr_init(fattr);
status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0); status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0);
if (status == 0) if (status == 0)
nfs_setattr_update_inode(inode, sattr); nfs_setattr_update_inode(inode, sattr, fattr);
dprintk("NFS reply setattr: %d\n", status); dprintk("NFS reply setattr: %d\n", status);
return status; return status;
} }
......
...@@ -2416,8 +2416,8 @@ static int _nfs4_do_open(struct inode *dir, ...@@ -2416,8 +2416,8 @@ static int _nfs4_do_open(struct inode *dir,
opendata->o_res.f_attr, sattr, opendata->o_res.f_attr, sattr,
state, label, olabel); state, label, olabel);
if (status == 0) { if (status == 0) {
nfs_setattr_update_inode(state->inode, sattr); nfs_setattr_update_inode(state->inode, sattr,
nfs_post_op_update_inode(state->inode, opendata->o_res.f_attr); opendata->o_res.f_attr);
nfs_setsecurity(state->inode, opendata->o_res.f_attr, olabel); nfs_setsecurity(state->inode, opendata->o_res.f_attr, olabel);
} }
} }
...@@ -3291,7 +3291,7 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr, ...@@ -3291,7 +3291,7 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
status = nfs4_do_setattr(inode, cred, fattr, sattr, state, NULL, label); status = nfs4_do_setattr(inode, cred, fattr, sattr, state, NULL, label);
if (status == 0) { if (status == 0) {
nfs_setattr_update_inode(inode, sattr); nfs_setattr_update_inode(inode, sattr, fattr);
nfs_setsecurity(inode, fattr, label); nfs_setsecurity(inode, fattr, label);
} }
nfs4_label_free(label); nfs4_label_free(label);
......
...@@ -139,7 +139,7 @@ nfs_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr, ...@@ -139,7 +139,7 @@ nfs_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
nfs_fattr_init(fattr); nfs_fattr_init(fattr);
status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0); status = rpc_call_sync(NFS_CLIENT(inode), &msg, 0);
if (status == 0) if (status == 0)
nfs_setattr_update_inode(inode, sattr); nfs_setattr_update_inode(inode, sattr, fattr);
dprintk("NFS reply setattr: %d\n", status); dprintk("NFS reply setattr: %d\n", status);
return status; return status;
} }
......
...@@ -356,7 +356,7 @@ extern int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *ino ...@@ -356,7 +356,7 @@ extern int nfs_revalidate_inode_rcu(struct nfs_server *server, struct inode *ino
extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *); extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *);
extern int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping); extern int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping);
extern int nfs_setattr(struct dentry *, struct iattr *); extern int nfs_setattr(struct dentry *, struct iattr *);
extern void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr); extern void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr, struct nfs_fattr *);
extern void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr, extern void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr,
struct nfs4_label *label); struct nfs4_label *label);
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);
......
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