On Thu, 10 Jul 2014 01:34:06 -0700 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > @@ -3343,20 +3347,15 @@ out: > > static __be32 > > nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp, struct nfsd4_open *open) > > { > > - u32 op_share_access = open->op_share_access; > > __be32 status; > > > > - if (!test_access(op_share_access, stp)) > > - status = nfs4_get_vfs_file(rqstp, fp, cur_fh, open); > > + if (!test_access(open->op_share_access, stp)) > > + status = nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open); > > else > > status = nfsd4_truncate(rqstp, cur_fh, open); > > > > if (status) > > return status; > > - > > - /* remember the open */ > > - set_access(op_share_access, stp); > > - set_deny(open->op_share_deny, stp); > > return nfs_ok; > > This function is trivial enough now to be merged into the only caller, > especially as that open actually has another call to nfs4_get_vfs_file > right next to it in another branch. > Yes, but in two patches (nfsd: make deny mode enforcement more efficient and close races in it) we add some complexity back in here and at that point I think we'll want this in a separate function. So I think we shouldn't fold that into the caller since it'll just increase the churn. > > > } else { > > - status = nfs4_get_vfs_file(rqstp, fp, current_fh, open); > > - if (status) > > - goto out; > > stp = open->op_stp; > > open->op_stp = NULL; > > init_open_stateid(stp, fp, open); > > + status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open); > > + if (status) { > > + release_open_stateid(stp); > > + goto out; > > + } > > I can't find a place where we set the access bits before here. > Correct, we don't. That's not a problem though, AFAICT. The idea here is to go ahead and hash the stateid and then try to get the access to the file. If that fails, we unhash it and free the stateid. In a later patch we'll need to do that anyway to ensure that the fi_deny_mode is properly handled if it needs to be recalculated while we're trying to get a new filp for the file. -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html