Re: [RFC 2/3] nfsd4: hold a reference on nfs4_client when it is used by a session

[" J. Bruce Fields" <bfields@citi.umich.edu>]

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
> >