[PATCH] nfs4_client ref count for cb_recall

Benny Halevy bhalevy at ns1.bhalevy.com
Mon May 14 09:46:01 EDT 2007


This patch changes the client reference counting technique
to struct kref, moves shutting down of the callback rpc_client
from expire_client time to free_nfs4_client time that's called
when the nfs4_client refcount goes to zero, adds a refcount
to the nfs4_client around nfs4_cb_recall to keep the rcp_client
around throughout the recall (otherwise it could have been used
after free), and finally it adds some BUG_ONs to assert that the
state lock is taken when taking or releasing reference for nfs4_file,
nfs4_client, and nfs4_delegation to ensure atomicity.
---
 fs/nfsd/nfs4callback.c     |    7 ++++-
 fs/nfsd/nfs4state.c        |   51 +++++++++++++++++++++++++++++++++++--------
 include/linux/nfsd/state.h |    3 +-
 net/sunrpc/sunrpc_syms.c   |    1 +
 4 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 32ffea0..65401da 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -433,7 +433,7 @@ nfsd4_probe_callback(struct nfs4_client *clp)
 		goto out_clnt;
 
 	/* the task holds a reference to the nfs4_client struct */
-	atomic_inc(&clp->cl_count);
+	get_nfs4_client(clp);
 
 	msg.rpc_cred = nfsd4_lookupcred(clp,0);
 	if (IS_ERR(msg.rpc_cred))
@@ -448,7 +448,7 @@ nfsd4_probe_callback(struct nfs4_client *clp)
 	return;
 
 out_rpciod:
-	atomic_dec(&clp->cl_count);
+	put_nfs4_client(clp);
 	rpciod_down();
 out_clnt:
 	rpc_shutdown_client(cb->cb_client);
@@ -533,6 +533,9 @@ out:
 	/* Success or failure, now we're either waiting for lease expiration
 	 * or deleg_return. */
 	dprintk("NFSD: nfs4_cb_recall: dp %p dl_flock %p dl_count %d\n",dp, dp->dl_flock, atomic_read(&dp->dl_count));
+	rpc_release_client(clnt);
+	nfs4_lock_state();
 	nfs4_put_delegation(dp);
+	nfs4_unlock_state();
 	return;
 }
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3cc8ce4..09bf029 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -90,6 +90,8 @@ static struct kmem_cache *file_slab = NULL;
 static struct kmem_cache *stateid_slab = NULL;
 static struct kmem_cache *deleg_slab = NULL;
 
+#define BUG_ON_UNLOCKED_STATE() BUG_ON(mutex_trylock(&client_mutex))
+
 void
 nfs4_lock_state(void)
 {
@@ -139,6 +141,7 @@ free_nfs4_file(struct kref *kref)
 static inline void
 put_nfs4_file(struct nfs4_file *fi)
 {
+	BUG_ON_UNLOCKED_STATE();
 	kref_put(&fi->fi_ref, free_nfs4_file);
 }
 
@@ -353,9 +356,22 @@ alloc_client(struct xdr_netobj name)
 	return clp;
 }
 
-static inline void
-free_client(struct nfs4_client *clp)
+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);
+	BUG_ON(!list_empty(&clp->cl_idhash));
+	BUG_ON(!list_empty(&clp->cl_strhash));
+	BUG_ON(!list_empty(&clp->cl_lru));
+	BUG_ON(!list_empty(&clp->cl_delegations));
+	BUG_ON(!list_empty(&clp->cl_openowners));
 	if (clp->cl_cred.cr_group_info)
 		put_group_info(clp->cl_cred.cr_group_info);
 	kfree(clp->cl_name.data);
@@ -365,8 +381,7 @@ free_client(struct nfs4_client *clp)
 void
 put_nfs4_client(struct nfs4_client *clp)
 {
-	if (atomic_dec_and_test(&clp->cl_count))
-		free_client(clp);
+	kref_put(&clp->cl_ref, free_nfs4_client);
 }
 
 static void
@@ -389,9 +404,10 @@ expire_client(struct nfs4_client *clp)
 	struct nfs4_delegation *dp;
 	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));
 
+	BUG_ON_UNLOCKED_STATE();
 	shutdown_callback_client(clp);
 
 	INIT_LIST_HEAD(&reaplist);
@@ -409,9 +425,9 @@ expire_client(struct nfs4_client *clp)
 		list_del_init(&dp->dl_recall_lru);
 		unhash_delegation(dp);
 	}
-	list_del(&clp->cl_idhash);
-	list_del(&clp->cl_strhash);
-	list_del(&clp->cl_lru);
+	list_del_init(&clp->cl_idhash);
+	list_del_init(&clp->cl_strhash);
+	list_del_init(&clp->cl_lru);
 	while (!list_empty(&clp->cl_openowners)) {
 		sop = list_entry(clp->cl_openowners.next, struct nfs4_stateowner, so_perclient);
 		release_stateowner(sop);
@@ -426,7 +442,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);
@@ -1213,6 +1229,7 @@ find_openstateowner_str(unsigned int hashval, struct nfsd4_open *open)
 {
 	struct nfs4_stateowner *so = NULL;
 
+	BUG_ON_UNLOCKED_STATE();
 	list_for_each_entry(so, &ownerstr_hashtbl[hashval], so_strhash) {
 		if (cmp_owner_str(so, &open->op_owner, &open->op_clientid))
 			return so;
@@ -1227,6 +1244,7 @@ find_file(struct inode *ino)
 	unsigned int hashval = file_hashval(ino);
 	struct nfs4_file *fp;
 
+	BUG_ON_UNLOCKED_STATE();
 	list_for_each_entry(fp, &file_hashtbl[hashval], fi_hash) {
 		if (fp->fi_inode == ino) {
 			get_nfs4_file(fp);
@@ -1342,12 +1360,22 @@ static
 void nfsd_break_deleg_cb(struct file_lock *fl)
 {
 	struct nfs4_delegation *dp=  (struct nfs4_delegation *)fl->fl_owner;
+	struct rpc_clnt *clnt;
 	struct task_struct *t;
 
 	dprintk("NFSD nfsd_break_deleg_cb: dp %p fl %p\n",dp,fl);
 	if (!dp)
 		return;
 
+	nfs4_lock_state();
+	clnt = dp->dl_client->cl_callback.cb_client;
+	if (!atomic_read(&dp->dl_client->cl_callback.cb_set) || !clnt) {
+		nfs4_unlock_state();
+		return;
+	}
+	atomic_inc(&clnt->cl_users);
+	nfs4_unlock_state();
+
 	/* We're assuming the state code never drops its reference
 	 * without first removing the lease.  Since we're in this lease
 	 * callback (and since the lease code is serialized by the kernel
@@ -1372,7 +1400,10 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
 		printk(KERN_INFO "NFSD: Callback thread failed for "
 			"for client (clientid %08x/%08x)\n",
 			clp->cl_clientid.cl_boot, clp->cl_clientid.cl_id);
+		nfs4_lock_state();
+		rpc_release_client(clnt);
 		nfs4_put_delegation(dp);
+		nfs4_unlock_state();
 	}
 }
 
diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
index ab5c236..84463a3 100644
--- a/include/linux/nfsd/state.h
+++ b/include/linux/nfsd/state.h
@@ -116,6 +116,7 @@ struct nfs4_callback {
  *	  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;
@@ -130,7 +131,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 */
 };
 
@@ -279,6 +279,7 @@ extern void nfs4_lock_state(void);
 extern void nfs4_unlock_state(void);
 extern int nfs4_in_grace(void);
 extern __be32 nfs4_check_open_reclaim(clientid_t *clid);
+extern struct nfs4_client *get_nfs4_client(struct nfs4_client *clp);
 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);
diff --git a/net/sunrpc/sunrpc_syms.c b/net/sunrpc/sunrpc_syms.c
index 0d35bc7..b27addf 100644
--- a/net/sunrpc/sunrpc_syms.c
+++ b/net/sunrpc/sunrpc_syms.c
@@ -36,6 +36,7 @@ EXPORT_SYMBOL(rpc_wake_up_status);
 /* RPC client functions */
 EXPORT_SYMBOL(rpc_clone_client);
 EXPORT_SYMBOL(rpc_bind_new_program);
+EXPORT_SYMBOL(rpc_release_client);
 EXPORT_SYMBOL(rpc_destroy_client);
 EXPORT_SYMBOL(rpc_shutdown_client);
 EXPORT_SYMBOL(rpc_killall_tasks);
-- 
1.5.1


--------------000102080809090106060905--


More information about the pNFS mailing list