On Fri, 7 Aug 2015 23:26:02 +0800
Kinglong Mee <kinglongmee@xxxxxxxxx> wrote:
> On 8/6/2015 05:13, Jeff Layton wrote:
> > Currently, nfsd uses a singlethread workqueue for this, but that's not
> > necessary. If we have multiple namespaces, then there's no need to
> > serialize the laundromat runs since they are only requeued at the end of
> > the work itself.
> >
> > Also, create_singlethread_workqueue adds the WQ_MEM_RECLAIM flag, which
> > doesn't really seem to be necessary. The laundromat jobs are always
> > kicked off via a timer, and not from memory reclaim paths. There's no
> > need for a rescuer thread.
> >
> > Signed-off-by: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfsd/nfs4state.c | 23 +++++------------------
> > fs/nfsd/nfsd.h | 1 +
> > fs/nfsd/nfssvc.c | 14 +++++++++++++-
> > 3 files changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index b3b306bd1830..c94859122e6f 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4366,7 +4366,6 @@ nfs4_laundromat(struct nfsd_net *nn)
> > return new_timeo;
> > }
> >
> > -static struct workqueue_struct *laundry_wq;
> > static void laundromat_main(struct work_struct *);
> >
> > static void
> > @@ -4380,7 +4379,7 @@ laundromat_main(struct work_struct *laundry)
> >
> > t = nfs4_laundromat(nn);
> > dprintk("NFSD: laundromat_main - sleeping for %ld seconds\n", t);
> > - queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ);
> > + queue_delayed_work(nfsd_laundry_wq, &nn->laundromat_work, t*HZ);
> > }
> >
> > static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp)
> > @@ -6569,7 +6568,8 @@ nfs4_state_start_net(struct net *net)
> > nfsd4_client_tracking_init(net);
> > printk(KERN_INFO "NFSD: starting %ld-second grace period (net %p)\n",
> > nn->nfsd4_grace, net);
> > - queue_delayed_work(laundry_wq, &nn->laundromat_work, nn->nfsd4_grace * HZ);
> > + queue_delayed_work(nfsd_laundry_wq, &nn->laundromat_work,
> > + nn->nfsd4_grace * HZ);
> > return 0;
> > }
> >
> > @@ -6583,22 +6583,10 @@ nfs4_state_start(void)
> > ret = set_callback_cred();
> > if (ret)
> > return -ENOMEM;
> > - laundry_wq = create_singlethread_workqueue("nfsd4");
> > - if (laundry_wq == NULL) {
> > - ret = -ENOMEM;
> > - goto out_recovery;
> > - }
> > ret = nfsd4_create_callback_queue();
> > - if (ret)
> > - goto out_free_laundry;
> > -
> > - set_max_delegations();
> > -
> > - return 0;
> > + if (!ret)
> > + set_max_delegations();
> >
> > -out_free_laundry:
> > - destroy_workqueue(laundry_wq);
> > -out_recovery:
> > return ret;
> > }
> >
> > @@ -6635,7 +6623,6 @@ nfs4_state_shutdown_net(struct net *net)
> > void
> > nfs4_state_shutdown(void)
> > {
> > - destroy_workqueue(laundry_wq);
> > nfsd4_destroy_callback_queue();
> > }
> >
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index cf980523898b..0199415344ff 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -62,6 +62,7 @@ struct readdir_cd {
> > extern struct svc_program nfsd_program;
> > extern struct svc_version nfsd_version2, nfsd_version3,
> > nfsd_version4;
> > +extern struct workqueue_struct *nfsd_laundry_wq;
> > extern struct mutex nfsd_mutex;
> > extern spinlock_t nfsd_drc_lock;
> > extern unsigned long nfsd_drc_max_mem;
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index ad4e2377dd63..ced9944201a0 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -17,6 +17,7 @@
> > #include <linux/lockd/bind.h>
> > #include <linux/nfsacl.h>
> > #include <linux/seq_file.h>
> > +#include <linux/workqueue.h>
> > #include <net/net_namespace.h>
> > #include "nfsd.h"
> > #include "cache.h"
> > @@ -28,6 +29,9 @@
> > extern struct svc_program nfsd_program;
> > static int nfsd(void *vrqstp);
> >
> > +/* A workqueue for nfsd-related cleanup tasks */
> > +struct workqueue_struct *nfsd_laundry_wq;
> > +
> > /*
> > * nfsd_mutex protects nn->nfsd_serv -- both the pointer itself and the members
> > * of the svc_serv struct. In particular, ->sv_nrthreads but also to some
> > @@ -224,11 +228,19 @@ static int nfsd_startup_generic(int nrservs)
> > if (ret)
> > goto dec_users;
> >
> > + ret = -ENOMEM;
> > + nfsd_laundry_wq = alloc_workqueue("%s", WQ_UNBOUND, 0, "nfsd-laundry");
> > + if (!nfsd_laundry_wq)
> > + goto out_racache;
> > +
> > ret = nfs4_state_start();
> > if (ret)
> > - goto out_racache;
> > + goto out_wq;
> > return 0;
> >
> > +out_wq:
> > + destroy_workqueue(nfsd_laundry_wq);
> > + nfsd_laundry_wq = NULL;
> > out_racache:
> > nfsd_racache_shutdown();
> > dec_users:
>
> Is a destroy_workqueue(nfsd_laundry_wq) needed in nfsd_shutdown_generic() ?
>
> thanks,
> Kinglong Mee
Yes! I had that in an earlier version of this, but somehow it got
dropped. Good catch. I'll fix that for the next respin.
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
