SUNRPC: discard sv_refcnt, and svc_get/svc_put
sv_refcnt is no longer useful. lockd and nfs-cb only ever have the svc active when there are a non-zero number of threads, so sv_refcnt mirrors sv_nrthreads. nfsd also keeps the svc active between when a socket is added and when the first thread is started, but we don't really need a refcount for that. We can simply not destroy the svc while there are any permanent sockets attached. So remove sv_refcnt and the get/put functions. Instead of a final call to svc_put(), call svc_destroy() instead. This is changed to also store NULL in the passed-in pointer to make it easier to avoid use-after-free situations. Signed-off-by: NeilBrown <neilb@suse.de> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
This commit is contained in:
parent
7b207ccd98
commit
1e3577a452
|
@ -345,10 +345,10 @@ static int lockd_get(void)
|
|||
|
||||
serv->sv_maxconn = nlm_max_connections;
|
||||
error = svc_set_num_threads(serv, NULL, 1);
|
||||
/* The thread now holds the only reference */
|
||||
svc_put(serv);
|
||||
if (error < 0)
|
||||
if (error < 0) {
|
||||
svc_destroy(&serv);
|
||||
return error;
|
||||
}
|
||||
|
||||
nlmsvc_serv = serv;
|
||||
register_inetaddr_notifier(&lockd_inetaddr_notifier);
|
||||
|
@ -372,11 +372,9 @@ static void lockd_put(void)
|
|||
unregister_inet6addr_notifier(&lockd_inet6addr_notifier);
|
||||
#endif
|
||||
|
||||
svc_get(nlmsvc_serv);
|
||||
svc_set_num_threads(nlmsvc_serv, NULL, 0);
|
||||
svc_put(nlmsvc_serv);
|
||||
timer_delete_sync(&nlmsvc_retry);
|
||||
nlmsvc_serv = NULL;
|
||||
svc_destroy(&nlmsvc_serv);
|
||||
dprintk("lockd_down: service destroyed\n");
|
||||
}
|
||||
|
||||
|
|
|
@ -187,7 +187,7 @@ static struct svc_serv *nfs_callback_create_svc(int minorversion)
|
|||
* Check whether we're already up and running.
|
||||
*/
|
||||
if (cb_info->serv)
|
||||
return svc_get(cb_info->serv);
|
||||
return cb_info->serv;
|
||||
|
||||
/*
|
||||
* Sanity check: if there's no task,
|
||||
|
@ -245,9 +245,10 @@ int nfs_callback_up(u32 minorversion, struct rpc_xprt *xprt)
|
|||
|
||||
cb_info->users++;
|
||||
err_net:
|
||||
if (!cb_info->users)
|
||||
cb_info->serv = NULL;
|
||||
svc_put(serv);
|
||||
if (!cb_info->users) {
|
||||
svc_set_num_threads(cb_info->serv, NULL, 0);
|
||||
svc_destroy(&cb_info->serv);
|
||||
}
|
||||
err_create:
|
||||
mutex_unlock(&nfs_callback_mutex);
|
||||
return ret;
|
||||
|
@ -271,11 +272,9 @@ void nfs_callback_down(int minorversion, struct net *net)
|
|||
nfs_callback_down_net(minorversion, serv, net);
|
||||
cb_info->users--;
|
||||
if (cb_info->users == 0) {
|
||||
svc_get(serv);
|
||||
svc_set_num_threads(serv, NULL, 0);
|
||||
svc_put(serv);
|
||||
dprintk("nfs_callback_down: service destroyed\n");
|
||||
cb_info->serv = NULL;
|
||||
svc_destroy(&cb_info->serv);
|
||||
}
|
||||
mutex_unlock(&nfs_callback_mutex);
|
||||
}
|
||||
|
|
|
@ -126,13 +126,6 @@ struct nfsd_net {
|
|||
struct svc_info nfsd_info;
|
||||
#define nfsd_serv nfsd_info.serv
|
||||
|
||||
/* When a listening socket is added to nfsd, keep_active is set
|
||||
* and this justifies a reference on nfsd_serv. This stops
|
||||
* nfsd_serv from being freed. When the number of threads is
|
||||
* set, keep_active is cleared and the reference is dropped. So
|
||||
* when the last thread exits, the service will be destroyed.
|
||||
*/
|
||||
int keep_active;
|
||||
|
||||
/*
|
||||
* clientid and stateid data for construction of net unique COPY
|
||||
|
|
|
@ -711,12 +711,8 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
|
|||
serv = nn->nfsd_serv;
|
||||
err = svc_addsock(serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred);
|
||||
|
||||
if (err < 0 && !serv->sv_nrthreads && !nn->keep_active)
|
||||
if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
|
||||
nfsd_last_thread(net);
|
||||
else if (err >= 0 && !serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
|
||||
svc_get(serv);
|
||||
|
||||
svc_put(serv);
|
||||
return err;
|
||||
}
|
||||
|
||||
|
@ -754,10 +750,6 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
|
|||
if (err < 0 && err != -EAFNOSUPPORT)
|
||||
goto out_close;
|
||||
|
||||
if (!serv->sv_nrthreads && !xchg(&nn->keep_active, 1))
|
||||
svc_get(serv);
|
||||
|
||||
svc_put(serv);
|
||||
return 0;
|
||||
out_close:
|
||||
xprt = svc_find_xprt(serv, transport, net, PF_INET, port);
|
||||
|
@ -766,10 +758,9 @@ out_close:
|
|||
svc_xprt_put(xprt);
|
||||
}
|
||||
out_err:
|
||||
if (!serv->sv_nrthreads && !nn->keep_active)
|
||||
if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
|
||||
nfsd_last_thread(net);
|
||||
|
||||
svc_put(serv);
|
||||
return err;
|
||||
}
|
||||
|
||||
|
|
|
@ -59,15 +59,6 @@ static __be32 nfsd_init_request(struct svc_rqst *,
|
|||
* nfsd_mutex protects nn->nfsd_serv -- both the pointer itself and some members
|
||||
* of the svc_serv struct such as ->sv_temp_socks and ->sv_permsocks.
|
||||
*
|
||||
* If (out side the lock) nn->nfsd_serv is non-NULL, then it must point to a
|
||||
* properly initialised 'struct svc_serv' with ->sv_nrthreads > 0 (unless
|
||||
* nn->keep_active is set). That number of nfsd threads must
|
||||
* exist and each must be listed in ->sp_all_threads in some entry of
|
||||
* ->sv_pools[].
|
||||
*
|
||||
* Each active thread holds a counted reference on nn->nfsd_serv, as does
|
||||
* the nn->keep_active flag and various transient calls to svc_get().
|
||||
*
|
||||
* Finally, the nfsd_mutex also protects some of the global variables that are
|
||||
* accessed when nfsd starts and that are settable via the write_* routines in
|
||||
* nfsctl.c. In particular:
|
||||
|
@ -572,6 +563,7 @@ void nfsd_last_thread(struct net *net)
|
|||
|
||||
nfsd_shutdown_net(net);
|
||||
nfsd_export_flush(net);
|
||||
svc_destroy(&serv);
|
||||
}
|
||||
|
||||
void nfsd_reset_versions(struct nfsd_net *nn)
|
||||
|
@ -646,11 +638,9 @@ void nfsd_shutdown_threads(struct net *net)
|
|||
return;
|
||||
}
|
||||
|
||||
svc_get(serv);
|
||||
/* Kill outstanding nfsd threads */
|
||||
svc_set_num_threads(serv, NULL, 0);
|
||||
nfsd_last_thread(net);
|
||||
svc_put(serv);
|
||||
mutex_unlock(&nfsd_mutex);
|
||||
}
|
||||
|
||||
|
@ -666,10 +656,9 @@ int nfsd_create_serv(struct net *net)
|
|||
struct svc_serv *serv;
|
||||
|
||||
WARN_ON(!mutex_is_locked(&nfsd_mutex));
|
||||
if (nn->nfsd_serv) {
|
||||
svc_get(nn->nfsd_serv);
|
||||
if (nn->nfsd_serv)
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (nfsd_max_blksize == 0)
|
||||
nfsd_max_blksize = nfsd_get_default_max_blksize();
|
||||
nfsd_reset_versions(nn);
|
||||
|
@ -680,7 +669,7 @@ int nfsd_create_serv(struct net *net)
|
|||
serv->sv_maxconn = nn->max_connections;
|
||||
error = svc_bind(serv, net);
|
||||
if (error < 0) {
|
||||
svc_put(serv);
|
||||
svc_destroy(&serv);
|
||||
return error;
|
||||
}
|
||||
spin_lock(&nfsd_notifier_lock);
|
||||
|
@ -764,7 +753,6 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
|
|||
nthreads[0] = 1;
|
||||
|
||||
/* apply the new numbers */
|
||||
svc_get(nn->nfsd_serv);
|
||||
for (i = 0; i < n; i++) {
|
||||
err = svc_set_num_threads(nn->nfsd_serv,
|
||||
&nn->nfsd_serv->sv_pools[i],
|
||||
|
@ -772,7 +760,6 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
|
|||
if (err)
|
||||
break;
|
||||
}
|
||||
svc_put(nn->nfsd_serv);
|
||||
return err;
|
||||
}
|
||||
|
||||
|
@ -814,13 +801,8 @@ nfsd_svc(int nrservs, struct net *net, const struct cred *cred)
|
|||
goto out_put;
|
||||
error = serv->sv_nrthreads;
|
||||
out_put:
|
||||
/* Threads now hold service active */
|
||||
if (xchg(&nn->keep_active, 0))
|
||||
svc_put(serv);
|
||||
|
||||
if (serv->sv_nrthreads == 0)
|
||||
nfsd_last_thread(net);
|
||||
svc_put(serv);
|
||||
out:
|
||||
mutex_unlock(&nfsd_mutex);
|
||||
return error;
|
||||
|
|
|
@ -69,7 +69,6 @@ struct svc_serv {
|
|||
struct svc_program * sv_program; /* RPC program */
|
||||
struct svc_stat * sv_stats; /* RPC statistics */
|
||||
spinlock_t sv_lock;
|
||||
struct kref sv_refcnt;
|
||||
unsigned int sv_nrthreads; /* # of server threads */
|
||||
unsigned int sv_maxconn; /* max connections allowed or
|
||||
* '0' causing max to be based
|
||||
|
@ -103,31 +102,7 @@ struct svc_info {
|
|||
struct mutex *mutex;
|
||||
};
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
static inline struct svc_serv *svc_get(struct svc_serv *serv)
|
||||
{
|
||||
kref_get(&serv->sv_refcnt);
|
||||
return serv;
|
||||
}
|
||||
|
||||
void svc_destroy(struct kref *);
|
||||
|
||||
/**
|
||||
* 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)
|
||||
{
|
||||
kref_put(&serv->sv_refcnt, svc_destroy);
|
||||
}
|
||||
void svc_destroy(struct svc_serv **svcp);
|
||||
|
||||
/*
|
||||
* Maximum payload size supported by a kernel RPC server.
|
||||
|
|
|
@ -463,7 +463,6 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
|
|||
return NULL;
|
||||
serv->sv_name = prog->pg_name;
|
||||
serv->sv_program = prog;
|
||||
kref_init(&serv->sv_refcnt);
|
||||
serv->sv_stats = prog->pg_stats;
|
||||
if (bufsize > RPCSVC_MAXPAYLOAD)
|
||||
bufsize = RPCSVC_MAXPAYLOAD;
|
||||
|
@ -564,11 +563,13 @@ EXPORT_SYMBOL_GPL(svc_create_pooled);
|
|||
* protect sv_permsocks and sv_tempsocks.
|
||||
*/
|
||||
void
|
||||
svc_destroy(struct kref *ref)
|
||||
svc_destroy(struct svc_serv **servp)
|
||||
{
|
||||
struct svc_serv *serv = container_of(ref, struct svc_serv, sv_refcnt);
|
||||
struct svc_serv *serv = *servp;
|
||||
unsigned int i;
|
||||
|
||||
*servp = NULL;
|
||||
|
||||
dprintk("svc: svc_destroy(%s)\n", serv->sv_program->pg_name);
|
||||
timer_shutdown_sync(&serv->sv_temptimer);
|
||||
|
||||
|
@ -675,7 +676,6 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node)
|
|||
if (!rqstp)
|
||||
return ERR_PTR(-ENOMEM);
|
||||
|
||||
svc_get(serv);
|
||||
spin_lock_bh(&serv->sv_lock);
|
||||
serv->sv_nrthreads += 1;
|
||||
spin_unlock_bh(&serv->sv_lock);
|
||||
|
@ -935,11 +935,6 @@ svc_exit_thread(struct svc_rqst *rqstp)
|
|||
|
||||
svc_rqst_free(rqstp);
|
||||
|
||||
svc_put(serv);
|
||||
/* That svc_put() cannot be the last, because the thread
|
||||
* waiting for SP_VICTIM_REMAINS to clear must hold
|
||||
* a reference. So it is still safe to access pool.
|
||||
*/
|
||||
clear_and_wake_up_bit(SP_VICTIM_REMAINS, &pool->sp_flags);
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(svc_exit_thread);
|
||||
|
|
Loading…
Reference in New Issue