On Mon, Jul 31, 2023 at 04:48:32PM +1000, NeilBrown wrote:
> Now that the last nfsd thread is stopped by an explicit act of calling
> svc_set_num_threads() with a count of zero, we only have a limited
> number of places that can happen, and don't need to call
> nfsd_last_thread() in nfsd_put()
>
> So separate that out and call it at the two places where the number of
> threads is set to zero.
>
> Move the clearing of ->nfsd_serv and the call to svc_xprt_destroy_all()
> into nfsd_last_thread(), as they are really part of the same action.
>
> nfsd_put() is now a thin wrapper around svc_put(), so make it a static inline.
>
> nfsd_put() cannot be called after nfsd_last_thread(), so in a couple of
> places we have to use svc_put() instead.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
I've applied 4/12 and 5/12.
I'll push these out later today.
> ---
> fs/nfsd/nfsd.h | 7 ++++++-
> fs/nfsd/nfssvc.c | 52 ++++++++++++++++++------------------------------
> 2 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index d88498f8b275..11c14faa6c67 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -96,7 +96,12 @@ 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_put(struct net *net);
> +static inline void nfsd_put(struct net *net)
> +{
> + struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +
> + svc_put(nn->nfsd_serv);
> +}
>
> bool i_am_nfsd(void);
>
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 33a80725e14e..1582af33e204 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -542,9 +542,14 @@ static struct notifier_block nfsd_inet6addr_notifier = {
> /* Only used under nfsd_mutex, so this atomic may be overkill: */
> static atomic_t nfsd_notifier_refcount = ATOMIC_INIT(0);
>
> -static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
> +static void nfsd_last_thread(struct net *net)
> {
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> + struct svc_serv *serv = nn->nfsd_serv;
> +
> + spin_lock(&nfsd_notifier_lock);
> + nn->nfsd_serv = NULL;
> + spin_unlock(&nfsd_notifier_lock);
>
> /* check if the notifier still has clients */
> if (atomic_dec_return(&nfsd_notifier_refcount) == 0) {
> @@ -554,6 +559,8 @@ static void nfsd_last_thread(struct svc_serv *serv, struct net *net)
> #endif
> }
>
> + svc_xprt_destroy_all(serv, net);
> +
> /*
> * write_ports can create the server without actually starting
> * any threads--if we get shut down before any threads are
> @@ -644,7 +651,8 @@ void nfsd_shutdown_threads(struct net *net)
> svc_get(serv);
> /* Kill outstanding nfsd threads */
> svc_set_num_threads(serv, NULL, 0);
> - nfsd_put(net);
> + nfsd_last_thread(net);
> + svc_put(serv);
> mutex_unlock(&nfsd_mutex);
> }
>
> @@ -674,9 +682,6 @@ int nfsd_create_serv(struct net *net)
> serv->sv_maxconn = nn->max_connections;
> error = svc_bind(serv, net);
> if (error < 0) {
> - /* NOT nfsd_put() as notifiers (see below) haven't
> - * been set up yet.
> - */
> svc_put(serv);
> return error;
> }
> @@ -719,29 +724,6 @@ int nfsd_get_nrthreads(int n, int *nthreads, struct net *net)
> return 0;
> }
>
> -/* This is the callback for kref_put() below.
> - * There is no code here as the first thing to be done is
> - * call svc_shutdown_net(), but we cannot get the 'net' from
> - * the kref. So do all the work when kref_put returns true.
> - */
> -static void nfsd_noop(struct kref *ref)
> -{
> -}
> -
> -void nfsd_put(struct net *net)
> -{
> - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> -
> - if (kref_put(&nn->nfsd_serv->sv_refcnt, nfsd_noop)) {
> - svc_xprt_destroy_all(nn->nfsd_serv, net);
> - nfsd_last_thread(nn->nfsd_serv, net);
> - svc_destroy(&nn->nfsd_serv->sv_refcnt);
> - spin_lock(&nfsd_notifier_lock);
> - nn->nfsd_serv = NULL;
> - spin_unlock(&nfsd_notifier_lock);
> - }
> -}
> -
> int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
> {
> int i = 0;
> @@ -792,7 +774,7 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
> if (err)
> break;
> }
> - nfsd_put(net);
> + svc_put(nn->nfsd_serv);
> return err;
> }
>
> @@ -807,6 +789,7 @@ nfsd_svc(int nrservs, struct net *net, const struct cred *cred)
> int error;
> bool nfsd_up_before;
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> + struct svc_serv *serv;
>
> mutex_lock(&nfsd_mutex);
> dprintk("nfsd: creating service\n");
> @@ -826,22 +809,25 @@ nfsd_svc(int nrservs, struct net *net, const struct cred *cred)
> goto out;
>
> nfsd_up_before = nn->nfsd_net_up;
> + serv = nn->nfsd_serv;
>
> error = nfsd_startup_net(net, cred);
> if (error)
> goto out_put;
> - error = svc_set_num_threads(nn->nfsd_serv, NULL, nrservs);
> + error = svc_set_num_threads(serv, NULL, nrservs);
> if (error)
> goto out_shutdown;
> - error = nn->nfsd_serv->sv_nrthreads;
> + error = serv->sv_nrthreads;
> + if (error == 0)
> + nfsd_last_thread(net);
> out_shutdown:
> if (error < 0 && !nfsd_up_before)
> nfsd_shutdown_net(net);
> out_put:
> /* Threads now hold service active */
> if (xchg(&nn->keep_active, 0))
> - nfsd_put(net);
> - nfsd_put(net);
> + svc_put(serv);
> + svc_put(serv);
> out:
> mutex_unlock(&nfsd_mutex);
> return error;
> --
> 2.40.1
>
--
Chuck Lever
