From 1e3577a4521ef33199eea05ce7b9099825848c49 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Fri, 15 Dec 2023 11:56:34 +1100 Subject: 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 Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/netns.h | 7 ------- fs/nfsd/nfsctl.c | 13 ++----------- fs/nfsd/nfssvc.c | 26 ++++---------------------- 3 files changed, 6 insertions(+), 40 deletions(-) (limited to 'fs/nfsd') diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index 16dbef245dbb..74b4360779a1 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -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 diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 46a001e81b55..5c2fae98d98a 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -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; } diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 365968737923..1a295c409343 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -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; -- cgit