On Tue, 2007-06-05 at 15:35 -0400, Jeff Layton wrote: > On Tue, 05 Jun 2007 14:13:02 -0400 > Trond Myklebust <trond.myklebust@xxxxxxxxxx> wrote: > > > On Tue, 2007-06-05 at 13:56 -0400, Jeff Layton wrote: > > > The Linux NFS4 client simply skips over the bitmask in an O_EXCL open > > > call and so it doesn't bother to reset any fields that may be holding > > > the verifier. This patch has us save the first two words of the bitmask > > > (which is all the current client has #defines for). The client then > > > later checks this bitmask and turns on the appropriate flags in the > > > sattr->ia_verify field for the following SETATTR call. > > > > > > This patch only currently checks to see if the server used the atime > > > and mtime slots for the verifier (which is what the Linux server uses > > > for this). I'm not sure of what other fields the server could > > > reasonably use, but adding checks for others should be trivial. > > > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > > > index 8e46e3e..34ddd66 100644 > > > --- a/fs/nfs/nfs4proc.c > > > +++ b/fs/nfs/nfs4proc.c > > > @@ -955,6 +955,22 @@ static struct nfs4_state *nfs4_open_delegated(struct inode *inode, int flags, st > > > } > > > > > > /* > > > + * on an EXCLUSIVE create, the server should send back a bitmask with FATTR4-* > > > + * fields corresponding to attributes that were used to store the verifier. > > > + * Make sure we clobber those fields in the later setattr call > > > + */ > > > +static inline void nfs4_exclusive_attrset (struct nfs4_opendata *opendata, struct iattr *sattr) > > > +{ > > > + if ((opendata->o_res.attrset[1] & FATTR4_WORD1_TIME_ACCESS) && > > > + !(sattr->ia_valid & ATTR_ATIME_SET)) > > > + sattr->ia_valid |= ATTR_ATIME; > > > + > > > + if ((opendata->o_res.attrset[1] & FATTR4_WORD1_TIME_MODIFY) && > > > + !(sattr->ia_valid & ATTR_MTIME_SET)) > > > + sattr->ia_valid |= ATTR_MTIME; > > > +} > > > + > > > +/* > > > * Returns a referenced nfs4_state > > > */ > > > static int _nfs4_do_open(struct inode *dir, struct dentry *dentry, int flags, struct iattr *sattr, struct rpc_cred *cred, struct nfs4_state **res) > > > @@ -985,6 +1001,9 @@ static int _nfs4_do_open(struct inode *dir, struct dentry *dentry, int flags, st > > > if (status != 0) > > > goto err_opendata_free; > > > > > > + if (opendata->o_arg.open_flags & O_EXCL) > > > + nfs4_exclusive_attrset(opendata, sattr); > > > + > > > status = -ENOMEM; > > > state = nfs4_opendata_to_nfs4_state(opendata); > > > if (state == NULL) > > > @@ -1787,6 +1806,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr, > > > status = nfs4_do_setattr(state->inode, &fattr, sattr, state); > > > if (status == 0) > > > nfs_setattr_update_inode(state->inode, sattr); > > > + nfs_refresh_inode(state->inode, &fattr); > > ^^^^ should be nfs_post_op_update_inode() to ensure > > that the attribute cache gets invalidated if the server fails to return > > the post-op attributes. > > > > It looks like nfs3_proc_create() uses nfs_refresh_inode in this spot (which was > why I initially used one here). Should it also be changed to use > nfs_post_op_update_inode(), or is it correct as-is? nfs3_proc_create() also appears to be calling nfs_setattr_update_inode() twice. I'll fix both issues in a separate patch. Trond - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html