* [RFC 0/3] nfsd41: do not expire client while in use by current compound
@ 2010-05-03 16:13 Benny Halevy
2010-05-03 16:31 ` [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres Benny Halevy
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Benny Halevy @ 2010-05-03 16:13 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
Bruce, the following patchset compiles but is not fully tested yet.
It is based on top of http://marc.info/?l=linux-nfs&m=127288934008791&w=2
Please review.
Thanks,
Benny
[RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres
[RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session
[RFC 3/3] nfsd4: do not expire nfs41 clients while in use
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres 2010-05-03 16:13 [RFC 0/3] nfsd41: do not expire client while in use by current compound Benny Halevy @ 2010-05-03 16:31 ` Benny Halevy 2010-05-03 22:37 ` J. Bruce Fields 2010-05-03 16:31 ` [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session Benny Halevy 2010-05-03 16:31 ` [RFC 3/3] nfsd4: do not expire nfs41 clients while in use Benny Halevy 2 siblings, 1 reply; 12+ messages in thread From: Benny Halevy @ 2010-05-03 16:31 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, Benny Halevy 'cs' is already computed, re-use it. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfsd/nfs4xdr.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index b04583c..19ff5a3 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -3311,9 +3311,9 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo if (cs->status != nfserr_replay_cache) { nfsd4_store_cache_entry(resp); dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); - resp->cstate.slot->sl_inuse = false; + cs->slot->sl_inuse = false; } - nfsd4_put_session(resp->cstate.session); + nfsd4_put_session(cs->session); } return 1; } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres 2010-05-03 16:31 ` [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres Benny Halevy @ 2010-05-03 22:37 ` J. Bruce Fields 0 siblings, 0 replies; 12+ messages in thread From: J. Bruce Fields @ 2010-05-03 22:37 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Mon, May 03, 2010 at 07:31:33PM +0300, Benny Halevy wrote: > 'cs' is already computed, re-use it. Thanks, applied. --b. > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > --- > fs/nfsd/nfs4xdr.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index b04583c..19ff5a3 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -3311,9 +3311,9 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > if (cs->status != nfserr_replay_cache) { > nfsd4_store_cache_entry(resp); > dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > - resp->cstate.slot->sl_inuse = false; > + cs->slot->sl_inuse = false; > } > - nfsd4_put_session(resp->cstate.session); > + nfsd4_put_session(cs->session); > } > return 1; > } > -- > 1.6.3.3 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-03 16:13 [RFC 0/3] nfsd41: do not expire client while in use by current compound Benny Halevy 2010-05-03 16:31 ` [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres Benny Halevy @ 2010-05-03 16:31 ` Benny Halevy 2010-05-03 22:36 ` J. Bruce Fields 2010-05-03 16:31 ` [RFC 3/3] nfsd4: do not expire nfs41 clients while in use Benny Halevy 2 siblings, 1 reply; 12+ messages in thread From: Benny Halevy @ 2010-05-03 16:31 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, Benny Halevy Although expire_client unhashes the session form the session table so no new compounds can find it, there's no refcount to keep the nfs4_client structure around while it's in use and referenced in the compound state via the session structure. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfsd/nfs4callback.c | 2 +- fs/nfsd/nfs4state.c | 4 +++- fs/nfsd/nfs4xdr.c | 1 + fs/nfsd/state.h | 6 ++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 7e32bd3..fef1dbe 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) } /* the task holds a reference to the nfs4_client struct */ - atomic_inc(&clp->cl_count); + get_nfs4_client(clp); do_probe_callback(clp); } diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 6dbcaf1..50b75af 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, out: /* Hold a session reference until done processing the compound. */ - if (cstate->session) + if (cstate->session) { nfsd4_get_session(cstate->session); + get_nfs4_client(cstate->session->se_client); + } spin_unlock(&sessionid_lock); /* Renew the clientid on success and on replay */ if (cstate->session) { diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 19ff5a3..aed733c 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); cs->slot->sl_inuse = false; } + put_nfs4_client(cs->session->se_client); nfsd4_put_session(cs->session); } return 1; diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index fefeae2..e3c002e 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -231,6 +231,12 @@ struct nfs4_client { /* wait here for slots */ }; +static inline void +get_nfs4_client(struct nfs4_client *clp) +{ + atomic_inc(&clp->cl_count); +} + /* struct nfs4_client_reset * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl * upon lease reset, or from upcall to state_daemon (to read in state -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-03 16:31 ` [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session Benny Halevy @ 2010-05-03 22:36 ` J. Bruce Fields 2010-05-04 1:12 ` J. Bruce Fields 0 siblings, 1 reply; 12+ messages in thread From: J. Bruce Fields @ 2010-05-03 22:36 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: > Although expire_client unhashes the session form the session table > so no new compounds can find it, there's no refcount to keep the > nfs4_client structure around while it's in use and referenced > in the compound state via the session structure. The code in my for-2.6.35 branch already has the cl_count removed (and doesn't use it for callbacks any more, instead destroying callbacks before the client is destroyed). So we need to add a new usage count. I'd prefer to call it something different, since it's being used for something different (cl_users?) since it's being used for something different, and I'd rather avoid confusion with the previous one. --b. > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > --- > fs/nfsd/nfs4callback.c | 2 +- > fs/nfsd/nfs4state.c | 4 +++- > fs/nfsd/nfs4xdr.c | 1 + > fs/nfsd/state.h | 6 ++++++ > 4 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index 7e32bd3..fef1dbe 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) > } > > /* the task holds a reference to the nfs4_client struct */ > - atomic_inc(&clp->cl_count); > + get_nfs4_client(clp); > > do_probe_callback(clp); > } > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 6dbcaf1..50b75af 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > > out: > /* Hold a session reference until done processing the compound. */ > - if (cstate->session) > + if (cstate->session) { > nfsd4_get_session(cstate->session); > + get_nfs4_client(cstate->session->se_client); > + } > spin_unlock(&sessionid_lock); > /* Renew the clientid on success and on replay */ > if (cstate->session) { > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index 19ff5a3..aed733c 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > cs->slot->sl_inuse = false; > } > + put_nfs4_client(cs->session->se_client); > nfsd4_put_session(cs->session); > } > return 1; > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index fefeae2..e3c002e 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -231,6 +231,12 @@ struct nfs4_client { > /* wait here for slots */ > }; > > +static inline void > +get_nfs4_client(struct nfs4_client *clp) > +{ > + atomic_inc(&clp->cl_count); > +} > + > /* struct nfs4_client_reset > * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl > * upon lease reset, or from upcall to state_daemon (to read in state > -- > 1.6.3.3 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-03 22:36 ` J. Bruce Fields @ 2010-05-04 1:12 ` J. Bruce Fields 2010-05-04 7:11 ` Benny Halevy 0 siblings, 1 reply; 12+ messages in thread From: J. Bruce Fields @ 2010-05-04 1:12 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Mon, May 03, 2010 at 06:36:20PM -0400, J. Bruce Fields wrote: > On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: > > Although expire_client unhashes the session form the session table > > so no new compounds can find it, there's no refcount to keep the > > nfs4_client structure around while it's in use and referenced > > in the compound state via the session structure. > > The code in my for-2.6.35 branch already has the cl_count removed (and > doesn't use it for callbacks any more, instead destroying callbacks > before the client is destroyed). > > So we need to add a new usage count. (Which you do in the next patch, OK!) --b. > I'd prefer to call it something > different, since it's being used for something different (cl_users?) > since it's being used for something different, and I'd rather avoid > confusion with the previous one. > > --b. > > > > > > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > > --- > > fs/nfsd/nfs4callback.c | 2 +- > > fs/nfsd/nfs4state.c | 4 +++- > > fs/nfsd/nfs4xdr.c | 1 + > > fs/nfsd/state.h | 6 ++++++ > > 4 files changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > > index 7e32bd3..fef1dbe 100644 > > --- a/fs/nfsd/nfs4callback.c > > +++ b/fs/nfsd/nfs4callback.c > > @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) > > } > > > > /* the task holds a reference to the nfs4_client struct */ > > - atomic_inc(&clp->cl_count); > > + get_nfs4_client(clp); > > > > do_probe_callback(clp); > > } > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index 6dbcaf1..50b75af 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > > > > out: > > /* Hold a session reference until done processing the compound. */ > > - if (cstate->session) > > + if (cstate->session) { > > nfsd4_get_session(cstate->session); > > + get_nfs4_client(cstate->session->se_client); > > + } > > spin_unlock(&sessionid_lock); > > /* Renew the clientid on success and on replay */ > > if (cstate->session) { > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index 19ff5a3..aed733c 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > > dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > > cs->slot->sl_inuse = false; > > } > > + put_nfs4_client(cs->session->se_client); > > nfsd4_put_session(cs->session); > > } > > return 1; > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > > index fefeae2..e3c002e 100644 > > --- a/fs/nfsd/state.h > > +++ b/fs/nfsd/state.h > > @@ -231,6 +231,12 @@ struct nfs4_client { > > /* wait here for slots */ > > }; > > > > +static inline void > > +get_nfs4_client(struct nfs4_client *clp) > > +{ > > + atomic_inc(&clp->cl_count); > > +} > > + > > /* struct nfs4_client_reset > > * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl > > * upon lease reset, or from upcall to state_daemon (to read in state > > -- > > 1.6.3.3 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-04 1:12 ` J. Bruce Fields @ 2010-05-04 7:11 ` Benny Halevy 2010-05-04 15:40 ` J. Bruce Fields 2010-05-04 17:45 ` J. Bruce Fields 0 siblings, 2 replies; 12+ messages in thread From: Benny Halevy @ 2010-05-04 7:11 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs On May. 04, 2010, 4:12 +0300, " J. Bruce Fields" <bfields@citi.umich.edu> wrote: > On Mon, May 03, 2010 at 06:36:20PM -0400, J. Bruce Fields wrote: >> On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: >>> Although expire_client unhashes the session form the session table >>> so no new compounds can find it, there's no refcount to keep the >>> nfs4_client structure around while it's in use and referenced >>> in the compound state via the session structure. >> The code in my for-2.6.35 branch already has the cl_count removed (and >> doesn't use it for callbacks any more, instead destroying callbacks >> before the client is destroyed). >> >> So we need to add a new usage count. > > (Which you do in the next patch, OK!) Right :) This patch fixes another race in which a client can be expired using expire_client(), not from the laundromat path, while it's being referenced by the session since we look up the session while holding just the sessionid lock and not the state lock. Therefore we must take a refcount on the client while inside the sessionid lock. Looks like the new usage count in your for-2.6.35 world (that I _think_ could just be a kref now) is needed for delegations as well, right? Benny > > --b. > >> I'd prefer to call it something >> different, since it's being used for something different (cl_users?) >> since it's being used for something different, and I'd rather avoid >> confusion with the previous one. >> >> --b. >> >> >> >>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>> --- >>> fs/nfsd/nfs4callback.c | 2 +- >>> fs/nfsd/nfs4state.c | 4 +++- >>> fs/nfsd/nfs4xdr.c | 1 + >>> fs/nfsd/state.h | 6 ++++++ >>> 4 files changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c >>> index 7e32bd3..fef1dbe 100644 >>> --- a/fs/nfsd/nfs4callback.c >>> +++ b/fs/nfsd/nfs4callback.c >>> @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) >>> } >>> >>> /* the task holds a reference to the nfs4_client struct */ >>> - atomic_inc(&clp->cl_count); >>> + get_nfs4_client(clp); >>> >>> do_probe_callback(clp); >>> } >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index 6dbcaf1..50b75af 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, >>> >>> out: >>> /* Hold a session reference until done processing the compound. */ >>> - if (cstate->session) >>> + if (cstate->session) { >>> nfsd4_get_session(cstate->session); >>> + get_nfs4_client(cstate->session->se_client); >>> + } >>> spin_unlock(&sessionid_lock); >>> /* Renew the clientid on success and on replay */ >>> if (cstate->session) { >>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c >>> index 19ff5a3..aed733c 100644 >>> --- a/fs/nfsd/nfs4xdr.c >>> +++ b/fs/nfsd/nfs4xdr.c >>> @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo >>> dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); >>> cs->slot->sl_inuse = false; >>> } >>> + put_nfs4_client(cs->session->se_client); >>> nfsd4_put_session(cs->session); >>> } >>> return 1; >>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h >>> index fefeae2..e3c002e 100644 >>> --- a/fs/nfsd/state.h >>> +++ b/fs/nfsd/state.h >>> @@ -231,6 +231,12 @@ struct nfs4_client { >>> /* wait here for slots */ >>> }; >>> >>> +static inline void >>> +get_nfs4_client(struct nfs4_client *clp) >>> +{ >>> + atomic_inc(&clp->cl_count); >>> +} >>> + >>> /* struct nfs4_client_reset >>> * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl >>> * upon lease reset, or from upcall to state_daemon (to read in state >>> -- >>> 1.6.3.3 >>> > -- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-04 7:11 ` Benny Halevy @ 2010-05-04 15:40 ` J. Bruce Fields 2010-05-04 17:45 ` J. Bruce Fields 1 sibling, 0 replies; 12+ messages in thread From: J. Bruce Fields @ 2010-05-04 15:40 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Tue, May 04, 2010 at 10:11:40AM +0300, Benny Halevy wrote: > On May. 04, 2010, 4:12 +0300, " J. Bruce Fields" <bfields@citi.umich.edu> wrote: > > On Mon, May 03, 2010 at 06:36:20PM -0400, J. Bruce Fields wrote: > >> On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: > >>> Although expire_client unhashes the session form the session table > >>> so no new compounds can find it, there's no refcount to keep the > >>> nfs4_client structure around while it's in use and referenced > >>> in the compound state via the session structure. > >> The code in my for-2.6.35 branch already has the cl_count removed (and > >> doesn't use it for callbacks any more, instead destroying callbacks > >> before the client is destroyed). > >> > >> So we need to add a new usage count. > > > > (Which you do in the next patch, OK!) > > Right :) > > This patch fixes another race in which a client can be expired using > expire_client(), not from the laundromat path, while it's being referenced > by the session since we look up the session while holding just the sessionid > lock and not the state lock. Therefore we must take a refcount on the > client while inside the sessionid lock. > > Looks like the new usage count in your for-2.6.35 world (that I _think_ > could just be a kref now) is needed for delegations as well, right? We should be able to destroy any delegations, recalls, or other callbacks before we destroy the client, so I don't think we need a reference count for those. --b. > > Benny > > > > > --b. > > > >> I'd prefer to call it something > >> different, since it's being used for something different (cl_users?) > >> since it's being used for something different, and I'd rather avoid > >> confusion with the previous one. > >> > >> --b. > >> > >> > >> > >>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> > >>> --- > >>> fs/nfsd/nfs4callback.c | 2 +- > >>> fs/nfsd/nfs4state.c | 4 +++- > >>> fs/nfsd/nfs4xdr.c | 1 + > >>> fs/nfsd/state.h | 6 ++++++ > >>> 4 files changed, 11 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > >>> index 7e32bd3..fef1dbe 100644 > >>> --- a/fs/nfsd/nfs4callback.c > >>> +++ b/fs/nfsd/nfs4callback.c > >>> @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) > >>> } > >>> > >>> /* the task holds a reference to the nfs4_client struct */ > >>> - atomic_inc(&clp->cl_count); > >>> + get_nfs4_client(clp); > >>> > >>> do_probe_callback(clp); > >>> } > >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > >>> index 6dbcaf1..50b75af 100644 > >>> --- a/fs/nfsd/nfs4state.c > >>> +++ b/fs/nfsd/nfs4state.c > >>> @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > >>> > >>> out: > >>> /* Hold a session reference until done processing the compound. */ > >>> - if (cstate->session) > >>> + if (cstate->session) { > >>> nfsd4_get_session(cstate->session); > >>> + get_nfs4_client(cstate->session->se_client); > >>> + } > >>> spin_unlock(&sessionid_lock); > >>> /* Renew the clientid on success and on replay */ > >>> if (cstate->session) { > >>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > >>> index 19ff5a3..aed733c 100644 > >>> --- a/fs/nfsd/nfs4xdr.c > >>> +++ b/fs/nfsd/nfs4xdr.c > >>> @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > >>> dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > >>> cs->slot->sl_inuse = false; > >>> } > >>> + put_nfs4_client(cs->session->se_client); > >>> nfsd4_put_session(cs->session); > >>> } > >>> return 1; > >>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > >>> index fefeae2..e3c002e 100644 > >>> --- a/fs/nfsd/state.h > >>> +++ b/fs/nfsd/state.h > >>> @@ -231,6 +231,12 @@ struct nfs4_client { > >>> /* wait here for slots */ > >>> }; > >>> > >>> +static inline void > >>> +get_nfs4_client(struct nfs4_client *clp) > >>> +{ > >>> + atomic_inc(&clp->cl_count); > >>> +} > >>> + > >>> /* struct nfs4_client_reset > >>> * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl > >>> * upon lease reset, or from upcall to state_daemon (to read in state > >>> -- > >>> 1.6.3.3 > >>> > > -- > > 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-04 7:11 ` Benny Halevy 2010-05-04 15:40 ` J. Bruce Fields @ 2010-05-04 17:45 ` J. Bruce Fields 2010-05-04 21:40 ` Benny Halevy 1 sibling, 1 reply; 12+ messages in thread From: J. Bruce Fields @ 2010-05-04 17:45 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Tue, May 04, 2010 at 10:11:40AM +0300, Benny Halevy wrote: > On May. 04, 2010, 4:12 +0300, " J. Bruce Fields" <bfields@citi.umich.edu> wrote: > > On Mon, May 03, 2010 at 06:36:20PM -0400, J. Bruce Fields wrote: > >> On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: > >>> Although expire_client unhashes the session form the session table > >>> so no new compounds can find it, there's no refcount to keep the > >>> nfs4_client structure around while it's in use and referenced > >>> in the compound state via the session structure. > >> The code in my for-2.6.35 branch already has the cl_count removed (and > >> doesn't use it for callbacks any more, instead destroying callbacks > >> before the client is destroyed). > >> > >> So we need to add a new usage count. > > > > (Which you do in the next patch, OK!) > > Right :) > > This patch fixes another race in which a client can be expired using > expire_client(), not from the laundromat path, Ouch, OK, I'd forgotten that case. --b. > while it's being referenced > by the session since we look up the session while holding just the sessionid > lock and not the state lock. Therefore we must take a refcount on the > client while inside the sessionid lock. > > Looks like the new usage count in your for-2.6.35 world (that I _think_ > could just be a kref now) is needed for delegations as well, right? > > Benny > > > > > --b. > > > >> I'd prefer to call it something > >> different, since it's being used for something different (cl_users?) > >> since it's being used for something different, and I'd rather avoid > >> confusion with the previous one. > >> > >> --b. > >> > >> > >> > >>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> > >>> --- > >>> fs/nfsd/nfs4callback.c | 2 +- > >>> fs/nfsd/nfs4state.c | 4 +++- > >>> fs/nfsd/nfs4xdr.c | 1 + > >>> fs/nfsd/state.h | 6 ++++++ > >>> 4 files changed, 11 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > >>> index 7e32bd3..fef1dbe 100644 > >>> --- a/fs/nfsd/nfs4callback.c > >>> +++ b/fs/nfsd/nfs4callback.c > >>> @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) > >>> } > >>> > >>> /* the task holds a reference to the nfs4_client struct */ > >>> - atomic_inc(&clp->cl_count); > >>> + get_nfs4_client(clp); > >>> > >>> do_probe_callback(clp); > >>> } > >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > >>> index 6dbcaf1..50b75af 100644 > >>> --- a/fs/nfsd/nfs4state.c > >>> +++ b/fs/nfsd/nfs4state.c > >>> @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > >>> > >>> out: > >>> /* Hold a session reference until done processing the compound. */ > >>> - if (cstate->session) > >>> + if (cstate->session) { > >>> nfsd4_get_session(cstate->session); > >>> + get_nfs4_client(cstate->session->se_client); > >>> + } > >>> spin_unlock(&sessionid_lock); > >>> /* Renew the clientid on success and on replay */ > >>> if (cstate->session) { > >>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > >>> index 19ff5a3..aed733c 100644 > >>> --- a/fs/nfsd/nfs4xdr.c > >>> +++ b/fs/nfsd/nfs4xdr.c > >>> @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > >>> dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > >>> cs->slot->sl_inuse = false; > >>> } > >>> + put_nfs4_client(cs->session->se_client); > >>> nfsd4_put_session(cs->session); > >>> } > >>> return 1; > >>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > >>> index fefeae2..e3c002e 100644 > >>> --- a/fs/nfsd/state.h > >>> +++ b/fs/nfsd/state.h > >>> @@ -231,6 +231,12 @@ struct nfs4_client { > >>> /* wait here for slots */ > >>> }; > >>> > >>> +static inline void > >>> +get_nfs4_client(struct nfs4_client *clp) > >>> +{ > >>> + atomic_inc(&clp->cl_count); > >>> +} > >>> + > >>> /* struct nfs4_client_reset > >>> * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl > >>> * upon lease reset, or from upcall to state_daemon (to read in state > >>> -- > >>> 1.6.3.3 > >>> > > -- > > 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session 2010-05-04 17:45 ` J. Bruce Fields @ 2010-05-04 21:40 ` Benny Halevy 0 siblings, 0 replies; 12+ messages in thread From: Benny Halevy @ 2010-05-04 21:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs On May. 04, 2010, 20:45 +0300, "J. Bruce Fields" <bfields@citi.umich.edu> wrote: > On Tue, May 04, 2010 at 10:11:40AM +0300, Benny Halevy wrote: >> On May. 04, 2010, 4:12 +0300, " J. Bruce Fields" <bfields@citi.umich.edu> wrote: >>> On Mon, May 03, 2010 at 06:36:20PM -0400, J. Bruce Fields wrote: >>>> On Mon, May 03, 2010 at 07:31:48PM +0300, Benny Halevy wrote: >>>>> Although expire_client unhashes the session form the session table >>>>> so no new compounds can find it, there's no refcount to keep the >>>>> nfs4_client structure around while it's in use and referenced >>>>> in the compound state via the session structure. >>>> The code in my for-2.6.35 branch already has the cl_count removed (and >>>> doesn't use it for callbacks any more, instead destroying callbacks >>>> before the client is destroyed). >>>> >>>> So we need to add a new usage count. >>> >>> (Which you do in the next patch, OK!) >> >> Right :) >> >> This patch fixes another race in which a client can be expired using >> expire_client(), not from the laundromat path, > > Ouch, OK, I'd forgotten that case. I'm working on a new version on top of your for-2.6.35 branch that will solve both cases using the new ref count. Benny > > --b. > >> while it's being referenced >> by the session since we look up the session while holding just the sessionid >> lock and not the state lock. Therefore we must take a refcount on the >> client while inside the sessionid lock. >> >> Looks like the new usage count in your for-2.6.35 world (that I _think_ >> could just be a kref now) is needed for delegations as well, right? >> >> Benny >> >>> >>> --b. >>> >>>> I'd prefer to call it something >>>> different, since it's being used for something different (cl_users?) >>>> since it's being used for something different, and I'd rather avoid >>>> confusion with the previous one. >>>> >>>> --b. >>>> >>>> >>>> >>>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>>> --- >>>>> fs/nfsd/nfs4callback.c | 2 +- >>>>> fs/nfsd/nfs4state.c | 4 +++- >>>>> fs/nfsd/nfs4xdr.c | 1 + >>>>> fs/nfsd/state.h | 6 ++++++ >>>>> 4 files changed, 11 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c >>>>> index 7e32bd3..fef1dbe 100644 >>>>> --- a/fs/nfsd/nfs4callback.c >>>>> +++ b/fs/nfsd/nfs4callback.c >>>>> @@ -571,7 +571,7 @@ nfsd4_probe_callback(struct nfs4_client *clp) >>>>> } >>>>> >>>>> /* the task holds a reference to the nfs4_client struct */ >>>>> - atomic_inc(&clp->cl_count); >>>>> + get_nfs4_client(clp); >>>>> >>>>> do_probe_callback(clp); >>>>> } >>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>>>> index 6dbcaf1..50b75af 100644 >>>>> --- a/fs/nfsd/nfs4state.c >>>>> +++ b/fs/nfsd/nfs4state.c >>>>> @@ -1458,8 +1458,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, >>>>> >>>>> out: >>>>> /* Hold a session reference until done processing the compound. */ >>>>> - if (cstate->session) >>>>> + if (cstate->session) { >>>>> nfsd4_get_session(cstate->session); >>>>> + get_nfs4_client(cstate->session->se_client); >>>>> + } >>>>> spin_unlock(&sessionid_lock); >>>>> /* Renew the clientid on success and on replay */ >>>>> if (cstate->session) { >>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c >>>>> index 19ff5a3..aed733c 100644 >>>>> --- a/fs/nfsd/nfs4xdr.c >>>>> +++ b/fs/nfsd/nfs4xdr.c >>>>> @@ -3313,6 +3313,7 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo >>>>> dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); >>>>> cs->slot->sl_inuse = false; >>>>> } >>>>> + put_nfs4_client(cs->session->se_client); >>>>> nfsd4_put_session(cs->session); >>>>> } >>>>> return 1; >>>>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h >>>>> index fefeae2..e3c002e 100644 >>>>> --- a/fs/nfsd/state.h >>>>> +++ b/fs/nfsd/state.h >>>>> @@ -231,6 +231,12 @@ struct nfs4_client { >>>>> /* wait here for slots */ >>>>> }; >>>>> >>>>> +static inline void >>>>> +get_nfs4_client(struct nfs4_client *clp) >>>>> +{ >>>>> + atomic_inc(&clp->cl_count); >>>>> +} >>>>> + >>>>> /* struct nfs4_client_reset >>>>> * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl >>>>> * upon lease reset, or from upcall to state_daemon (to read in state >>>>> -- >>>>> 1.6.3.3 >>>>> >>> -- >>> 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 >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC 3/3] nfsd4: do not expire nfs41 clients while in use 2010-05-03 16:13 [RFC 0/3] nfsd41: do not expire client while in use by current compound Benny Halevy 2010-05-03 16:31 ` [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres Benny Halevy 2010-05-03 16:31 ` [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session Benny Halevy @ 2010-05-03 16:31 ` Benny Halevy 2010-05-04 5:39 ` Benny Halevy 2 siblings, 1 reply; 12+ messages in thread From: Benny Halevy @ 2010-05-03 16:31 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, Benny Halevy Add a cl_use_count atomic counter to struct nfs4_client. Hold a use count while the client is in use by a compound that begins with SEQUENCE. Renew the client when the comound completes. The laundromat skips clients that have a positive use count. Otherwise, the laundromat marks the client as expired by setting cl_use_count to -1. Note that we don't want to use the state lock to synchronize with nfsd4_sequence as we want to diminish the state lock scope. An alternative to using atomic_cmpxchg is to grab the sessionid_lock spin lock while accessing cl_use_count, but that would cause some contention in the laundromat if it needs to lock and unlock it for every client. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfsd/nfs4state.c | 40 ++++++++++++++++++++++++++++++++++------ fs/nfsd/nfs4xdr.c | 2 ++ fs/nfsd/state.h | 9 +++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 50b75af..c95ddca 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -647,6 +647,14 @@ renew_client(struct nfs4_client *clp) clp->cl_time = get_seconds(); } +void +renew_nfs4_client(struct nfs4_client *clp) +{ + nfs4_lock_state(); + renew_client(clp); + nfs4_unlock_state(); +} + /* SETCLIENTID and SETCLIENTID_CONFIRM Helper functions */ static int STALE_CLIENTID(clientid_t *clid) @@ -715,6 +723,24 @@ put_nfs4_client(struct nfs4_client *clp) free_client(clp); } +/* + * atomically mark client as used, as long as it's not already expired. + * return 0 on success, or a negative value if client already expired. + */ +int +use_nfs4_client(struct nfs4_client *clp) +{ + int orig; + + do { + orig = atomic_read(&clp->cl_use_count); + if (orig < 0) + return orig; + } while (atomic_cmpxchg(&clp->cl_use_count, orig, orig + 1) != orig); + + return 0; +} + static void expire_client(struct nfs4_client *clp) { @@ -840,6 +866,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir, memcpy(clp->cl_recdir, recdir, HEXDIR_LEN); atomic_set(&clp->cl_count, 1); + atomic_set(&clp->cl_use_count, 0); atomic_set(&clp->cl_cb_conn.cb_set, 0); INIT_LIST_HEAD(&clp->cl_idhash); INIT_LIST_HEAD(&clp->cl_strhash); @@ -1423,6 +1450,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, if (!session) goto out; + /* keep the client from expiring */ + if (use_nfs4_client(cstate->session->se_client) < 0) + goto out; + status = nfserr_badslot; if (seq->slotid >= session->se_fchannel.maxreqs) goto out; @@ -1463,12 +1494,6 @@ out: get_nfs4_client(cstate->session->se_client); } spin_unlock(&sessionid_lock); - /* Renew the clientid on success and on replay */ - if (cstate->session) { - nfs4_lock_state(); - renew_client(session->se_client); - nfs4_unlock_state(); - } dprintk("%s: return %d\n", __func__, ntohl(status)); return status; } @@ -2575,6 +2600,9 @@ nfs4_laundromat(void) nfsd4_end_grace(); list_for_each_safe(pos, next, &client_lru) { clp = list_entry(pos, struct nfs4_client, cl_lru); + /* skip client if in use (cl_use_count is greater than zero) */ + if (atomic_cmpxchg(&clp->cl_use_count, 0, -1) > 0) + continue; if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) { t = clp->cl_time - cutoff; if (clientid_val > t) diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index aed733c..0f0b857 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -3313,6 +3313,8 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); cs->slot->sl_inuse = false; } + renew_nfs4_client(cs->session->se_client); + unuse_nfs4_client(cs->session->se_client); put_nfs4_client(cs->session->se_client); nfsd4_put_session(cs->session); } diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index e3c002e..806d9b8 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -214,6 +214,7 @@ struct nfs4_client { nfs4_verifier cl_confirm; /* generated by server */ struct nfs4_cb_conn cl_cb_conn; /* callback info */ atomic_t cl_count; /* ref count */ + atomic_t cl_use_count; /* session use count */ u32 cl_firststate; /* recovery dir creation */ /* for nfs41 */ @@ -237,6 +238,12 @@ get_nfs4_client(struct nfs4_client *clp) atomic_inc(&clp->cl_count); } +static inline void +unuse_nfs4_client(struct nfs4_client *clp) +{ + atomic_dec(&clp->cl_use_count); +} + /* struct nfs4_client_reset * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl * upon lease reset, or from upcall to state_daemon (to read in state @@ -384,6 +391,8 @@ extern void nfs4_unlock_state(void); extern int nfs4_in_grace(void); extern __be32 nfs4_check_open_reclaim(clientid_t *clid); extern void put_nfs4_client(struct nfs4_client *clp); +extern int use_nfs4_client(struct nfs4_client *clp); +extern void renew_nfs4_client(struct nfs4_client *clp); extern void nfs4_free_stateowner(struct kref *kref); extern int set_callback_cred(void); extern void nfsd4_probe_callback(struct nfs4_client *clp); -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC 3/3] nfsd4: do not expire nfs41 clients while in use 2010-05-03 16:31 ` [RFC 3/3] nfsd4: do not expire nfs41 clients while in use Benny Halevy @ 2010-05-04 5:39 ` Benny Halevy 0 siblings, 0 replies; 12+ messages in thread From: Benny Halevy @ 2010-05-04 5:39 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Benny Halevy wrote: > Add a cl_use_count atomic counter to struct nfs4_client. > Hold a use count while the client is in use by a compound that begins > with SEQUENCE. > Renew the client when the comound completes. > The laundromat skips clients that have a positive use count. > Otherwise, the laundromat marks the client as expired by setting > cl_use_count to -1. > > Note that we don't want to use the state lock to synchronize with nfsd4_sequence > as we want to diminish the state lock scope. > An alternative to using atomic_cmpxchg is to grab the sessionid_lock spin lock > while accessing cl_use_count, but that would cause some contention in the > laundromat if it needs to lock and unlock it for every client. > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > --- > fs/nfsd/nfs4state.c | 40 ++++++++++++++++++++++++++++++++++------ > fs/nfsd/nfs4xdr.c | 2 ++ > fs/nfsd/state.h | 9 +++++++++ > 3 files changed, 45 insertions(+), 6 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 50b75af..c95ddca 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -647,6 +647,14 @@ renew_client(struct nfs4_client *clp) > clp->cl_time = get_seconds(); > } > > +void > +renew_nfs4_client(struct nfs4_client *clp) > +{ > + nfs4_lock_state(); > + renew_client(clp); > + nfs4_unlock_state(); > +} > + > /* SETCLIENTID and SETCLIENTID_CONFIRM Helper functions */ > static int > STALE_CLIENTID(clientid_t *clid) > @@ -715,6 +723,24 @@ put_nfs4_client(struct nfs4_client *clp) > free_client(clp); > } > > +/* > + * atomically mark client as used, as long as it's not already expired. > + * return 0 on success, or a negative value if client already expired. > + */ > +int > +use_nfs4_client(struct nfs4_client *clp) > +{ > + int orig; > + > + do { > + orig = atomic_read(&clp->cl_use_count); > + if (orig < 0) > + return orig; > + } while (atomic_cmpxchg(&clp->cl_use_count, orig, orig + 1) != orig); > + > + return 0; > +} > + > static void > expire_client(struct nfs4_client *clp) > { > @@ -840,6 +866,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir, > > memcpy(clp->cl_recdir, recdir, HEXDIR_LEN); > atomic_set(&clp->cl_count, 1); > + atomic_set(&clp->cl_use_count, 0); > atomic_set(&clp->cl_cb_conn.cb_set, 0); > INIT_LIST_HEAD(&clp->cl_idhash); > INIT_LIST_HEAD(&clp->cl_strhash); > @@ -1423,6 +1450,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > if (!session) > goto out; > > + /* keep the client from expiring */ > + if (use_nfs4_client(cstate->session->se_client) < 0) > + goto out; > + > status = nfserr_badslot; > if (seq->slotid >= session->se_fchannel.maxreqs) > goto out; > @@ -1463,12 +1494,6 @@ out: > get_nfs4_client(cstate->session->se_client); > } > spin_unlock(&sessionid_lock); > - /* Renew the clientid on success and on replay */ > - if (cstate->session) { > - nfs4_lock_state(); > - renew_client(session->se_client); > - nfs4_unlock_state(); > - } > dprintk("%s: return %d\n", __func__, ntohl(status)); > return status; > } > @@ -2575,6 +2600,9 @@ nfs4_laundromat(void) > nfsd4_end_grace(); > list_for_each_safe(pos, next, &client_lru) { > clp = list_entry(pos, struct nfs4_client, cl_lru); > + /* skip client if in use (cl_use_count is greater than zero) */ > + if (atomic_cmpxchg(&clp->cl_use_count, 0, -1) > 0) > + continue; I'll move that check after the time comparison since there's no need for the atomic instruction if the client's time out hasn't expired yet. Benny > if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) { > t = clp->cl_time - cutoff; > if (clientid_val > t) > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index aed733c..0f0b857 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -3313,6 +3313,8 @@ nfs4svc_encode_compoundres(struct svc_rqst *rqstp, __be32 *p, struct nfsd4_compo > dprintk("%s: SET SLOT STATE TO AVAILABLE\n", __func__); > cs->slot->sl_inuse = false; > } > + renew_nfs4_client(cs->session->se_client); > + unuse_nfs4_client(cs->session->se_client); > put_nfs4_client(cs->session->se_client); > nfsd4_put_session(cs->session); > } > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index e3c002e..806d9b8 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -214,6 +214,7 @@ struct nfs4_client { > nfs4_verifier cl_confirm; /* generated by server */ > struct nfs4_cb_conn cl_cb_conn; /* callback info */ > atomic_t cl_count; /* ref count */ > + atomic_t cl_use_count; /* session use count */ > u32 cl_firststate; /* recovery dir creation */ > > /* for nfs41 */ > @@ -237,6 +238,12 @@ get_nfs4_client(struct nfs4_client *clp) > atomic_inc(&clp->cl_count); > } > > +static inline void > +unuse_nfs4_client(struct nfs4_client *clp) > +{ > + atomic_dec(&clp->cl_use_count); > +} > + > /* struct nfs4_client_reset > * one per old client. Populates reset_str_hashtbl. Filled from conf_id_hashtbl > * upon lease reset, or from upcall to state_daemon (to read in state > @@ -384,6 +391,8 @@ extern void nfs4_unlock_state(void); > extern int nfs4_in_grace(void); > extern __be32 nfs4_check_open_reclaim(clientid_t *clid); > extern void put_nfs4_client(struct nfs4_client *clp); > +extern int use_nfs4_client(struct nfs4_client *clp); > +extern void renew_nfs4_client(struct nfs4_client *clp); > extern void nfs4_free_stateowner(struct kref *kref); > extern int set_callback_cred(void); > extern void nfsd4_probe_callback(struct nfs4_client *clp); ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-05-04 21:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-03 16:13 [RFC 0/3] nfsd41: do not expire client while in use by current compound Benny Halevy 2010-05-03 16:31 ` [RFC 1/3] nfsd4: use local variable in nfs4svc_encode_compoundres Benny Halevy 2010-05-03 22:37 ` J. Bruce Fields 2010-05-03 16:31 ` [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session Benny Halevy 2010-05-03 22:36 ` J. Bruce Fields 2010-05-04 1:12 ` J. Bruce Fields 2010-05-04 7:11 ` Benny Halevy 2010-05-04 15:40 ` J. Bruce Fields 2010-05-04 17:45 ` J. Bruce Fields 2010-05-04 21:40 ` Benny Halevy 2010-05-03 16:31 ` [RFC 3/3] nfsd4: do not expire nfs41 clients while in use Benny Halevy 2010-05-04 5:39 ` Benny Halevy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).