Re: [NFS] [PATCH 005 of 9] knfsd: Be more selective in which sockets lockd listens on.

J. Bruce Fields
2006-07-26T19:17:23Z

On Tue, Jul 25, 2006 at 11:54:47AM +1000, NeilBrown wrote:
> @@ -112,6 +114,7 @@ lockd(struct svc_rqst *rqstp)
> * Let our maker know we're running.
> */
> nlmsvc_pid = current->pid;
> + nlmsvc_serv = serv;

Nitpick: any reason not to just get rid of the local variable "serv"
after that?

> @@ -224,8 +259,10 @@ lockd_up(void)
> /*
> * Check whether we're already up and running.
> */
> - if (nlmsvc_pid)
> + if (nlmsvc_pid) {
> + error = make_socks(nlmsvc_serv, proto);
> goto out;

...

> + if ((error = make_socks(serv, proto)) < 0) {
> if (warned++ == 0)
> printk(KERN_WARNING
> "lockd_up: makesock failed, error=%d\n", error);

The warning is printk'ed a little inconsistently. (If we care, maybe it
should just go inside make_socks?)

By the way, why don't most callers use the error returned from
lockd_up()?

> diff .prev/fs/nfsd/nfssvc.c ./fs/nfsd/nfssvc.c
> --- .prev/fs/nfsd/nfssvc.c 2006-07-24 15:14:31.000000000 +1000
> +++ ./fs/nfsd/nfssvc.c 2006-07-24 15:15:04.000000000 +1000
> @@ -134,6 +134,9 @@ static int killsig = 0; /* signal that w
> static void nfsd_last_thread(struct svc_serv *serv)
> {
> /* When last nfsd thread exits we need to do some clean-up */
> + struct svc_sock *svsk;
> + list_for_each_entry(svsk, &serv->sv_permsocks, sk_list)
> + lockd_down();

So I guess it's a minor point, but: we take the trouble to only open tcp
or udp sockets as necessary, but then won't close them down till all the
mounts and nfsd's go away at which point we close them all down.

Would it be that bad just to always listen on both?

--b.