| Message ID | 1397846704-14567-38-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Patch-sequencing nit: I'm guessing this wasn't actually a problem until previous patches added sequenceid reference counting, turning this case into a reference leak. Therefore to avoid a temporary regression we should probably make this change before introducing the reference counting rather than after. --b. On Fri, Apr 18, 2014 at 02:44:31PM -0400, Trond Myklebust wrote: > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfsd/nfs4state.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index b9d6da652fb1..03a3f51d2828 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -4068,6 +4068,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, > { > __be32 status; > struct nfs4_stid *s; > + struct nfs4_ol_stateid *stp = NULL; > > dprintk("NFSD: %s: seqid=%d stateid = " STATEID_FMT "\n", __func__, > seqid, STATEID_VAL(stateid)); > @@ -4077,10 +4078,13 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, > cstate->minorversion, nn); > if (status) > return status; > - *stpp = openlockstateid(s); > - nfsd4_cstate_assign_replay(cstate, (*stpp)->st_stateowner); > + stp = openlockstateid(s); > + nfsd4_cstate_assign_replay(cstate, stp->st_stateowner); > > - return nfs4_seqid_op_checks(cstate, stateid, seqid, *stpp); > + status = nfs4_seqid_op_checks(cstate, stateid, seqid, stp); > + if (!status) > + *stpp = stp; > + return status; > } > > static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, > -- > 1.9.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 7, 2014 at 3:58 PM, Bruce Fields <bfields@fieldses.org> wrote: > Patch-sequencing nit: I'm guessing this wasn't actually a problem until > previous patches added sequenceid reference counting, turning this case > into a reference leak. > > Therefore to avoid a temporary regression we should probably make this > change before introducing the reference counting rather than after. No, there should be no regression. This patch is a cleanup in preparation for the introduction of reference counting. > --b. > > On Fri, Apr 18, 2014 at 02:44:31PM -0400, Trond Myklebust wrote: >> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> --- >> fs/nfsd/nfs4state.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index b9d6da652fb1..03a3f51d2828 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -4068,6 +4068,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, >> { >> __be32 status; >> struct nfs4_stid *s; >> + struct nfs4_ol_stateid *stp = NULL; >> >> dprintk("NFSD: %s: seqid=%d stateid = " STATEID_FMT "\n", __func__, >> seqid, STATEID_VAL(stateid)); >> @@ -4077,10 +4078,13 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, >> cstate->minorversion, nn); >> if (status) >> return status; >> - *stpp = openlockstateid(s); >> - nfsd4_cstate_assign_replay(cstate, (*stpp)->st_stateowner); >> + stp = openlockstateid(s); >> + nfsd4_cstate_assign_replay(cstate, stp->st_stateowner); >> >> - return nfs4_seqid_op_checks(cstate, stateid, seqid, *stpp); >> + status = nfs4_seqid_op_checks(cstate, stateid, seqid, stp); >> + if (!status) >> + *stpp = stp; >> + return status; >> } >> >> static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, >> -- >> 1.9.0 >>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b9d6da652fb1..03a3f51d2828 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4068,6 +4068,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, { __be32 status; struct nfs4_stid *s; + struct nfs4_ol_stateid *stp = NULL; dprintk("NFSD: %s: seqid=%d stateid = " STATEID_FMT "\n", __func__, seqid, STATEID_VAL(stateid)); @@ -4077,10 +4078,13 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, cstate->minorversion, nn); if (status) return status; - *stpp = openlockstateid(s); - nfsd4_cstate_assign_replay(cstate, (*stpp)->st_stateowner); + stp = openlockstateid(s); + nfsd4_cstate_assign_replay(cstate, stp->st_stateowner); - return nfs4_seqid_op_checks(cstate, stateid, seqid, *stpp); + status = nfs4_seqid_op_checks(cstate, stateid, seqid, stp); + if (!status) + *stpp = stp; + return status; } static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfsd/nfs4state.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)