SUNRPC: always treat sv_nrpools==1 as "not pooled"

Currently 'pooled' services hold a reference on the pool_map, and
'unpooled' services do not.
svc_destroy() uses the presence of ->svo_function (via
svc_serv_is_pooled()) to determine if the reference should be dropped.
There is no direct correlation between being pooled and the use of
svo_function, though in practice, lockd is the only non-pooled service,
and the only one not to use svo_function.

This is untidy and would cause problems if we changed lockd to use
svc_set_num_threads(), which requires the use of ->svo_function.

So change the test for "is the service pooled" to "is sv_nrpools > 1".

This means that when svc_pool_map_get() returns 1, it must NOT take a
reference to the pool.

We discard svc_serv_is_pooled(), and test sv_nrpools directly.

Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index f0dd9ef7e0cde755084fa9191b000562b80d728d..5fbe7f55289e1577b61e6035ba61da15701a2e6e 100644 (file)
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -37,8 +37,6 @@
 
 static void svc_unregister(const struct svc_serv *serv, struct net *net);
 
-#define svc_serv_is_pooled(serv)    ((serv)->sv_ops->svo_function)
-
 #define SVC_POOL_DEFAULT       SVC_POOL_GLOBAL
 
 /*
@@ -240,8 +238,10 @@ svc_pool_map_init_pernode(struct svc_pool_map *m)
 
 /*
  * Add a reference to the global map of cpus to pools (and
- * vice versa).  Initialise the map if we're the first user.
- * Returns the number of pools.
+ * vice versa) if pools are in use.
+ * Initialise the map if we're the first user.
+ * Returns the number of pools. If this is '1', no reference
+ * was taken.
  */
 static unsigned int
 svc_pool_map_get(void)
@@ -253,6 +253,7 @@ svc_pool_map_get(void)
 
        if (m->count++) {
                mutex_unlock(&svc_pool_map_mutex);
+               WARN_ON_ONCE(m->npools <= 1);
                return m->npools;
        }
 
@@ -268,29 +269,36 @@ svc_pool_map_get(void)
                break;
        }
 
-       if (npools < 0) {
+       if (npools <= 0) {
                /* default, or memory allocation failure */
                npools = 1;
                m->mode = SVC_POOL_GLOBAL;
        }
        m->npools = npools;
 
+       if (npools == 1)
+               /* service is unpooled, so doesn't hold a reference */
+               m->count--;
+
        mutex_unlock(&svc_pool_map_mutex);
-       return m->npools;
+       return npools;
 }
 
 /*
- * Drop a reference to the global map of cpus to pools.
+ * Drop a reference to the global map of cpus to pools, if
+ * pools were in use, i.e. if npools > 1.
  * When the last reference is dropped, the map data is
  * freed; this allows the sysadmin to change the pool
  * mode using the pool_mode module option without
  * rebooting or re-loading sunrpc.ko.
  */
 static void
-svc_pool_map_put(void)
+svc_pool_map_put(int npools)
 {
        struct svc_pool_map *m = &svc_pool_map;
 
+       if (npools <= 1)
+               return;
        mutex_lock(&svc_pool_map_mutex);
 
        if (!--m->count) {
@@ -359,21 +367,18 @@ svc_pool_for_cpu(struct svc_serv *serv, int cpu)
        struct svc_pool_map *m = &svc_pool_map;
        unsigned int pidx = 0;
 
-       /*
-        * An uninitialised map happens in a pure client when
-        * lockd is brought up, so silently treat it the
-        * same as SVC_POOL_GLOBAL.
-        */
-       if (svc_serv_is_pooled(serv)) {
-               switch (m->mode) {
-               case SVC_POOL_PERCPU:
-                       pidx = m->to_pool[cpu];
-                       break;
-               case SVC_POOL_PERNODE:
-                       pidx = m->to_pool[cpu_to_node(cpu)];
-                       break;
-               }
+       if (serv->sv_nrpools <= 1)
+               return serv->sv_pools;
+
+       switch (m->mode) {
+       case SVC_POOL_PERCPU:
+               pidx = m->to_pool[cpu];
+               break;
+       case SVC_POOL_PERNODE:
+               pidx = m->to_pool[cpu_to_node(cpu)];
+               break;
        }
+
        return &serv->sv_pools[pidx % serv->sv_nrpools];
 }
 
@@ -526,7 +531,7 @@ svc_create_pooled(struct svc_program *prog, unsigned int bufsize,
                goto out_err;
        return serv;
 out_err:
-       svc_pool_map_put();
+       svc_pool_map_put(npools);
        return NULL;
 }
 EXPORT_SYMBOL_GPL(svc_create_pooled);
@@ -561,8 +566,7 @@ svc_destroy(struct kref *ref)
 
        cache_clean_deferred(serv);
 
-       if (svc_serv_is_pooled(serv))
-               svc_pool_map_put();
+       svc_pool_map_put(serv->sv_nrpools);
 
        kfree(serv->sv_pools);
        kfree(serv);