From: "J. Bruce Fields" <bfields@fieldses.org>
Subject: Re: [PATCH 1/5] knfsd: Replace lock_kernel with a mutex for nfsd
	thread startup/shutdown locking.
Date: Fri, 13 Jun 2008 14:04:55 -0400
Message-ID: <20080613180455.GC8501@fieldses.org>
References: <1213101639-26423-1-git-send-email-jlayton@redhat.com>
	<1213101639-26423-2-git-send-email-jlayton@redhat.com>
	<18512.39490.36187.783761@notabene.brown>
	<20080612083713.30282035@barsoom.rdu.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: linux-nfs@vger.kernel.org, nfsv4@linux-nfs.org, gnb@melbourne.sgi.com
To: Jeff Layton <jlayton@redhat.com>
In-Reply-To: <20080612083713.30282035@barsoom.rdu.redhat.com>
Sender: nfsv4-bounces@linux-nfs.org
Errors-To: nfsv4-bounces@linux-nfs.org

On Thu, Jun 12, 2008 at 08:37:13AM -0400, Jeff Layton wrote:
> On Thu, 12 Jun 2008 13:38:42 +1000
> Neil Brown <neilb@suse.de> wrote:
> 
> > On Tuesday June 10, jlayton@redhat.com wrote:
> > > From: Neil Brown <neilb@suse.de>
> > > 
> > > This removes the BKL from the RPC service creation codepath. The BKL
> > > really isn't adequate for this job since some of this info needs
> > > protection across sleeps.
> > > 
> > > Also, add some comments to try and clarify how the locking should work
> > > and to make it clear that the BKL isn't necessary as long as there is
> > > adequate locking between tasks when touching the svc_serv fields.
> > > 
> > > Signed-off-by: Neil Brown <neilb@suse.de>
> > > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > 
> > Thanks for proceeding with this.
> > As well as the fixes for this that you included in 2/5, you need this
> > patch as well for correctness.... Well, you need the first hunk.  The
> > second is unrelated but probably should be fixed.
> > 
> > The rest all looks good.
> > 
> > NeilBrown
> > 
> > 
> > ---------------------------------
> > 
> > Signed-off-by: Neil Brown <neilb@suse.de>
> > 
> > ### Diffstat output
> >  ./fs/nfsd/nfssvc.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff .prev/fs/nfsd/nfssvc.c ./fs/nfsd/nfssvc.c
> > --- .prev/fs/nfsd/nfssvc.c	2008-06-12 13:27:47.000000000 +1000
> > +++ ./fs/nfsd/nfssvc.c	2008-06-12 13:32:14.000000000 +1000
> > @@ -169,10 +169,12 @@ int nfsd_vers(int vers, enum vers_op cha
> >  
> >  int nfsd_nrthreads(void)
> >  {
> > -	if (nfsd_serv == NULL)
> > -		return 0;
> > -	else
> > -		return nfsd_serv->sv_nrthreads;
> > +	int rv = 0;
> > +	mutex_lock(&nfsd_mutex);
> > +	if (nfsd_serv)
> > +		rv = nfsd_serv->sv_nrthreads;
> > +	mutex_unlock(&nfsd_mutex);
> > +	return rv;
> >  }
> >  
> 
> ACK on this first part. Good catch...

Applied that--thanks Neil.

--b.