Re: [PATCH 1/5] knfsd: Replace lock_kernel with a mutex for nfsd thread startup/shutdown locking.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 12 Jun 2008 13:38:42 +1000
Neil Brown <neilb@xxxxxxx> wrote:

> On Tuesday June 10, jlayton@xxxxxxxxxx wrote:
> > From: Neil Brown <neilb@xxxxxxx>
> > 
> > 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@xxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> 
> 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@xxxxxxx>
> 
> ### 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...

>  static int killsig;	/* signal that was used to kill last nfsd */
> @@ -275,7 +277,7 @@ static int nfsd_init_socks(int port)
>  					SVC_SOCK_DEFAULTS);
>  		if (error < 0)
>  			lockd_down();
> -}
> +	}
>  	if (error < 0)
>  		return error;
>  	return 0;

This looks correct already in Bruce's tree. Am I missing something?

Cheers,
-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
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

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux