Return-Path: Received: from mail-yk0-f170.google.com ([209.85.160.170]:35539 "EHLO mail-yk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932432AbbHGRMD (ORCPT ); Fri, 7 Aug 2015 13:12:03 -0400 Received: by ykcq64 with SMTP id q64so88839385ykc.2 for ; Fri, 07 Aug 2015 10:12:02 -0700 (PDT) Date: Fri, 7 Aug 2015 13:12:00 -0400 From: Jeff Layton To: Kinglong Mee Cc: bfields@fieldses.org, linux-nfs@vger.kernel.org Subject: Re: [PATCH v2 03/18] nfsd: convert laundry_wq to something less nfsd4 specific Message-ID: <20150807131200.0ee2951d@tlielax.poochiereds.net> In-Reply-To: <55C4CE0A.7080401@gmail.com> References: <1438264341-18048-1-git-send-email-jeff.layton@primarydata.com> <1438809216-4846-1-git-send-email-jeff.layton@primarydata.com> <1438809216-4846-4-git-send-email-jeff.layton@primarydata.com> <55C4CE0A.7080401@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Fri, 7 Aug 2015 23:26:02 +0800 Kinglong Mee 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 > > --- > > 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 > > #include > > #include > > +#include > > #include > > #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