| Message ID | 20181019152905.32418-11-olga.kornievskaia@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | server-side support for "inter" SSC copy | expand |
On Fri, Oct 19, 2018 at 11:29:02AM -0400, Olga Kornievskaia wrote: > From: Olga Kornievskaia <kolga@netapp.com> > > Incoming stateid (used by a READ) could be a saved copy stateid. > On first use make it active and check that the copy has started > within the allowable lease time. > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > --- > fs/nfsd/nfs4state.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 7764a8b..16359de 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -5206,6 +5206,47 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > > return 0; > } > +/* > + * A READ from an inter server to server COPY will have a > + * copy stateid. Return the parent nfs4_stid. > + */ > +static __be32 _find_cpntf_state(struct nfsd_net *nn, stateid_t *st, > + struct nfs4_cpntf_state **cps) > +{ > + struct nfs4_cpntf_state *state = NULL; > + > + if (st->si_opaque.so_clid.cl_id != nn->s2s_cp_cl_id) > + return nfserr_bad_stateid; > + spin_lock(&nn->s2s_cp_lock); > + state = idr_find(&nn->s2s_cp_stateids, st->si_opaque.so_id); > + spin_unlock(&nn->s2s_cp_lock); > + if (!state) > + return nfserr_bad_stateid; > + *cps = state; > + return 0; > +} > + > +static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st, > + struct nfs4_stid **stid) > +{ > + __be32 status; > + struct nfs4_cpntf_state *cps = NULL; > + > + status = _find_cpntf_state(nn, st, &cps); > + if (status) > + return status; > + > + /* Did the inter server to server copy start in time? */ > + if (cps->cp_active == false && !time_after(cps->cp_timeout, jiffies)) Is there any other time limit, or can the destination server keeping using this stateid indefinitely as long as it manages to get its first READ in before cp_timeout? > + return nfserr_partner_no_auth; > + else > + cps->cp_active = true; > + > + *stid = cps->cp_p_stid; > + refcount_inc(&cps->cp_p_stid->sc_count); Does the caller hold some lock? If not, what's prevnting cps from being freed before we get around to incrementing sc_count here? I would have expected the refcount_inc to happen before we drop s2s_cp_lock, but, like I say, maybe I'm missing some other locking. --b. > + > + return nfs_ok; > +} > > /* > * Checks for stateid operations > @@ -5238,6 +5279,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > status = nfsd4_lookup_stateid(cstate, stateid, > NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID, > &s, nn); > + if (status == nfserr_bad_stateid) > + status = find_cpntf_state(nn, stateid, &s); > if (status) > return status; > status = nfsd4_stid_check_stateid_generation(stateid, s, > -- > 1.8.3.1
On Mon, Nov 5, 2018 at 4:33 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Oct 19, 2018 at 11:29:02AM -0400, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@netapp.com> > > > > Incoming stateid (used by a READ) could be a saved copy stateid. > > On first use make it active and check that the copy has started > > within the allowable lease time. > > > > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > > --- > > fs/nfsd/nfs4state.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index 7764a8b..16359de 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -5206,6 +5206,47 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > > > > return 0; > > } > > +/* > > + * A READ from an inter server to server COPY will have a > > + * copy stateid. Return the parent nfs4_stid. > > + */ > > +static __be32 _find_cpntf_state(struct nfsd_net *nn, stateid_t *st, > > + struct nfs4_cpntf_state **cps) > > +{ > > + struct nfs4_cpntf_state *state = NULL; > > + > > + if (st->si_opaque.so_clid.cl_id != nn->s2s_cp_cl_id) > > + return nfserr_bad_stateid; > > + spin_lock(&nn->s2s_cp_lock); > > + state = idr_find(&nn->s2s_cp_stateids, st->si_opaque.so_id); > > + spin_unlock(&nn->s2s_cp_lock); > > + if (!state) > > + return nfserr_bad_stateid; > > + *cps = state; > > + return 0; > > +} > > + > > +static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st, > > + struct nfs4_stid **stid) > > +{ > > + __be32 status; > > + struct nfs4_cpntf_state *cps = NULL; > > + > > + status = _find_cpntf_state(nn, st, &cps); > > + if (status) > > + return status; > > + > > + /* Did the inter server to server copy start in time? */ > > + if (cps->cp_active == false && !time_after(cps->cp_timeout, jiffies)) > > Is there any other time limit, or can the destination server keeping > using this stateid indefinitely as long as it manages to get its first > READ in before cp_timeout? I don't believe there is anything in the spec that prohibits the stateid from being used once it was started within a timeout period (a copy might take a long time). Then it's invalidated either by OFFLOAD_CANCEL or the parent stateid going away. > > > + return nfserr_partner_no_auth; > > + else > > + cps->cp_active = true; > > + > > + *stid = cps->cp_p_stid; > > + refcount_inc(&cps->cp_p_stid->sc_count); > > Does the caller hold some lock? If not, what's prevnting cps from being > freed before we get around to incrementing sc_count here? > > I would have expected the refcount_inc to happen before we drop > s2s_cp_lock, but, like I say, maybe I'm missing some other locking. No you are not missing it, the lock is taken only to find it in the list. I'll move the refcount_inc into the lock section. > > --b. > > > + > > + return nfs_ok; > > +} > > > > /* > > * Checks for stateid operations > > @@ -5238,6 +5279,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) > > status = nfsd4_lookup_stateid(cstate, stateid, > > NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID, > > &s, nn); > > + if (status == nfserr_bad_stateid) > > + status = find_cpntf_state(nn, stateid, &s); > > if (status) > > return status; > > status = nfsd4_stid_check_stateid_generation(stateid, s, > > -- > > 1.8.3.1
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 7764a8b..16359de 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -5206,6 +5206,47 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) return 0; } +/* + * A READ from an inter server to server COPY will have a + * copy stateid. Return the parent nfs4_stid. + */ +static __be32 _find_cpntf_state(struct nfsd_net *nn, stateid_t *st, + struct nfs4_cpntf_state **cps) +{ + struct nfs4_cpntf_state *state = NULL; + + if (st->si_opaque.so_clid.cl_id != nn->s2s_cp_cl_id) + return nfserr_bad_stateid; + spin_lock(&nn->s2s_cp_lock); + state = idr_find(&nn->s2s_cp_stateids, st->si_opaque.so_id); + spin_unlock(&nn->s2s_cp_lock); + if (!state) + return nfserr_bad_stateid; + *cps = state; + return 0; +} + +static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st, + struct nfs4_stid **stid) +{ + __be32 status; + struct nfs4_cpntf_state *cps = NULL; + + status = _find_cpntf_state(nn, st, &cps); + if (status) + return status; + + /* Did the inter server to server copy start in time? */ + if (cps->cp_active == false && !time_after(cps->cp_timeout, jiffies)) + return nfserr_partner_no_auth; + else + cps->cp_active = true; + + *stid = cps->cp_p_stid; + refcount_inc(&cps->cp_p_stid->sc_count); + + return nfs_ok; +} /* * Checks for stateid operations @@ -5238,6 +5279,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid) status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID, &s, nn); + if (status == nfserr_bad_stateid) + status = find_cpntf_state(nn, stateid, &s); if (status) return status; status = nfsd4_stid_check_stateid_generation(stateid, s,