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: NeilBrown <neilb@suse.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 2f50d5b2a8a429aa9e62f2bb8a43a49fd06ecdf6..135bd86ed3adb4c95682ff34cbd9ab22341de54a 100644 (file)
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -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;

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 6e5e742a42b8bcd0b1d99dd63fcbf0431c5b5c38..edbc7579b4aae35ea559ed0848cd2dc8ce032b1a 100644 (file)
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -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;
        }

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index bf4c9996ad926fc2b0d5ef4ac599b640cf3a6160..17521fada83f6594279abf1433baff261f393b1e 100644 (file)
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -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 @@ out_err:
        if (!list_empty(&nn->nfsd_serv->sv_permsocks))
                nn->nfsd_serv->sv_nrthreads--;
         else
-               nfsd_destroy(net);
+               nfsd_put(net);
        return err;
 }
 

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 498e5a48982605d7638814b62bdabcf7d39c2a2d..3e5008b475ff0c5ccfca8f83981a9822c80ce712 100644 (file)
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -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);
 

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 80431921e5d79932127502b6feb1ab7f626e11e7..a0a7564e6c73e3552de9c47be048f2908dff93b9 100644 (file)
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -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,16 +700,16 @@ 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)
+               svc_destroy(nn->nfsd_serv);
                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 @@ out:
        /* 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;
 }

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 5d9568953fcd8e92624f7a4a422aec980dae70c9..73d56d33a36d9ecd3eceb54a17fd50ded2fb5812 100644 (file)
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -114,8 +114,13 @@ struct svc_serv {
 #endif /* CONFIG_SUNRPC_BACKCHANNEL */
 };
 
-/*
- * We use sv_nrthreads as a reference count.  svc_destroy() drops
+/**
+ * svc_get() - increment reference count on a SUNRPC serv
+ * @serv:  the svc_serv to have count incremented
+ *
+ * Returns: the svc_serv that was passed in.
+ *
+ * We use sv_nrthreads as a reference count.  svc_put() drops
  * this refcount, so we need to bump it up around operations that
  * change the number of threads.  Horrible, but there it is.
  * Should be called with the "service mutex" held.
@@ -126,6 +131,22 @@ static inline struct svc_serv *svc_get(struct svc_serv *serv)
        return serv;
 }
 
+void svc_destroy(struct svc_serv *serv);
+
+/**
+ * svc_put - decrement reference count on a SUNRPC serv
+ * @serv:  the svc_serv to have count decremented
+ *
+ * When the reference count reaches zero, svc_destroy()
+ * is called to clean up and free the serv.
+ */
+static inline void svc_put(struct svc_serv *serv)
+{
+       serv->sv_nrthreads -= 1;
+       if (serv->sv_nrthreads == 0)
+               svc_destroy(serv);
+}
+
 /*
  * Maximum payload size supported by a kernel RPC server.
  * This is use to determine the max number of pages nfsd is
@@ -515,7 +536,6 @@ struct svc_serv *  svc_create_pooled(struct svc_program *, unsigned int,
 int               svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
 int               svc_set_num_threads_sync(struct svc_serv *, struct svc_pool *, int);
 int               svc_pool_stats_open(struct svc_serv *serv, struct file *file);
-void              svc_destroy(struct svc_serv *);
 void              svc_shutdown_net(struct svc_serv *, struct net *);
 int               svc_process(struct svc_rqst *);
 int               bc_svc_process(struct svc_serv *, struct rpc_rqst *,

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 4292278a955263ffb602597370793139235ea443..55a1bf0d129f1bc9a9b005cd58b9a17648552ff8 100644 (file)
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -528,17 +528,7 @@ EXPORT_SYMBOL_GPL(svc_shutdown_net);
 void
 svc_destroy(struct svc_serv *serv)
 {
-       dprintk("svc: svc_destroy(%s, %d)\n",
-                               serv->sv_program->pg_name,
-                               serv->sv_nrthreads);
-
-       if (serv->sv_nrthreads) {
-               if (--(serv->sv_nrthreads) != 0) {
-                       svc_sock_update_bufs(serv);
-                       return;
-               }
-       } else
-               printk("svc_destroy: no threads for serv=%p!\n", serv);
+       dprintk("svc: svc_destroy(%s)\n", serv->sv_program->pg_name);
 
        del_timer_sync(&serv->sv_temptimer);
 
@@ -892,9 +882,10 @@ svc_exit_thread(struct svc_rqst *rqstp)
 
        svc_rqst_free(rqstp);
 
-       /* Release the server */
-       if (serv)
-               svc_destroy(serv);
+       if (!serv)
+               return;
+       svc_sock_update_bufs(serv);
+       svc_destroy(serv);
 }
 EXPORT_SYMBOL_GPL(svc_exit_thread);