Commit 8c62d127 authored by NeilBrown's avatar NeilBrown Committed by Chuck Lever
Browse files

SUNRPC/NFSD: clean up get/put functions. 



svc_destroy() is poorly named - it doesn't necessarily destroy the svc,
it might just reduce the ref count.
nfsd_destroy() is poorly named for the same reason.

This patch:
 - removes the refcount functionality from svc_destroy(), moving it to
   a new svc_put().  Almost all previous callers of svc_destroy() now
   call svc_put().
 - renames nfsd_destroy() to nfsd_put() and improves the code, using
   the new svc_destroy() rather than svc_put()
 - removes a few comments that explain the important for balanced
   get/put calls.  This should be obvious.

The only non-trivial part of this is that svc_destroy() would call
svc_sock_update() on a non-final decrement.  It can no longer do that,
and svc_put() isn't really a good place of it.  This call is now made
from svc_exit_thread() which seems like a good place.  This makes the
call *before* sv_nrthreads is decremented rather than after.  This
is not particularly important as the call just sets a flag which
causes sv_nrthreads set be checked later.  A subsequent patch will
improve the ordering.

Signed-off-by: default avatarNeilBrown <neilb@suse.de>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent df5e49c8

fs/lockd/svc.c

+1 −5
Original line number Diff line number Diff line
@@ -431,10 +431,6 @@ static struct svc_serv *lockd_create_svc(void) 
	 * Check whether we're already up and running.

	 */

	if (nlmsvc_rqst)

		/*

		 * Note: increase service usage, because later in case of error

		 * svc_destroy() will be called.

		 */

		return svc_get(nlmsvc_rqst->rq_server);



	/*
@@ -495,7 +491,7 @@ int lockd_up(struct net *net, const struct cred *cred) 
	 * so we exit through here on both success and failure.

	 */

err_put:

	svc_destroy(serv);

	svc_put(serv);

err_create:

	mutex_unlock(&nlmsvc_mutex);

	return error;

fs/nfs/callback.c

+2 −12
Original line number Diff line number Diff line
@@ -267,10 +267,6 @@ static struct svc_serv *nfs_callback_create_svc(int minorversion) 
	 * Check whether we're already up and running.

	 */

	if (cb_info->serv)

		/*

		 * Note: increase service usage, because later in case of error

		 * svc_destroy() will be called.

		 */

		return svc_get(cb_info->serv);



	switch (minorversion) {
@@ -333,16 +329,10 @@ int nfs_callback_up(u32 minorversion, struct rpc_xprt *xprt) 
		goto err_start;



	cb_info->users++;

	/*

	 * svc_create creates the svc_serv with sv_nrthreads == 1, and then

	 * svc_prepare_thread increments that. So we need to call svc_destroy

	 * on both success and failure so that the refcount is 1 when the

	 * thread exits.

	 */

err_net:

	if (!cb_info->users)

		cb_info->serv = NULL;

	svc_destroy(serv);

	svc_put(serv);

err_create:

	mutex_unlock(&nfs_callback_mutex);

	return ret;
@@ -368,7 +358,7 @@ void nfs_callback_down(int minorversion, struct net *net) 
	if (cb_info->users == 0) {

		svc_get(serv);

		serv->sv_ops->svo_setup(serv, NULL, 0);

		svc_destroy(serv);

		svc_put(serv);

		dprintk("nfs_callback_down: service destroyed\n");

		cb_info->serv = NULL;

	}

fs/nfsd/nfsctl.c

+2 −2
Original line number Diff line number Diff line
@@ -743,7 +743,7 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred 


	err = svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);

	if (err < 0 && list_empty(&nn->nfsd_serv->sv_permsocks)) {

		nfsd_destroy(net);

		nfsd_put(net);

		return err;

	}
@@ -796,7 +796,7 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr 
	if (!list_empty(&nn->nfsd_serv->sv_permsocks))

		nn->nfsd_serv->sv_nrthreads--;

	 else

		nfsd_destroy(net);

		nfsd_put(net);

	return err;

}

fs/nfsd/nfsd.h

+1 −1
Original line number Diff line number Diff line
@@ -97,7 +97,7 @@ int nfsd_pool_stats_open(struct inode *, struct file *); 
int		nfsd_pool_stats_release(struct inode *, struct file *);

void		nfsd_shutdown_threads(struct net *net);



void		nfsd_destroy(struct net *net);

void		nfsd_put(struct net *net);



bool		i_am_nfsd(void);

fs/nfsd/nfssvc.c

+16 −14
Original line number Diff line number Diff line
@@ -623,7 +623,7 @@ void nfsd_shutdown_threads(struct net *net) 
	svc_get(serv);

	/* Kill outstanding nfsd threads */

	serv->sv_ops->svo_setup(serv, NULL, 0);

	nfsd_destroy(net);

	nfsd_put(net);

	mutex_unlock(&nfsd_mutex);

	/* Wait for shutdown of nfsd_serv to complete */

	wait_for_completion(&nn->nfsd_shutdown_complete);
@@ -656,7 +656,10 @@ int nfsd_create_serv(struct net *net) 
	nn->nfsd_serv->sv_maxconn = nn->max_connections;

	error = svc_bind(nn->nfsd_serv, net);

	if (error < 0) {

		svc_destroy(nn->nfsd_serv);

		/* NOT nfsd_put() as notifiers (see below) haven't

		 * been set up yet.

		 */

		svc_put(nn->nfsd_serv);

		nfsd_complete_shutdown(net);

		return error;

	}
@@ -697,17 +700,17 @@ int nfsd_get_nrthreads(int n, int *nthreads, struct net *net) 
	return 0;

}



void nfsd_destroy(struct net *net)

void nfsd_put(struct net *net)

{

	struct nfsd_net *nn = net_generic(net, nfsd_net_id);

	int destroy = (nn->nfsd_serv->sv_nrthreads == 1);



	if (destroy)

	nn->nfsd_serv->sv_nrthreads -= 1;

	if (nn->nfsd_serv->sv_nrthreads == 0) {

		svc_shutdown_net(nn->nfsd_serv, net);

		svc_destroy(nn->nfsd_serv);

	if (destroy)

		nfsd_complete_shutdown(net);

	}

}



int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)

{
@@ -758,7 +761,7 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net) 
		if (err)

			break;

	}

	nfsd_destroy(net);

	nfsd_put(net);

	return err;

}
@@ -795,7 +798,7 @@ nfsd_svc(int nrservs, struct net *net, const struct cred *cred) 


	error = nfsd_startup_net(net, cred);

	if (error)

		goto out_destroy;

		goto out_put;

	error = nn->nfsd_serv->sv_ops->svo_setup(nn->nfsd_serv,

			NULL, nrservs);

	if (error)
@@ -808,8 +811,8 @@ nfsd_svc(int nrservs, struct net *net, const struct cred *cred) 
out_shutdown:

	if (error < 0 && !nfsd_up_before)

		nfsd_shutdown_net(net);

out_destroy:

	nfsd_destroy(net);		/* Release server */

out_put:

	nfsd_put(net);

out:

	mutex_unlock(&nfsd_mutex);

	return error;
@@ -982,7 +985,7 @@ nfsd(void *vrqstp) 
	/* Release the thread */

	svc_exit_thread(rqstp);



	nfsd_destroy(net);

	nfsd_put(net);



	/* Release module */

	mutex_unlock(&nfsd_mutex);
@@ -1109,8 +1112,7 @@ int nfsd_pool_stats_release(struct inode *inode, struct file *file) 
	struct net *net = inode->i_sb->s_fs_info;



	mutex_lock(&nfsd_mutex);

	/* this function really, really should have been called svc_put() */

	nfsd_destroy(net);

	nfsd_put(net);

	mutex_unlock(&nfsd_mutex);

	return ret;

}