[pnfs] [PATCH 4/12] use kref for nfsd nfs4_client

J. Bruce Fields bfields at fieldses.org
Mon May 7 18:06:12 EDT 2007


If this would also apply to mainline, I'd be happy to submit it, if it
came with a comment justifying the changes....  (Which are actually
two--it switches to kref, but also seems to make the reference counting
a little finer-grained?)

--b.

On Mon, May 07, 2007 at 10:34:34PM +0300, Benny Halevy wrote:
> 

> >From 514cd510ad1aa4bd7930eecb64cc731c1396e6e9 Mon Sep 17 00:00:00 2001
> From: Benny Halevy <bhalevy at ns1.bhalevy.com>
> Date: Mon, 7 May 2007 16:36:49 +0300
> Subject: [PATCH] use kref for nfsd nfs4_client
> 
> ---
>  fs/nfsd/nfs4callback.c     |    8 +------
>  fs/nfsd/nfs4state.c        |   50 +++++++++++++++++++++++++++++--------------
>  include/linux/nfsd/state.h |    3 +-
>  3 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index cd88b1e..c2f4732 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -520,9 +520,6 @@ nfsd4_probe_callback(struct nfs4_client *clp)
>  
>  	cb->cb_client = clnt;
>  
> -	/* the task holds a reference to the nfs4_client struct */
> -	atomic_inc(&clp->cl_count);
> -
>  	msg.rpc_cred = nfsd4_lookupcred(clp,0);
>  	if (IS_ERR(msg.rpc_cred))
>  		goto out_rpciod;
> @@ -536,7 +533,6 @@ nfsd4_probe_callback(struct nfs4_client *clp)
>  	return;
>  
>  out_rpciod:
> -	atomic_dec(&clp->cl_count);
>  	rpciod_down();
>  	cb->cb_client = NULL;
>  out_clnt:
> @@ -558,12 +554,10 @@ nfs4_cb_null(struct rpc_task *task, void *dummy)
>  	if (task->tk_status < 0) {
>  		dprintk("NFSD: callback establishment to client %.*s failed\n",
>  			(int)clp->cl_name.len, clp->cl_name.data);
> -		goto out;
> +		return;
>  	}
>  	atomic_set(&cb->cb_set, 1);
>  	dprintk("NFSD: callback set to client %u.%u.%u.%u\n", NIPQUAD(addr));
> -out:
> -	put_nfs4_client(clp);
>  }
>  
>  static const struct rpc_call_ops nfs4_cb_null_ops = {
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index e33faa9..d1355b2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -535,20 +535,28 @@ alloc_client(struct xdr_netobj name)
>  	return clp;
>  }
>  
> -static inline void
> -free_client(struct nfs4_client *clp)
> +static inline struct nfs4_client *
> +get_nfs4_client(struct nfs4_client *clp)
>  {
> +	kref_get(&clp->cl_ref);
> +	return clp;
> +}
> +
> +static void
> +free_nfs4_client(struct kref *kref)
> +{
> +	struct nfs4_client *clp = container_of(kref, struct nfs4_client, cl_ref);
>  	if (clp->cl_cred.cr_group_info)
>  		put_group_info(clp->cl_cred.cr_group_info);
>  	kfree(clp->cl_name.data);
>  	kfree(clp);
>  }
>  
> -void
> +static inline void
>  put_nfs4_client(struct nfs4_client *clp)
>  {
> -	if (atomic_dec_and_test(&clp->cl_count))
> -		free_client(clp);
> +	if (clp)
> +		kref_put(&clp->cl_ref, free_nfs4_client);
>  }
>  
>  static void
> @@ -573,8 +581,8 @@ expire_client(struct nfs4_client *clp)
>  	struct nfs41_session  *ses;
>  	struct list_head reaplist;
>  
> -	dprintk("NFSD: expire_client cl_count %d\n",
> -	                    atomic_read(&clp->cl_count));
> +	dprintk("NFSD: expire_client refcount %d\n",
> +	                    atomic_read(&clp->cl_ref.refcount));
>  
>  	shutdown_callback_client(clp);
>  
> @@ -632,7 +640,7 @@ create_client(struct xdr_netobj name, char *recdir) {
>  	if (!(clp = alloc_client(name)))
>  		goto out;
>  	memcpy(clp->cl_recdir, recdir, HEXDIR_LEN);
> -	atomic_set(&clp->cl_count, 1);
> +	kref_init(&clp->cl_ref);
>  	atomic_set(&clp->cl_callback.cb_set, 0);
>  	INIT_LIST_HEAD(&clp->cl_idhash);
>  	INIT_LIST_HEAD(&clp->cl_strhash);
> @@ -751,7 +759,7 @@ find_confirmed_client(clientid_t *clid)
>  
>  	list_for_each_entry(clp, &conf_id_hashtbl[idhashval], cl_idhash) {
>  		if (cmp_clid(&clp->cl_clientid, clid))
> -			return clp;
> +			return get_nfs4_client(clp);
>  	}
>  	return NULL;
>  }
> @@ -764,7 +772,7 @@ find_unconfirmed_client(clientid_t *clid)
>  
>  	list_for_each_entry(clp, &unconf_id_hashtbl[idhashval], cl_idhash) {
>  		if (cmp_clid(&clp->cl_clientid, clid))
> -			return clp;
> +			return get_nfs4_client(clp);
>  	}
>  	return NULL;
>  }
> @@ -776,7 +784,7 @@ find_confirmed_client_by_str(const char *dname, unsigned int hashval)
>  
>  	list_for_each_entry(clp, &conf_str_hashtbl[hashval], cl_strhash) {
>  		if (same_name(clp->cl_recdir, dname))
> -			return clp;
> +			return get_nfs4_client(clp);
>  	}
>  	return NULL;
>  }
> @@ -788,7 +796,7 @@ find_unconfirmed_client_by_str(const char *dname, unsigned int hashval)
>  
>  	list_for_each_entry(clp, &unconf_str_hashtbl[hashval], cl_strhash) {
>  		if (same_name(clp->cl_recdir, dname))
> -			return clp;
> +			return get_nfs4_client(clp);
>  	}
>  	return NULL;
>  }
> @@ -929,7 +937,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_setclientid *setclid)
>  	};
>  	nfs4_verifier		clverifier = setclid->se_verf;
>  	unsigned int 		strhashval;
> -	struct nfs4_client	*conf, *unconf, *new;
> +	struct nfs4_client	*conf = NULL, *unconf = NULL, *new;
>  	int 			status;
>  	char                    dname[HEXDIR_LEN];
>  	
> @@ -1073,6 +1081,8 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_setclientid *setclid)
>  	status = nfs_ok;
>  out:
>  	nfs4_unlock_state();
> +	put_nfs4_client(conf);
> +	put_nfs4_client(unconf);
>  	return status;
>  }
>  
> @@ -1083,7 +1093,7 @@ void nfsd4_setup_callback_channel(void)
>  
>  int nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_exchange_id *clid)
>  {
> -        struct nfs4_client *unconf, *conf, *new;
> +        struct nfs4_client *unconf = NULL, *conf = NULL, *new;
>          int status;
>          unsigned int            strhashval;
>          char                    dname[HEXDIR_LEN];
> @@ -1175,7 +1185,8 @@ out_copy:
>  
>  out:
>          nfs4_unlock_state();
> -
> +	put_nfs4_client(conf);
> +	put_nfs4_client(conf);
>          dprintk("nfsd4_exchange_id returns %d\n", status);
>          return status;
>  }
> @@ -1267,9 +1278,11 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp, struct nfsd4_setclientid_confi
>  			if (conf) {
>  				nfsd4_remove_clid_dir(conf);
>  				expire_client(conf);
> +				put_nfs4_client(conf);
>  			}
>  			move_to_confirmed(unconf);
>  			conf = unconf;
> +			unconf = NULL;
>  			status = nfs_ok;
>  		}
>  	} else if ((!conf || (conf && !cmp_verf(&conf->cl_confirm, &confirm)))
> @@ -1290,6 +1303,8 @@ out:
>  	if (!status)
>  		nfsd4_probe_callback(conf);
>  	nfs4_unlock_state();
> +	put_nfs4_client(conf);
> +	put_nfs4_client(unconf);
>  	return status;
>  }
>  
> @@ -1343,7 +1358,6 @@ int nfsd4_create_session(struct svc_rqst *rqstp, struct nfsd4_create_session *se
>  			dprintk("Got a create_session replay!\n");
>  			goto out_replay;
>  		}
> -
>          }
>          else if (unconf) {
>  		if (!cmp_creds(&unconf->cl_cred, &rqstp->rq_cred) || (ip_addr != unconf->cl_addr)) {
> @@ -1359,7 +1373,9 @@ int nfsd4_create_session(struct svc_rqst *rqstp, struct nfsd4_create_session *se
>  		unconf->cl_seqid++;
>  		move_to_confirmed(unconf);
>  		nfsd4_probe_callback(unconf);
> +		put_nfs4_client(conf);
>  		conf = unconf;
> +		unconf = NULL;
>  	}
>  	status = alloc_init_session(conf, session);
>  
> @@ -1375,6 +1391,8 @@ out_replay:
>  
>  out:
>  	nfs4_unlock_state();
> +	put_nfs4_client(conf);
> +	put_nfs4_client(unconf);
>  	dprintk("%s returns %d %d\n", __FUNCTION__, status, ntohl(status));
>  	return status;
>  }
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index 92feae0..c23ae98 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -193,6 +193,7 @@ struct current_session {
>   *	  when we expire the nfs4_client
>   */
>  struct nfs4_client {
> +	struct kref		cl_ref;
>  	struct list_head	cl_idhash; 	/* hash by cl_clientid.id */
>  	struct list_head	cl_strhash; 	/* hash by cl_name */
>  	struct list_head	cl_openowners;
> @@ -211,7 +212,6 @@ struct nfs4_client {
>  	clientid_t		cl_clientid;	/* generated by server */
>  	nfs4_verifier		cl_confirm;	/* generated by server */
>  	struct nfs4_callback	cl_callback;    /* callback info */
> -	atomic_t		cl_count;	/* ref count */
>  	u32			cl_firststate;	/* recovery dir creation */
>  		/* NFSv4.1 create_session slot */
>  	u32			cl_seqid;	/* seqid for create_session */
> @@ -428,7 +428,6 @@ extern void nfs4_lock_state(void);
>  extern void nfs4_unlock_state(void);
>  extern int nfs4_in_grace(void);
>  extern int nfs4_check_open_reclaim(clientid_t *clid);
> -extern void put_nfs4_client(struct nfs4_client *clp);
>  extern void nfs4_free_stateowner(struct kref *kref);
>  extern void nfsd4_probe_callback(struct nfs4_client *clp);
>  extern void nfsd4_cb_recall(struct nfs4_delegation *dp);
> -- 
> 1.5.1
> 

> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs


More information about the pNFS mailing list