[PATCH 000 of 19] knfsd: lockd improvements

[PATCH 000 of 19] knfsd: lockd improvements

[NeilBrown]

Following 19 patches are reviewed and revised versions of
some of a series of patches that Olaf Kirch posted to the
nfs list a few weeks ago.

They are suitable for 2.6.19.

They have been in the SuSE kernel (in a few different forms)
for quite some time and so have received a fair amount of testing,
though some of the code here is inevitably quite new (it needs
to be just to keep up with the ever-changing kernel ;-)

Apart from a few issues that I have posted about on the 
nfs list, the only changes I have made are enhancing a few
change logs, modifying a couple of comments, removing some dead code,
and adding the odd module parameter to parallel a new sysctl.

This set does *not* include the in-kernel implementation of statd that
came at the end of Olaf's series.  I think that is a big enough change
that it deserves a separate posting, and I haven't finished my review
of it yet anyway :-)

The main item of functionality provided by this set of patches
it to enable lock to identify peer by name instead of by IP address.
This can be important for multi-homed machines.  This is an
option only and must be explicitly selected by a sysctl or
module parameter.  The default is to use IP addresses, which
is generally safer if no multi-homed hosts are present.

This functionality is spread over several patches (there is a fair
bit of infrastructure needed) and there are a lot of clean-ups and
a few minor bug fixes as well.

This patch set compiles and passes basic locking tests (both client
side and server side).
I haven't yet tested the various client-reboot and server-reboot
handling as they are a little harder to test.

If anyone out there is in a position to easily run the connectathon
locking tests on a kernel with these patches..... :-)

 [PATCH 001 of 19] knfsd: Hide use of lockd's h_monitored flag
 [PATCH 002 of 19] knfsd: Consolidate common code for statd->lockd notification
 [PATCH 003 of 19] knfsd: When looking up a lockd host, pass hostname & length
 [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle
 [PATCH 005 of 19] knfsd: Misc minor fixes, indentation changes
 [PATCH 006 of 19] knfsd: lockd: Make nlm_host_rebooted use the nsm_handle
 [PATCH 007 of 19] knfsd: lockd: make the nsm upcalls use the nsm_handle
 [PATCH 008 of 19] knfsd: lockd: make the hash chains use a hlist_node
 [PATCH 009 of 19] knfsd: lockd: Change list of blocked list to list_node
 [PATCH 010 of 19] knfsd: Change nlm_file to use a hlist
 [PATCH 011 of 19] knfsd: lockd: make nlm_traverse_* more flexible
 [PATCH 012 of 19] knfsd: lockd: Add nlm_destroy_host
 [PATCH 013 of 19] knfsd: Simplify nlmsvc_invalidate_all
 [PATCH 014 of 19] knfsd: lockd: optionally use hostnames for identifying peers
 [PATCH 015 of 19] knfsd: make nlmclnt_next_cookie SMP safe
 [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies
 [PATCH 017 of 19] knfsd: Export nsm_local_state to user space via sysctl
 [PATCH 018 of 19] knfsd: lockd: fix use of h_nextrebind
 [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[PATCH 001 of 19] knfsd: Hide use of lockd's h_monitored flag

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch moves all checks of the h_monitored flag into
  the nsm_monitor/unmonitor functions.
  A subsequent patch will replace the mechanism by which we
  mark a host as being monitored.

  There is still one occurence of h_monitored outside of
  mon.c and that is in clntlock.c where we respond to a
  reboot. The subsequent patch will modify this too.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/clntproc.c |    2 +-
 ./fs/lockd/host.c     |    9 ++++++---
 ./fs/lockd/mon.c      |   19 ++++++++++++-------
 ./fs/lockd/svc4proc.c |    2 +-
 ./fs/lockd/svcproc.c  |    2 +-
 5 files changed, 21 insertions(+), 13 deletions(-)

diff .prev/fs/lockd/clntproc.c ./fs/lockd/clntproc.c
--- .prev/fs/lockd/clntproc.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/clntproc.c	2006-08-31 16:12:30.000000000 +1000
@@ -499,7 +499,7 @@ nlmclnt_lock(struct nlm_rqst *req, struc
 	unsigned char fl_flags = fl->fl_flags;
 	int status = -ENOLCK;
 
-	if (!host->h_monitored && nsm_monitor(host) < 0) {
+	if (nsm_monitor(host) < 0) {
 		printk(KERN_NOTICE "lockd: failed to monitor %s\n",
 					host->h_name);
 		goto out;

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 16:12:30.000000000 +1000
@@ -331,9 +331,12 @@ nlm_gc_hosts(void)
 			}
 			dprintk("lockd: delete host %s\n", host->h_name);
 			*q = host->h_next;
-			/* Don't unmonitor hosts that have been invalidated */
-			if (host->h_monitored && !host->h_killed)
-				nsm_unmonitor(host);
+
+			/*
+			 * Unmonitor unless host was invalidated (i.e. lockd restarted)
+			 */
+			nsm_unmonitor(host);
+
 			if ((clnt = host->h_rpcclnt) != NULL) {
 				if (atomic_read(&clnt->cl_users)) {
 					printk(KERN_WARNING

diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
--- .prev/fs/lockd/mon.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/mon.c	2006-08-31 16:12:30.000000000 +1000
@@ -74,6 +74,8 @@ nsm_monitor(struct nlm_host *host)
 	int		status;
 
 	dprintk("lockd: nsm_monitor(%s)\n", host->h_name);
+	if (host->h_monitored)
+		return 0;
 
 	status = nsm_mon_unmon(host, SM_MON, &res);
 
@@ -91,15 +93,18 @@ int
 nsm_unmonitor(struct nlm_host *host)
 {
 	struct nsm_res	res;
-	int		status;
+	int		status = 0;
 
 	dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
-
-	status = nsm_mon_unmon(host, SM_UNMON, &res);
-	if (status < 0)
-		printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", host->h_name);
-	else
-		host->h_monitored = 0;
+	if (!host->h_monitored)
+		return 0;
+	host->h_monitored = 0;
+
+	if (!host->h_killed) {
+		status = nsm_mon_unmon(host, SM_UNMON, &res);
+		if (status < 0)
+			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", host->h_name);
+	}
 	return status;
 }
 

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-08-31 16:12:30.000000000 +1000
@@ -39,7 +39,7 @@ nlm4svc_retrieve_args(struct svc_rqst *r
 
 	/* Obtain host handle */
 	if (!(host = nlmsvc_lookup_host(rqstp))
-	 || (argp->monitor && !host->h_monitored && nsm_monitor(host) < 0))
+	 || (argp->monitor && nsm_monitor(host) < 0))
 		goto no_locks;
 	*hostp = host;
 

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-08-31 16:12:30.000000000 +1000
@@ -67,7 +67,7 @@ nlmsvc_retrieve_args(struct svc_rqst *rq
 
 	/* Obtain host handle */
 	if (!(host = nlmsvc_lookup_host(rqstp))
-	 || (argp->monitor && !host->h_monitored && nsm_monitor(host) < 0))
+	 || (argp->monitor && nsm_monitor(host) < 0))
 		goto no_locks;
 	*hostp = host;
 

[PATCH 002 of 19] knfsd: Consolidate common code for statd->lockd notification

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  Common code from nlm4svc_proc_sm_notify and nlmsvc_proc_sm_notify
  is moved into a new nlm_host_rebooted.

  This is in preparation of a patch that will change the
  reboot notification handling entirely.

Signed-off-by: okir@suse.de
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c             |   30 ++++++++++++++++++++++++++++--
 ./fs/lockd/svc4proc.c         |   19 ++-----------------
 ./fs/lockd/svcproc.c          |   17 ++---------------
 ./include/linux/lockd/lockd.h |    5 +++--
 4 files changed, 35 insertions(+), 36 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 16:16:24.000000000 +1000
@@ -39,7 +39,7 @@ static void			nlm_gc_hosts(void);
  * Find an NLM server handle in the cache. If there is none, create it.
  */
 struct nlm_host *
-nlmclnt_lookup_host(struct sockaddr_in *sin, int proto, int version)
+nlmclnt_lookup_host(const struct sockaddr_in *sin, int proto, int version)
 {
 	return nlm_lookup_host(0, sin, proto, version);
 }
@@ -58,7 +58,7 @@ nlmsvc_lookup_host(struct svc_rqst *rqst
  * Common host lookup routine for server & client
  */
 struct nlm_host *
-nlm_lookup_host(int server, struct sockaddr_in *sin,
+nlm_lookup_host(int server, const struct sockaddr_in *sin,
 					int proto, int version)
 {
 	struct nlm_host	*host, **hp;
@@ -260,6 +260,32 @@ void nlm_release_host(struct nlm_host *h
 }
 
 /*
+ * We were notified that the host indicated by address &sin
+ * has rebooted.
+ * Release all resources held by that peer.
+ */
+void nlm_host_rebooted(const struct sockaddr_in *sin, const struct nlm_reboot *argp)
+{
+	struct nlm_host *host;
+
+	/* Obtain the host pointer for this NFS server and try to
+	 * reclaim all locks we hold on this server.
+	 */
+	if ((argp->proto & 1)==0) {
+		/* We are client, he's the server: try to reclaim all locks. */
+		if ((host = nlmclnt_lookup_host(sin, argp->proto >> 1, argp->vers)) == NULL)
+			return;
+		nlmclnt_recovery(host, argp->state);
+	} else {
+		/* He's the client, we're the server: delete all locks held by the client */
+		if ((host = nlm_lookup_host(1, sin, argp->proto >> 1, argp->vers)) == NULL)
+			return;
+		nlmsvc_free_host_resources(host);
+	}
+	nlm_release_host(host);
+}
+
+/*
  * Shut down the hosts module.
  * Note that this routine is called only at server shutdown time.
  */

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-08-31 16:16:24.000000000 +1000
@@ -420,10 +420,6 @@ nlm4svc_proc_sm_notify(struct svc_rqst *
 					      void	        *resp)
 {
 	struct sockaddr_in	saddr = rqstp->rq_addr;
-	int			vers = argp->vers;
-	int			prot = argp->proto >> 1;
-
-	struct nlm_host		*host;
 
 	dprintk("lockd: SM_NOTIFY     called\n");
 	if (saddr.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
@@ -438,21 +434,10 @@ nlm4svc_proc_sm_notify(struct svc_rqst *
 	/* Obtain the host pointer for this NFS server and try to
 	 * reclaim all locks we hold on this server.
 	 */
+	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_addr.s_addr = argp->addr;
+	nlm_host_rebooted(&saddr, argp);
 
-	if ((argp->proto & 1)==0) {
-		if ((host = nlmclnt_lookup_host(&saddr, prot, vers)) != NULL) {
-			nlmclnt_recovery(host, argp->state);
-			nlm_release_host(host);
-		}
-	} else {
-		/* If we run on an NFS server, delete all locks held by the client */
-
-		if ((host = nlm_lookup_host(1, &saddr, prot, vers)) != NULL) {
-			nlmsvc_free_host_resources(host);
-			nlm_release_host(host);
-		}
-	}
 	return rpc_success;
 }
 

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-08-31 16:16:24.000000000 +1000
@@ -449,9 +449,6 @@ nlmsvc_proc_sm_notify(struct svc_rqst *r
 					      void	        *resp)
 {
 	struct sockaddr_in	saddr = rqstp->rq_addr;
-	int			vers = argp->vers;
-	int			prot = argp->proto >> 1;
-	struct nlm_host		*host;
 
 	dprintk("lockd: SM_NOTIFY     called\n");
 	if (saddr.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
@@ -466,19 +463,9 @@ nlmsvc_proc_sm_notify(struct svc_rqst *r
 	/* Obtain the host pointer for this NFS server and try to
 	 * reclaim all locks we hold on this server.
 	 */
+	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_addr.s_addr = argp->addr;
-	if ((argp->proto & 1)==0) {
-		if ((host = nlmclnt_lookup_host(&saddr, prot, vers)) != NULL) {
-			nlmclnt_recovery(host, argp->state);
-			nlm_release_host(host);
-		}
-	} else {
-		/* If we run on an NFS server, delete all locks held by the client */
-		if ((host = nlm_lookup_host(1, &saddr, prot, vers)) != NULL) {
-			nlmsvc_free_host_resources(host);
-			nlm_release_host(host);
-		}
-	}
+	nlm_host_rebooted(&saddr, argp);
 
 	return rpc_success;
 }

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-08-31 16:16:24.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-08-31 16:16:24.000000000 +1000
@@ -161,15 +161,16 @@ int		  nlmclnt_reclaim(struct nlm_host *
 /*
  * Host cache
  */
-struct nlm_host * nlmclnt_lookup_host(struct sockaddr_in *, int, int);
+struct nlm_host * nlmclnt_lookup_host(const struct sockaddr_in *, int, int);
 struct nlm_host * nlmsvc_lookup_host(struct svc_rqst *);
-struct nlm_host * nlm_lookup_host(int server, struct sockaddr_in *, int, int);
+struct nlm_host * nlm_lookup_host(int server, const struct sockaddr_in *, int, int);
 struct rpc_clnt * nlm_bind_host(struct nlm_host *);
 void		  nlm_rebind_host(struct nlm_host *);
 struct nlm_host * nlm_get_host(struct nlm_host *);
 void		  nlm_release_host(struct nlm_host *);
 void		  nlm_shutdown_hosts(void);
 extern struct nlm_host *nlm_find_client(void);
+extern void	  nlm_host_rebooted(const struct sockaddr_in *, const struct nlm_reboot *);
 
 
 /*

[PATCH 003 of 19] knfsd: When looking up a lockd host, pass hostname & length

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch adds the peer's hostname (and name length) to
  all calls to nlm*_lookup_host functions. A subsequent patch
  will make use of these (is requested by a sysctl).

Signed-off-by: okir@suse.de
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/clntproc.c         |    5 ++++-
 ./fs/lockd/host.c             |   37 +++++++++++++++++++++++++------------
 ./fs/lockd/svc4proc.c         |    6 ++++--
 ./fs/lockd/svclock.c          |    3 ++-
 ./fs/lockd/svcproc.c          |    6 ++++--
 ./include/linux/lockd/lockd.h |    6 +++---
 6 files changed, 42 insertions(+), 21 deletions(-)

diff .prev/fs/lockd/clntproc.c ./fs/lockd/clntproc.c
--- .prev/fs/lockd/clntproc.c	2006-08-31 16:59:10.000000000 +1000
+++ ./fs/lockd/clntproc.c	2006-08-31 16:32:15.000000000 +1000
@@ -153,6 +153,7 @@ nlmclnt_proc(struct inode *inode, int cm
 {
 	struct rpc_clnt		*client = NFS_CLIENT(inode);
 	struct sockaddr_in	addr;
+	struct nfs_server	*nfssrv = NFS_SERVER(inode);
 	struct nlm_host		*host;
 	struct nlm_rqst		*call;
 	sigset_t		oldset;
@@ -166,7 +167,9 @@ nlmclnt_proc(struct inode *inode, int cm
 	}
 
 	rpc_peeraddr(client, (struct sockaddr *) &addr, sizeof(addr));
-	host = nlmclnt_lookup_host(&addr, client->cl_xprt->prot, vers);
+	host = nlmclnt_lookup_host(&addr, client->cl_xprt->prot, vers,
+				   nfssrv->nfs_client->cl_hostname,
+				   strlen(nfssrv->nfs_client->cl_hostname));
 	if (host == NULL)
 		return -ENOLCK;
 

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 16:59:10.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 16:23:12.000000000 +1000
@@ -39,19 +39,23 @@ static void			nlm_gc_hosts(void);
  * Find an NLM server handle in the cache. If there is none, create it.
  */
 struct nlm_host *
-nlmclnt_lookup_host(const struct sockaddr_in *sin, int proto, int version)
+nlmclnt_lookup_host(const struct sockaddr_in *sin, int proto, int version,
+			const char *hostname, int hostname_len)
 {
-	return nlm_lookup_host(0, sin, proto, version);
+	return nlm_lookup_host(0, sin, proto, version,
+			       hostname, hostname_len);
 }
 
 /*
  * Find an NLM client handle in the cache. If there is none, create it.
  */
 struct nlm_host *
-nlmsvc_lookup_host(struct svc_rqst *rqstp)
+nlmsvc_lookup_host(struct svc_rqst *rqstp,
+			const char *hostname, int hostname_len)
 {
 	return nlm_lookup_host(1, &rqstp->rq_addr,
-			       rqstp->rq_prot, rqstp->rq_vers);
+			       rqstp->rq_prot, rqstp->rq_vers,
+			       hostname, hostname_len);
 }
 
 /*
@@ -59,14 +63,20 @@ nlmsvc_lookup_host(struct svc_rqst *rqst
  */
 struct nlm_host *
 nlm_lookup_host(int server, const struct sockaddr_in *sin,
-					int proto, int version)
+					int proto, int version,
+					const char *hostname,
+					int hostname_len)
 {
 	struct nlm_host	*host, **hp;
 	u32		addr;
 	int		hash;
 
-	dprintk("lockd: nlm_lookup_host(%08x, p=%d, v=%d)\n",
-			(unsigned)(sin? ntohl(sin->sin_addr.s_addr) : 0), proto, version);
+	dprintk("lockd: nlm_lookup_host(%u.%u.%u.%u, p=%d, v=%d, my role=%s, name=%.*s)\n",
+			NIPQUAD(sin->sin_addr.s_addr), proto, version,
+			server? "server" : "client",
+			hostname_len,
+			hostname? hostname : "<none>");
+
 
 	hash = NLM_ADDRHASH(sin->sin_addr.s_addr);
 
@@ -267,19 +277,22 @@ void nlm_release_host(struct nlm_host *h
 void nlm_host_rebooted(const struct sockaddr_in *sin, const struct nlm_reboot *argp)
 {
 	struct nlm_host *host;
+	int server;
 
 	/* Obtain the host pointer for this NFS server and try to
 	 * reclaim all locks we hold on this server.
 	 */
-	if ((argp->proto & 1)==0) {
+	server = (argp->proto & 1)? 1 : 0;
+	host = nlm_lookup_host(server, sin, argp->proto >> 1, argp->vers,
+			argp->mon, argp->len);
+	if (host == NULL)
+		return;
+
+	if (server == 0) {
 		/* We are client, he's the server: try to reclaim all locks. */
-		if ((host = nlmclnt_lookup_host(sin, argp->proto >> 1, argp->vers)) == NULL)
-			return;
 		nlmclnt_recovery(host, argp->state);
 	} else {
 		/* He's the client, we're the server: delete all locks held by the client */
-		if ((host = nlm_lookup_host(1, sin, argp->proto >> 1, argp->vers)) == NULL)
-			return;
 		nlmsvc_free_host_resources(host);
 	}
 	nlm_release_host(host);

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-08-31 16:59:10.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-08-31 16:59:53.000000000 +1000
@@ -38,7 +38,7 @@ nlm4svc_retrieve_args(struct svc_rqst *r
 		return nlm_lck_denied_nolocks;
 
 	/* Obtain host handle */
-	if (!(host = nlmsvc_lookup_host(rqstp))
+	if (!(host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len))
 	 || (argp->monitor && nsm_monitor(host) < 0))
 		goto no_locks;
 	*hostp = host;
@@ -260,7 +260,9 @@ static int nlm4svc_callback(struct svc_r
 	struct nlm_rqst	*call;
 	int stat;
 
-	host = nlmsvc_lookup_host(rqstp);
+	host = nlmsvc_lookup_host(rqstp,
+				  argp->lock.caller,
+				  argp->lock.len);
 	if (host == NULL)
 		return rpc_system_err;
 

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-08-31 16:59:10.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-08-31 16:23:12.000000000 +1000
@@ -179,7 +179,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
 	struct nlm_rqst		*call = NULL;
 
 	/* Create host handle for callback */
-	host = nlmsvc_lookup_host(rqstp);
+	host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
 	if (host == NULL)
 		return NULL;
 
@@ -451,6 +451,7 @@ nlmsvc_testlock(struct nlm_file *file, s
 				(long long)conflock->fl.fl_start,
 				(long long)conflock->fl.fl_end);
 		conflock->caller = "somehost";	/* FIXME */
+		conflock->len = strlen(conflock->caller);
 		conflock->oh.len = 0;		/* don't return OH info */
 		conflock->svid = conflock->fl.fl_pid;
 		return nlm_lck_denied;

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-08-31 16:59:10.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-08-31 16:59:39.000000000 +1000
@@ -66,7 +66,7 @@ nlmsvc_retrieve_args(struct svc_rqst *rq
 		return nlm_lck_denied_nolocks;
 
 	/* Obtain host handle */
-	if (!(host = nlmsvc_lookup_host(rqstp))
+	if (!(host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len))
 	 || (argp->monitor && nsm_monitor(host) < 0))
 		goto no_locks;
 	*hostp = host;
@@ -287,7 +287,9 @@ static int nlmsvc_callback(struct svc_rq
 	struct nlm_rqst	*call;
 	int stat;
 
-	host = nlmsvc_lookup_host(rqstp);
+	host = nlmsvc_lookup_host(rqstp,
+				  argp->lock.caller,
+				  argp->lock.len);
 	if (host == NULL)
 		return rpc_system_err;
 

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-08-31 16:59:10.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-08-31 16:23:12.000000000 +1000
@@ -161,9 +161,9 @@ int		  nlmclnt_reclaim(struct nlm_host *
 /*
  * Host cache
  */
-struct nlm_host * nlmclnt_lookup_host(const struct sockaddr_in *, int, int);
-struct nlm_host * nlmsvc_lookup_host(struct svc_rqst *);
-struct nlm_host * nlm_lookup_host(int server, const struct sockaddr_in *, int, int);
+struct nlm_host * nlmclnt_lookup_host(const struct sockaddr_in *, int, int, const char *, int);
+struct nlm_host * nlmsvc_lookup_host(struct svc_rqst *, const char *, int);
+struct nlm_host * nlm_lookup_host(int server, const struct sockaddr_in *, int, int, const char *, int);
 struct rpc_clnt * nlm_bind_host(struct nlm_host *);
 void		  nlm_rebind_host(struct nlm_host *);
 struct nlm_host * nlm_get_host(struct nlm_host *);

[PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch introduces the nsm_handle, which is shared by
  all nlm_host objects referring to the same client.

  With this patch applied, all nlm_hosts from the same address
  will share the same nsm_handle. A future patch will add sharing
  by name.

  Note: this patch changes h_name so that it is no longer guaranteed
  to be an IP address of the host.  When the host represents an NFS server,
  h_name will be the name passed in the mount call.  When the host
  represents a client, h_name will be the name presented in the lock
  request received from the client.  A h_name is only used for printing
  informational messages, this change should not be significant.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/clntlock.c         |    3 -
 ./fs/lockd/host.c             |  121 ++++++++++++++++++++++++++++++++++++++----
 ./fs/lockd/mon.c              |   14 +++-
 ./include/linux/lockd/lockd.h |   17 ++++-
 4 files changed, 136 insertions(+), 19 deletions(-)

diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
--- .prev/fs/lockd/clntlock.c	2006-08-31 16:44:23.000000000 +1000
+++ ./fs/lockd/clntlock.c	2006-08-31 17:00:03.000000000 +1000
@@ -150,7 +150,8 @@ u32 nlmclnt_grant(const struct sockaddr_
 static void nlmclnt_prepare_reclaim(struct nlm_host *host)
 {
 	down_write(&host->h_rwsem);
-	host->h_monitored = 0;
+	if (host->h_nsmhandle)
+		host->h_nsmhandle->sm_monitored = 0;
 	host->h_state++;
 	host->h_nextrebind = 0;
 	nlm_rebind_host(host);

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 16:23:12.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 17:00:03.000000000 +1000
@@ -34,6 +34,8 @@ static DEFINE_MUTEX(nlm_host_mutex);
 
 
 static void			nlm_gc_hosts(void);
+static struct nsm_handle *	__nsm_find(const struct sockaddr_in *,
+					const char *, int, int);
 
 /*
  * Find an NLM server handle in the cache. If there is none, create it.
@@ -68,7 +70,7 @@ nlm_lookup_host(int server, const struct
 					int hostname_len)
 {
 	struct nlm_host	*host, **hp;
-	u32		addr;
+	struct nsm_handle *nsm = NULL;
 	int		hash;
 
 	dprintk("lockd: nlm_lookup_host(%u.%u.%u.%u, p=%d, v=%d, my role=%s, name=%.*s)\n",
@@ -86,7 +88,21 @@ nlm_lookup_host(int server, const struct
 	if (time_after_eq(jiffies, next_gc))
 		nlm_gc_hosts();
 
+	/* We may keep several nlm_host objects for a peer, because each
+	 * nlm_host is identified by
+	 * (address, protocol, version, server/client)
+	 * We could probably simplify this a little by putting all those
+	 * different NLM rpc_clients into one single nlm_host object.
+	 * This would allow us to have one nlm_host per address.
+	 */
 	for (hp = &nlm_hosts[hash]; (host = *hp) != 0; hp = &host->h_next) {
+		if (!nlm_cmp_addr(&host->h_addr, sin))
+			continue;
+
+		/* See if we have an NSM handle for this client */
+		if (!nsm && (nsm = host->h_nsmhandle) != 0)
+			atomic_inc(&nsm->sm_count);
+
 		if (host->h_proto != proto)
 			continue;
 		if (host->h_version != version)
@@ -94,7 +110,7 @@ nlm_lookup_host(int server, const struct
 		if (host->h_server != server)
 			continue;
 
-		if (nlm_cmp_addr(&host->h_addr, sin)) {
+		{
 			if (hp != nlm_hosts + hash) {
 				*hp = host->h_next;
 				host->h_next = nlm_hosts[hash];
@@ -106,16 +122,18 @@ nlm_lookup_host(int server, const struct
 		}
 	}
 
-	/* Ooops, no host found, create it */
-	dprintk("lockd: creating host entry\n");
+	/* Sadly, the host isn't in our hash table yet. See if
+	 * we have an NSM handle for it. If not, create one.
+	 */
+	if (!nsm && !(nsm = nsm_find(sin, hostname, hostname_len)))
+		goto out;
 
 	host = kzalloc(sizeof(*host), GFP_KERNEL);
-	if (!host)
-		goto nohost;
-
-	addr = sin->sin_addr.s_addr;
-	sprintf(host->h_name, "%u.%u.%u.%u", NIPQUAD(addr));
-
+	if (!host) {
+		nsm_release(nsm);
+		goto out;
+	}
+	host->h_name	   = nsm->sm_name;
 	host->h_addr       = *sin;
 	host->h_addr.sin_port = 0;	/* ouch! */
 	host->h_version    = version;
@@ -129,6 +147,7 @@ nlm_lookup_host(int server, const struct
 	init_rwsem(&host->h_rwsem);
 	host->h_state      = 0;			/* pseudo NSM state */
 	host->h_nsmstate   = 0;			/* real NSM state */
+	host->h_nsmhandle  = nsm;
 	host->h_server	   = server;
 	host->h_next       = nlm_hosts[hash];
 	nlm_hosts[hash]    = host;
@@ -140,7 +159,7 @@ nlm_lookup_host(int server, const struct
 	if (++nrhosts > NLM_HOST_MAX)
 		next_gc = 0;
 
-nohost:
+out:
 	mutex_unlock(&nlm_host_mutex);
 	return host;
 }
@@ -393,3 +412,83 @@ nlm_gc_hosts(void)
 	next_gc = jiffies + NLM_HOST_COLLECT;
 }
 
+
+/*
+ * Manage NSM handles
+ */
+static LIST_HEAD(nsm_handles);
+static DECLARE_MUTEX(nsm_sema);
+
+static struct nsm_handle *
+__nsm_find(const struct sockaddr_in *sin,
+		const char *hostname, int hostname_len,
+		int create)
+{
+	struct nsm_handle *nsm = NULL;
+	struct list_head *pos;
+
+	if (!sin)
+		return NULL;
+
+	if (hostname && memchr(hostname, '/', hostname_len) != NULL) {
+		if (printk_ratelimit()) {
+			printk(KERN_WARNING "Invalid hostname \"%.*s\" "
+					    "in NFS lock request\n",
+				hostname_len, hostname);
+		}
+		return NULL;
+	}
+
+	down(&nsm_sema);
+	list_for_each(pos, &nsm_handles) {
+		nsm = list_entry(pos, struct nsm_handle, sm_link);
+
+		if (!nlm_cmp_addr(&nsm->sm_addr, sin))
+			continue;
+		atomic_inc(&nsm->sm_count);
+		goto out;
+	}
+
+	if (!create) {
+		nsm = NULL;
+		goto out;
+	}
+
+	nsm = kzalloc(sizeof(*nsm) + hostname_len + 1, GFP_KERNEL);
+	if (nsm != NULL) {
+		nsm->sm_addr = *sin;
+		nsm->sm_name = (char *) (nsm + 1);
+		memcpy(nsm->sm_name, hostname, hostname_len);
+		nsm->sm_name[hostname_len] = '\0';
+		atomic_set(&nsm->sm_count, 1);
+
+		list_add(&nsm->sm_link, &nsm_handles);
+	}
+
+out:	up(&nsm_sema);
+	return nsm;
+}
+
+struct nsm_handle *
+nsm_find(const struct sockaddr_in *sin, const char *hostname, int hostname_len)
+{
+	return __nsm_find(sin, hostname, hostname_len, 1);
+}
+
+/*
+ * Release an NSM handle
+ */
+void
+nsm_release(struct nsm_handle *nsm)
+{
+	if (!nsm)
+		return;
+	if (atomic_read(&nsm->sm_count) == 1) {
+		down(&nsm_sema);
+		if (atomic_dec_and_test(&nsm->sm_count)) {
+			list_del(&nsm->sm_link);
+			kfree(nsm);
+		}
+		up(&nsm_sema);
+	}
+}

diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
--- .prev/fs/lockd/mon.c	2006-08-31 16:12:30.000000000 +1000
+++ ./fs/lockd/mon.c	2006-08-31 17:00:03.000000000 +1000
@@ -70,11 +70,14 @@ nsm_mon_unmon(struct nlm_host *host, u32
 int
 nsm_monitor(struct nlm_host *host)
 {
+	struct nsm_handle *nsm = host->h_nsmhandle;
 	struct nsm_res	res;
 	int		status;
 
 	dprintk("lockd: nsm_monitor(%s)\n", host->h_name);
-	if (host->h_monitored)
+	BUG_ON(nsm == NULL);
+
+	if (nsm->sm_monitored)
 		return 0;
 
 	status = nsm_mon_unmon(host, SM_MON, &res);
@@ -82,7 +85,7 @@ nsm_monitor(struct nlm_host *host)
 	if (status < 0 || res.status != 0)
 		printk(KERN_NOTICE "lockd: cannot monitor %s\n", host->h_name);
 	else
-		host->h_monitored = 1;
+		nsm->sm_monitored = 1;
 	return status;
 }
 
@@ -92,19 +95,22 @@ nsm_monitor(struct nlm_host *host)
 int
 nsm_unmonitor(struct nlm_host *host)
 {
+	struct nsm_handle *nsm = host->h_nsmhandle;
 	struct nsm_res	res;
 	int		status = 0;
 
 	dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
-	if (!host->h_monitored)
+	if (nsm == NULL)
 		return 0;
-	host->h_monitored = 0;
+	host->h_nsmhandle = NULL;
 
 	if (!host->h_killed) {
 		status = nsm_mon_unmon(host, SM_UNMON, &res);
 		if (status < 0)
 			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", host->h_name);
+		nsm->sm_monitored = 0;
 	}
+	nsm_release(nsm);
 	return status;
 }
 

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-08-31 16:23:12.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-08-31 17:00:03.000000000 +1000
@@ -40,14 +40,13 @@ struct nlm_host {
 	struct nlm_host *	h_next;		/* linked list (hash table) */
 	struct sockaddr_in	h_addr;		/* peer address */
 	struct rpc_clnt	*	h_rpcclnt;	/* RPC client to talk to peer */
-	char			h_name[20];	/* remote hostname */
+	char *			h_name;		/* remote hostname */
 	u32			h_version;	/* interface version */
 	unsigned short		h_proto;	/* transport proto */
 	unsigned short		h_reclaiming : 1,
 				h_server     : 1, /* server side, not client side */
 				h_inuse      : 1,
-				h_killed     : 1,
-				h_monitored  : 1;
+				h_killed     : 1;
 	wait_queue_head_t	h_gracewait;	/* wait while reclaiming */
 	struct rw_semaphore	h_rwsem;	/* Reboot recovery lock */
 	u32			h_state;	/* pseudo-state counter */
@@ -61,6 +60,16 @@ struct nlm_host {
 	spinlock_t		h_lock;
 	struct list_head	h_granted;	/* Locks in GRANTED state */
 	struct list_head	h_reclaim;	/* Locks in RECLAIM state */
+	struct nsm_handle *	h_nsmhandle;	/* NSM status handle */
+};
+
+struct nsm_handle {
+	struct list_head	sm_link;
+	atomic_t		sm_count;
+	char *			sm_name;
+	struct sockaddr_in	sm_addr;
+	unsigned int		sm_monitored : 1,
+				sm_sticky : 1;	/* don't unmonitor */
 };
 
 /*
@@ -171,6 +180,8 @@ void		  nlm_release_host(struct nlm_host
 void		  nlm_shutdown_hosts(void);
 extern struct nlm_host *nlm_find_client(void);
 extern void	  nlm_host_rebooted(const struct sockaddr_in *, const struct nlm_reboot *);
+struct nsm_handle *nsm_find(const struct sockaddr_in *, const char *, int);
+void		  nsm_release(struct nsm_handle *);
 
 
 /*

Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Andrew Morton]

On Fri, 1 Sep 2006 14:38:25 +1000
NeilBrown <neilb@suse.de> wrote:

> +static DECLARE_MUTEX(nsm_sema);
> ...
> +	down(&nsm_sema);

Next you'll all be wearing bell-bottomed jeans?

Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Neil Brown]

On Thursday August 31, akpm@osdl.org wrote:
> On Fri, 1 Sep 2006 14:38:25 +1000
> NeilBrown <neilb@suse.de> wrote:
> 
> > +static DECLARE_MUTEX(nsm_sema);
> > ...
> > +	down(&nsm_sema);
> 
> Next you'll all be wearing bell-bottomed jeans?

You've been peeking in my wardrobe, haven't you :-)

NeilBrown


---
Subject: Convert lockd to use the newer mutex instead of the older semaphore

Both the (recently introduces) nsm_sema and the older f_sema are
converted over.

Cc: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c             |   11 ++++++-----
 ./fs/lockd/svclock.c          |   22 +++++++++++-----------
 ./fs/lockd/svcsubs.c          |    2 +-
 ./include/linux/lockd/lockd.h |    2 +-
 4 files changed, 19 insertions(+), 18 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 12:26:45.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-01 18:56:39.000000000 +1000
@@ -436,7 +436,7 @@ nlm_gc_hosts(void)
  * Manage NSM handles
  */
 static LIST_HEAD(nsm_handles);
-static DECLARE_MUTEX(nsm_sema);
+static DEFINE_MUTEX(nsm_mutex);
 
 static struct nsm_handle *
 __nsm_find(const struct sockaddr_in *sin,
@@ -458,7 +458,7 @@ __nsm_find(const struct sockaddr_in *sin
 		return NULL;
 	}
 
-	down(&nsm_sema);
+	mutex_lock(&nsm_mutex);
 	list_for_each(pos, &nsm_handles) {
 		nsm = list_entry(pos, struct nsm_handle, sm_link);
 
@@ -488,7 +488,8 @@ __nsm_find(const struct sockaddr_in *sin
 		list_add(&nsm->sm_link, &nsm_handles);
 	}
 
-out:	up(&nsm_sema);
+out:
+	mutex_unlock(&nsm_mutex);
 	return nsm;
 }
 
@@ -507,11 +508,11 @@ nsm_release(struct nsm_handle *nsm)
 	if (!nsm)
 		return;
 	if (atomic_read(&nsm->sm_count) == 1) {
-		down(&nsm_sema);
+		mutex_lock(&nsm_mutex);
 		if (atomic_dec_and_test(&nsm->sm_count)) {
 			list_del(&nsm->sm_link);
 			kfree(nsm);
 		}
-		up(&nsm_sema);
+		mutex_unlock(&nsm_mutex);
 	}
 }

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-09-01 12:17:21.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-09-01 18:54:53.000000000 +1000
@@ -254,9 +254,9 @@ static void nlmsvc_free_block(struct kre
 	dprintk("lockd: freeing block %p...\n", block);
 
 	/* Remove block from file's list of blocks */
-	down(&file->f_sema);
+	mutex_lock(&file->f_mutex);
 	list_del_init(&block->b_flist);
-	up(&file->f_sema);
+	mutex_unlock(&file->f_mutex);
 
 	nlmsvc_freegrantargs(block->b_call);
 	nlm_release_call(block->b_call);
@@ -281,7 +281,7 @@ void nlmsvc_traverse_blocks(struct nlm_h
 	struct nlm_block *block, *next;
 
 restart:
-	down(&file->f_sema);
+	mutex_lock(&file->f_mutex);
 	list_for_each_entry_safe(block, next, &file->f_blocks, b_flist) {
 		if (!match(block->b_host, host))
 			continue;
@@ -290,12 +290,12 @@ restart:
 		if (list_empty(&block->b_list))
 			continue;
 		kref_get(&block->b_count);
-		up(&file->f_sema);
+		mutex_unlock(&file->f_mutex);
 		nlmsvc_unlink_block(block);
 		nlmsvc_release_block(block);
 		goto restart;
 	}
-	up(&file->f_sema);
+	mutex_unlock(&file->f_mutex);
 }
 
 /*
@@ -354,7 +354,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, stru
 	lock->fl.fl_flags &= ~FL_SLEEP;
 again:
 	/* Lock file against concurrent access */
-	down(&file->f_sema);
+	mutex_lock(&file->f_mutex);
 	/* Get existing block (in case client is busy-waiting) */
 	block = nlmsvc_lookup_block(file, lock);
 	if (block == NULL) {
@@ -392,10 +392,10 @@ again:
 
 	/* If we don't have a block, create and initialize it. Then
 	 * retry because we may have slept in kmalloc. */
-	/* We have to release f_sema as nlmsvc_create_block may try to
+	/* We have to release f_mutex as nlmsvc_create_block may try to
 	 * to claim it while doing host garbage collection */
 	if (newblock == NULL) {
-		up(&file->f_sema);
+		mutex_unlock(&file->f_mutex);
 		dprintk("lockd: blocking on this lock (allocating).\n");
 		if (!(newblock = nlmsvc_create_block(rqstp, file, lock, cookie)))
 			return nlm_lck_denied_nolocks;
@@ -405,7 +405,7 @@ again:
 	/* Append to list of blocked */
 	nlmsvc_insert_block(newblock, NLM_NEVER);
 out:
-	up(&file->f_sema);
+	mutex_unlock(&file->f_mutex);
 	nlmsvc_release_block(newblock);
 	nlmsvc_release_block(block);
 	dprintk("lockd: nlmsvc_lock returned %u\n", ret);
@@ -489,9 +489,9 @@ nlmsvc_cancel_blocked(struct nlm_file *f
 				(long long)lock->fl.fl_start,
 				(long long)lock->fl.fl_end);
 
-	down(&file->f_sema);
+	mutex_lock(&file->f_mutex);
 	block = nlmsvc_lookup_block(file, lock);
-	up(&file->f_sema);
+	mutex_unlock(&file->f_mutex);
 	if (block != NULL) {
 		status = nlmsvc_unlink_block(block);
 		nlmsvc_release_block(block);

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-09-01 11:38:14.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-09-01 18:55:56.000000000 +1000
@@ -106,7 +106,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 		goto out_unlock;
 
 	memcpy(&file->f_handle, f, sizeof(struct nfs_fh));
-	init_MUTEX(&file->f_sema);
+	mutex_init(&file->f_mutex);
 	INIT_HLIST_NODE(&file->f_list);
 	INIT_LIST_HEAD(&file->f_blocks);
 

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 12:26:43.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 18:50:32.000000000 +1000
@@ -111,7 +111,7 @@ struct nlm_file {
 	struct list_head	f_blocks;	/* blocked locks */
 	unsigned int		f_locks;	/* guesstimate # of locks */
 	unsigned int		f_count;	/* reference count */
-	struct semaphore	f_sema;		/* avoid concurrent access */
+	struct mutex		f_mutex;	/* avoid concurrent access */
 };
 
 /*

-- 
VGER BF report: H 2.88825e-13

Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Andrew Morton]

On Fri, 1 Sep 2006 14:38:25 +1000
NeilBrown <neilb@suse.de> wrote:

> +nsm_release(struct nsm_handle *nsm)
> +{
> +	if (!nsm)
> +		return;
> +	if (atomic_read(&nsm->sm_count) == 1) {
> +		down(&nsm_sema);
> +		if (atomic_dec_and_test(&nsm->sm_count)) {
> +			list_del(&nsm->sm_link);
> +			kfree(nsm);
> +		}
> +		up(&nsm_sema);
> +	}
> +}

That's weird-looking.  Are you sure?  afaict, if ->sm_count ever gets a
value of 2 or more, it's unfreeable.  

Re: [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Neil Brown]

On Thursday August 31, akpm@osdl.org wrote:
> On Fri, 1 Sep 2006 14:38:25 +1000
> NeilBrown <neilb@suse.de> wrote:
> 
> > +nsm_release(struct nsm_handle *nsm)
> > +{
> > +	if (!nsm)
> > +		return;
> > +	if (atomic_read(&nsm->sm_count) == 1) {
> > +		down(&nsm_sema);
> > +		if (atomic_dec_and_test(&nsm->sm_count)) {
> > +			list_del(&nsm->sm_link);
> > +			kfree(nsm);
> > +		}
> > +		up(&nsm_sema);
> > +	}
> > +}
> 
> That's weird-looking.  Are you sure?  afaict, if ->sm_count ever gets a
> value of 2 or more, it's unfreeable.  

How do you *do* that?  I thought I had already found the bugs...

Thanks :-)
NeilBrown


---
Subject: Fix locking in nsm_release

The locking is all backwards and broken.

We first atomic_dec_and_test.
If this fails, someone else has an active reference and we need do
no more.
If it succeeds, then the only ref is in the hash table, but someone
might be about to find and use that reference.  nsm_mutex provides
exclusion against this.  If sm_count is still 0 once the
mutex has been gained, then it is safe to discard the nsm.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 18:56:39.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-02 09:44:04.000000000 +1000
@@ -507,9 +507,9 @@ nsm_release(struct nsm_handle *nsm)
 {
 	if (!nsm)
 		return;
-	if (atomic_read(&nsm->sm_count) == 1) {
+	if (atomic_dec_and_test(&nsm->sm_count)) {
 		mutex_lock(&nsm_mutex);
-		if (atomic_dec_and_test(&nsm->sm_count)) {
+		if (atomic_read(&nsm->sm_count) == 0) {
 			list_del(&nsm->sm_link);
 			kfree(nsm);
 		}

-- 
VGER BF report: H 0

Re: [NFS] [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Trond Myklebust]

On Fri, 2006-09-01 at 14:38 +1000, NeilBrown wrote:
> From: Olaf Kirch <okir@suse.de>
> 
>   This patch introduces the nsm_handle, which is shared by
>   all nlm_host objects referring to the same client.

>   With this patch applied, all nlm_hosts from the same address
>   will share the same nsm_handle. A future patch will add sharing
>   by name.

<boggle>
Exactly why is it desirable to have > 1 nlm_host from the same address?
</boggle>

If we can map several clients into a single nsm_handle, then surely it
makes sense to map them into the same nlm_host too.

>   Note: this patch changes h_name so that it is no longer guaranteed
>   to be an IP address of the host.  When the host represents an NFS server,
>   h_name will be the name passed in the mount call.  When the host
>   represents a client, h_name will be the name presented in the lock
>   request received from the client.  A h_name is only used for printing
>   informational messages, this change should not be significant.

> Signed-off-by: Olaf Kirch <okir@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
> 
> ### Diffstat output
>  ./fs/lockd/clntlock.c         |    3 -
>  ./fs/lockd/host.c             |  121 ++++++++++++++++++++++++++++++++++++++----
>  ./fs/lockd/mon.c              |   14 +++-
>  ./include/linux/lockd/lockd.h |   17 ++++-
>  4 files changed, 136 insertions(+), 19 deletions(-)
> 
> diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
> --- .prev/fs/lockd/clntlock.c	2006-08-31 16:44:23.000000000 +1000
> +++ ./fs/lockd/clntlock.c	2006-08-31 17:00:03.000000000 +1000
> @@ -150,7 +150,8 @@ u32 nlmclnt_grant(const struct sockaddr_
>  static void nlmclnt_prepare_reclaim(struct nlm_host *host)
>  {
>  	down_write(&host->h_rwsem);
> -	host->h_monitored = 0;
> +	if (host->h_nsmhandle)
> +		host->h_nsmhandle->sm_monitored = 0;
>  	host->h_state++;
>  	host->h_nextrebind = 0;
>  	nlm_rebind_host(host);
> 
> diff .prev/fs/lockd/host.c ./fs/lockd/host.c
> --- .prev/fs/lockd/host.c	2006-08-31 16:23:12.000000000 +1000
> +++ ./fs/lockd/host.c	2006-08-31 17:00:03.000000000 +1000
> @@ -34,6 +34,8 @@ static DEFINE_MUTEX(nlm_host_mutex);
>  
> 
>  static void			nlm_gc_hosts(void);
> +static struct nsm_handle *	__nsm_find(const struct sockaddr_in *,
> +					const char *, int, int);
>  
>  /*
>   * Find an NLM server handle in the cache. If there is none, create it.
> @@ -68,7 +70,7 @@ nlm_lookup_host(int server, const struct
>  					int hostname_len)
>  {
>  	struct nlm_host	*host, **hp;
> -	u32		addr;
> +	struct nsm_handle *nsm = NULL;
>  	int		hash;
>  
>  	dprintk("lockd: nlm_lookup_host(%u.%u.%u.%u, p=%d, v=%d, my role=%s, name=%.*s)\n",
> @@ -86,7 +88,21 @@ nlm_lookup_host(int server, const struct
>  	if (time_after_eq(jiffies, next_gc))
>  		nlm_gc_hosts();
>  
> +	/* We may keep several nlm_host objects for a peer, because each
> +	 * nlm_host is identified by
> +	 * (address, protocol, version, server/client)
> +	 * We could probably simplify this a little by putting all those
> +	 * different NLM rpc_clients into one single nlm_host object.
> +	 * This would allow us to have one nlm_host per address.
> +	 */
>  	for (hp = &nlm_hosts[hash]; (host = *hp) != 0; hp = &host->h_next) {
> +		if (!nlm_cmp_addr(&host->h_addr, sin))
> +			continue;
> +
> +		/* See if we have an NSM handle for this client */
> +		if (!nsm && (nsm = host->h_nsmhandle) != 0)
> +			atomic_inc(&nsm->sm_count);
> +
>  		if (host->h_proto != proto)
>  			continue;
>  		if (host->h_version != version)
> @@ -94,7 +110,7 @@ nlm_lookup_host(int server, const struct
>  		if (host->h_server != server)
>  			continue;
>  
> -		if (nlm_cmp_addr(&host->h_addr, sin)) {
> +		{
>  			if (hp != nlm_hosts + hash) {
>  				*hp = host->h_next;
>  				host->h_next = nlm_hosts[hash];
> @@ -106,16 +122,18 @@ nlm_lookup_host(int server, const struct
>  		}
>  	}
>  
> -	/* Ooops, no host found, create it */
> -	dprintk("lockd: creating host entry\n");
> +	/* Sadly, the host isn't in our hash table yet. See if
> +	 * we have an NSM handle for it. If not, create one.
> +	 */
> +	if (!nsm && !(nsm = nsm_find(sin, hostname, hostname_len)))
> +		goto out;
>  
>  	host = kzalloc(sizeof(*host), GFP_KERNEL);
> -	if (!host)
> -		goto nohost;
> -
> -	addr = sin->sin_addr.s_addr;
> -	sprintf(host->h_name, "%u.%u.%u.%u", NIPQUAD(addr));
> -
> +	if (!host) {
> +		nsm_release(nsm);
> +		goto out;
> +	}
> +	host->h_name	   = nsm->sm_name;
>  	host->h_addr       = *sin;
>  	host->h_addr.sin_port = 0;	/* ouch! */
>  	host->h_version    = version;
> @@ -129,6 +147,7 @@ nlm_lookup_host(int server, const struct
>  	init_rwsem(&host->h_rwsem);
>  	host->h_state      = 0;			/* pseudo NSM state */
>  	host->h_nsmstate   = 0;			/* real NSM state */
> +	host->h_nsmhandle  = nsm;
>  	host->h_server	   = server;
>  	host->h_next       = nlm_hosts[hash];
>  	nlm_hosts[hash]    = host;
> @@ -140,7 +159,7 @@ nlm_lookup_host(int server, const struct
>  	if (++nrhosts > NLM_HOST_MAX)
>  		next_gc = 0;
>  
> -nohost:
> +out:
>  	mutex_unlock(&nlm_host_mutex);
>  	return host;
>  }
> @@ -393,3 +412,83 @@ nlm_gc_hosts(void)
>  	next_gc = jiffies + NLM_HOST_COLLECT;
>  }
>  
> +
> +/*
> + * Manage NSM handles
> + */
> +static LIST_HEAD(nsm_handles);
> +static DECLARE_MUTEX(nsm_sema);
> +
> +static struct nsm_handle *
> +__nsm_find(const struct sockaddr_in *sin,
> +		const char *hostname, int hostname_len,
> +		int create)
> +{
> +	struct nsm_handle *nsm = NULL;
> +	struct list_head *pos;
> +
> +	if (!sin)
> +		return NULL;
> +
> +	if (hostname && memchr(hostname, '/', hostname_len) != NULL) {
> +		if (printk_ratelimit()) {
> +			printk(KERN_WARNING "Invalid hostname \"%.*s\" "
> +					    "in NFS lock request\n",
> +				hostname_len, hostname);
> +		}
> +		return NULL;
> +	}
> +
> +	down(&nsm_sema);
^^^^^^^^^^^^^^^^^^^^^^^^^^

Growl! Can we do this properly (i.e using spinlocks)? This semaphore
crap may help simplify the code for adding new entries, but it is
counterproductive as far as scalability goes, and leads straight down
the path of damnation in the form of the broken 'atomic_read()'
heuristics in nsm_release().

> +	list_for_each(pos, &nsm_handles) {
> +		nsm = list_entry(pos, struct nsm_handle, sm_link);
> +
> +		if (!nlm_cmp_addr(&nsm->sm_addr, sin))
> +			continue;
> +		atomic_inc(&nsm->sm_count);
> +		goto out;
> +	}
> +
> +	if (!create) {
> +		nsm = NULL;
> +		goto out;
> +	}
> +
> +	nsm = kzalloc(sizeof(*nsm) + hostname_len + 1, GFP_KERNEL);
> +	if (nsm != NULL) {
> +		nsm->sm_addr = *sin;
> +		nsm->sm_name = (char *) (nsm + 1);
> +		memcpy(nsm->sm_name, hostname, hostname_len);
> +		nsm->sm_name[hostname_len] = '\0';
> +		atomic_set(&nsm->sm_count, 1);
> +
> +		list_add(&nsm->sm_link, &nsm_handles);
> +	}
> +
> +out:	up(&nsm_sema);
> +	return nsm;
> +}
> +
> +struct nsm_handle *
> +nsm_find(const struct sockaddr_in *sin, const char *hostname, int hostname_len)
> +{
> +	return __nsm_find(sin, hostname, hostname_len, 1);
> +}
> +
> +/*
> + * Release an NSM handle
> + */
> +void
> +nsm_release(struct nsm_handle *nsm)
> +{
> +	if (!nsm)
> +		return;
> +	if (atomic_read(&nsm->sm_count) == 1) {
> +		down(&nsm_sema);
> +		if (atomic_dec_and_test(&nsm->sm_count)) {
> +			list_del(&nsm->sm_link);
> +			kfree(nsm);
> +		}
> +		up(&nsm_sema);
> +	}
> +}

As Andrew pointed out, this function doesn't actually decrement sm_count
if there is more than one reference to it.

atomic_dec_and_lock() is your friend.

> diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
> --- .prev/fs/lockd/mon.c	2006-08-31 16:12:30.000000000 +1000
> +++ ./fs/lockd/mon.c	2006-08-31 17:00:03.000000000 +1000
> @@ -70,11 +70,14 @@ nsm_mon_unmon(struct nlm_host *host, u32
>  int
>  nsm_monitor(struct nlm_host *host)
>  {
> +	struct nsm_handle *nsm = host->h_nsmhandle;
>  	struct nsm_res	res;
>  	int		status;
>  
>  	dprintk("lockd: nsm_monitor(%s)\n", host->h_name);
> -	if (host->h_monitored)
> +	BUG_ON(nsm == NULL);
> +
> +	if (nsm->sm_monitored)
>  		return 0;
>  
>  	status = nsm_mon_unmon(host, SM_MON, &res);
> @@ -82,7 +85,7 @@ nsm_monitor(struct nlm_host *host)
>  	if (status < 0 || res.status != 0)
>  		printk(KERN_NOTICE "lockd: cannot monitor %s\n", host->h_name);
>  	else
> -		host->h_monitored = 1;
> +		nsm->sm_monitored = 1;
>  	return status;
>  }
>  
> @@ -92,19 +95,22 @@ nsm_monitor(struct nlm_host *host)
>  int
>  nsm_unmonitor(struct nlm_host *host)
>  {
> +	struct nsm_handle *nsm = host->h_nsmhandle;
>  	struct nsm_res	res;
>  	int		status = 0;
>  
>  	dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
> -	if (!host->h_monitored)
> +	if (nsm == NULL)
>  		return 0;
> -	host->h_monitored = 0;
> +	host->h_nsmhandle = NULL;
>  
>  	if (!host->h_killed) {
>  		status = nsm_mon_unmon(host, SM_UNMON, &res);
>  		if (status < 0)
>  			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", host->h_name);
> +		nsm->sm_monitored = 0;
>  	}
> +	nsm_release(nsm);
>  	return status;
>  }
>  
> 
> diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
> --- .prev/include/linux/lockd/lockd.h	2006-08-31 16:23:12.000000000 +1000
> +++ ./include/linux/lockd/lockd.h	2006-08-31 17:00:03.000000000 +1000
> @@ -40,14 +40,13 @@ struct nlm_host {
>  	struct nlm_host *	h_next;		/* linked list (hash table) */
>  	struct sockaddr_in	h_addr;		/* peer address */
>  	struct rpc_clnt	*	h_rpcclnt;	/* RPC client to talk to peer */
> -	char			h_name[20];	/* remote hostname */
> +	char *			h_name;		/* remote hostname */
>  	u32			h_version;	/* interface version */
>  	unsigned short		h_proto;	/* transport proto */
>  	unsigned short		h_reclaiming : 1,
>  				h_server     : 1, /* server side, not client side */
>  				h_inuse      : 1,
> -				h_killed     : 1,
> -				h_monitored  : 1;
> +				h_killed     : 1;
>  	wait_queue_head_t	h_gracewait;	/* wait while reclaiming */
>  	struct rw_semaphore	h_rwsem;	/* Reboot recovery lock */
>  	u32			h_state;	/* pseudo-state counter */
> @@ -61,6 +60,16 @@ struct nlm_host {
>  	spinlock_t		h_lock;
>  	struct list_head	h_granted;	/* Locks in GRANTED state */
>  	struct list_head	h_reclaim;	/* Locks in RECLAIM state */
> +	struct nsm_handle *	h_nsmhandle;	/* NSM status handle */
> +};
> +
> +struct nsm_handle {
> +	struct list_head	sm_link;
> +	atomic_t		sm_count;
> +	char *			sm_name;
> +	struct sockaddr_in	sm_addr;
> +	unsigned int		sm_monitored : 1,
> +				sm_sticky : 1;	/* don't unmonitor */
>  };
>  
>  /*
> @@ -171,6 +180,8 @@ void		  nlm_release_host(struct nlm_host
>  void		  nlm_shutdown_hosts(void);
>  extern struct nlm_host *nlm_find_client(void);
>  extern void	  nlm_host_rebooted(const struct sockaddr_in *, const struct nlm_reboot *);
> +struct nsm_handle *nsm_find(const struct sockaddr_in *, const char *, int);
> +void		  nsm_release(struct nsm_handle *);
>  
> 
>  /*
> 
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs

Re: [NFS] [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Olaf Kirch]

On Fri, Sep 01, 2006 at 11:50:20AM -0400, Trond Myklebust wrote:
> >   With this patch applied, all nlm_hosts from the same address
> >   will share the same nsm_handle. A future patch will add sharing
> >   by name.
> 
> <boggle>
> Exactly why is it desirable to have > 1 nlm_host from the same address?
> </boggle>
> 
> If we can map several clients into a single nsm_handle, then surely it
> makes sense to map them into the same nlm_host too.

This is all related to the reasons for introducing NSM notification
by name in the first place.

On the client side, we may have mounted several volumes from a multi-homed
server, using different addresses, and you have several NLM client
handles, each with one of these addresses - and each in a different
nlm_host object.

Or you have an NFS server in a HA configuration, listening on a virtual
address. As the server fails over, the alias moves to the backup
machine.

Or (once we have IPv6) you may have a mix of v4 and v6 mounts.

Now when the server reboots, it will send you one or more SM_NOTIFY
messages. You do not know which addresses it will use. In the multihomed
case, you will get one SM_NOTIFY for each server address if the server
is a Linux box. Other server OSs will send you just one SM_NOTIFY,
and the sender address will be more or less random. In the HA case
described above, the sender address will not match the address
you used at all (since the UDP packet will have the interface's
primary IP address, not the alias).

This is the main motivation for introducing the nsm_handle, and this is
also the reason why there is potentially a 1-to-many relationship between
nsm_handles (representing a "host") and nlm_host, representing a tuple of
(NLM version, transport protocol, address).

Maybe we should rename nlm_host to something less confusing.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

Re: [NFS] [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Trond Myklebust]

On Fri, 2006-09-01 at 18:11 +0200, Olaf Kirch wrote:
> This is all related to the reasons for introducing NSM notification
> by name in the first place.
> 
> On the client side, we may have mounted several volumes from a multi-homed
> server, using different addresses, and you have several NLM client
> handles, each with one of these addresses - and each in a different
> nlm_host object.
> 
> Or you have an NFS server in a HA configuration, listening on a virtual
> address. As the server fails over, the alias moves to the backup
> machine.
> 
> Or (once we have IPv6) you may have a mix of v4 and v6 mounts.
> 
> Now when the server reboots, it will send you one or more SM_NOTIFY
> messages. You do not know which addresses it will use. In the multihomed
> case, you will get one SM_NOTIFY for each server address if the server
> is a Linux box. Other server OSs will send you just one SM_NOTIFY,
> and the sender address will be more or less random. In the HA case
> described above, the sender address will not match the address
> you used at all (since the UDP packet will have the interface's
> primary IP address, not the alias).
> 
> This is the main motivation for introducing the nsm_handle, and this is
> also the reason why there is potentially a 1-to-many relationship between
> nsm_handles (representing a "host") and nlm_host, representing a tuple of
> (NLM version, transport protocol, address).
> 
> Maybe we should rename nlm_host to something less confusing.

The local statd process is supposed to decode the notification from the
remote client/server, and then notify the kernel. It already sends that
notification on a per-nlm_host basis (i.e. it the call down to the
kernel contains the <address,version,transport protocol>.

If we need to mark more than one <address,version,transport protocol>
tuple as rebooting when we get a notification from the remote
client/server, then why not fix statd so that it does so. Why perform
these extra mappings in the kernel, which doesn't have the benefit of
reverse DNS lookups etc to help it out?

Cheers,
  Trond

Re: [NFS] [PATCH 004 of 19] knfsd: lockd: introduce nsm_handle

[Olaf Kirch]

On Fri, Sep 01, 2006 at 12:41:33PM -0400, Trond Myklebust wrote:
> The local statd process is supposed to decode the notification from the
> remote client/server, and then notify the kernel. It already sends that
> notification on a per-nlm_host basis (i.e. it the call down to the
> kernel contains the <address,version,transport protocol>.

Why does statd need to send the full <address,version,transport protocol>
to the kernel in the first place?

All that's really needed is a unique identification of the host having
rebooted, nevermind how we have been talking to it, and in what role.

I consider the current practice a side effect of a bad implementation
which duplicates a lot of state by dumping everything into the nlm_host.

With the current code, we cannot even monitor IPv6 addresses, because
there's not enough room in the NSM_MON packet. With my proposed change,
we can ditch version and transport protocol, and all of a sudden we
have 16 bytes for the address - ie enough to make IPv6 happy.

In the long run, we could clean out nlm_host even more - there's
a lot of cruft in there.

	h_name		just h_nsmhandle->sm_name
	h_gracewait	could be shared as well
	h_state		should move to nsmhandle as well
	h_nsmstate	currently not used, could move to nsmhandle as well
	h_pidcount	currently allocated per nlm_host, which leads
			to aliasing if we mix NFSv2 and v3 mounts

On a side note, we may want to always allocate an RPC client for each
nlm_host.  Then we can ditch the following variables as well, which are
in the rpc_client's portmap info anyway:

	h_proto		pm_prot
	h_version	pm_vers
	h_sema		useless
	h_nextrebind	we can stop rebinding every 60s, the sunrpc
			doesn't need that anymore. During recovery,
			we can just call rpc_force_rebind
			directly.

Or going even further, one could make the nlm_host agnostic of transports
and protocol versions. Just stick a (short) array of RPC clients in the
nlm_host - any code that places NLM calls will need some extra logic
to select the right client, but it would save on memory and reduce
complexity.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

-- 
VGER BF report: H 2.68873e-09

[PATCH 005 of 19] knfsd: Misc minor fixes, indentation changes

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

This cleans up some code in lockd/host.c, fixes an error printk
and makes it a fatal BUG if nlmsvc_free_host_resources fails.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c    |   15 ++++++---------
 ./fs/lockd/svcsubs.c |    8 +++++---
 2 files changed, 11 insertions(+), 12 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 17:00:03.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 17:01:00.000000000 +1000
@@ -110,16 +110,13 @@ nlm_lookup_host(int server, const struct
 		if (host->h_server != server)
 			continue;
 
-		{
-			if (hp != nlm_hosts + hash) {
-				*hp = host->h_next;
-				host->h_next = nlm_hosts[hash];
-				nlm_hosts[hash] = host;
-			}
-			nlm_get_host(host);
-			mutex_unlock(&nlm_host_mutex);
-			return host;
+		if (hp != nlm_hosts + hash) {
+			*hp = host->h_next;
+			host->h_next = nlm_hosts[hash];
+			nlm_hosts[hash] = host;
 		}
+		nlm_get_host(host);
+		goto out;
 	}
 
 	/* Sadly, the host isn't in our hash table yet. See if

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-08-31 17:01:00.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-08-31 17:01:00.000000000 +1000
@@ -115,7 +115,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 	 * the file.
 	 */
 	if ((nfserr = nlmsvc_ops->fopen(rqstp, f, &file->f_file)) != 0) {
-		dprintk("lockd: open failed (nfserr %d)\n", ntohl(nfserr));
+		dprintk("lockd: open failed (error %d)\n", nfserr);
 		goto out_free;
 	}
 
@@ -313,10 +313,12 @@ nlmsvc_free_host_resources(struct nlm_ho
 {
 	dprintk("lockd: nlmsvc_free_host_resources\n");
 
-	if (nlm_traverse_files(host, NLM_ACT_UNLOCK))
+	if (nlm_traverse_files(host, NLM_ACT_UNLOCK)) {
 		printk(KERN_WARNING
-			"lockd: couldn't remove all locks held by %s",
+			"lockd: couldn't remove all locks held by %s\n",
 			host->h_name);
+		BUG();
+	}
 }
 
 /*

[PATCH 006 of 19] knfsd: lockd: Make nlm_host_rebooted use the nsm_handle

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch makes the SM_NOTIFY handling understand and use
  the nsm_handle.

  To make it a bit clear what is happening:
    nlmclent_prepare_reclaim and nlmclnt_finish_reclaim
    get open-coded into 'reclaimer'
  The result is tidied up.
  Then some of that functionality is moved out into
  nlm_host_rebooted (which calls nlmclnt_recovery which
    starts a thread which runs reclaimer).

  Also host_rebooted now finds an nsm_handle rather than a
  host, then then iterates over all hosts and deals with
  each host that shares that nsm_handle.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>
 
### Diffstat output
 ./fs/lockd/clntlock.c         |   55 ++++++++++++-----------------------
 ./fs/lockd/host.c             |   65 ++++++++++++++++++++++++++++++------------
 ./fs/lockd/svc4proc.c         |    2 -
 ./fs/lockd/svcproc.c          |    2 -
 ./include/linux/lockd/lockd.h |    4 +-
 5 files changed, 70 insertions(+), 58 deletions(-)

diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
--- .prev/fs/lockd/clntlock.c	2006-08-31 17:00:03.000000000 +1000
+++ ./fs/lockd/clntlock.c	2006-08-31 17:02:23.000000000 +1000
@@ -144,43 +144,12 @@ u32 nlmclnt_grant(const struct sockaddr_
  */
 
 /*
- * Someone has sent us an SM_NOTIFY. Ensure we bind to the new port number,
- * that we mark locks for reclaiming, and that we bump the pseudo NSM state.
- */
-static void nlmclnt_prepare_reclaim(struct nlm_host *host)
-{
-	down_write(&host->h_rwsem);
-	if (host->h_nsmhandle)
-		host->h_nsmhandle->sm_monitored = 0;
-	host->h_state++;
-	host->h_nextrebind = 0;
-	nlm_rebind_host(host);
-
-	/*
-	 * Mark the locks for reclaiming.
-	 */
-	list_splice_init(&host->h_granted, &host->h_reclaim);
-
-	dprintk("NLM: reclaiming locks for host %s\n", host->h_name);
-}
-
-static void nlmclnt_finish_reclaim(struct nlm_host *host)
-{
-	host->h_reclaiming = 0;
-	up_write(&host->h_rwsem);
-	dprintk("NLM: done reclaiming locks for host %s", host->h_name);
-}
-
-/*
  * Reclaim all locks on server host. We do this by spawning a separate
  * reclaimer thread.
  */
 void
-nlmclnt_recovery(struct nlm_host *host, u32 newstate)
+nlmclnt_recovery(struct nlm_host *host)
 {
-	if (host->h_nsmstate == newstate)
-		return;
-	host->h_nsmstate = newstate;
 	if (!host->h_reclaiming++) {
 		nlm_get_host(host);
 		__module_get(THIS_MODULE);
@@ -200,18 +169,30 @@ reclaimer(void *ptr)
 	daemonize("%s-reclaim", host->h_name);
 	allow_signal(SIGKILL);
 
+	down_write(&host->h_rwsem);
+
 	/* This one ensures that our parent doesn't terminate while the
 	 * reclaim is in progress */
 	lock_kernel();
 	lockd_up(0); /* note: this cannot fail as lockd is already running */
 
-	nlmclnt_prepare_reclaim(host);
-	/* First, reclaim all locks that have been marked. */
+	dprintk("lockd: reclaiming locks for host %s", host->h_name);
+
 restart:
 	nsmstate = host->h_nsmstate;
+
+	/* Force a portmap getport - the peer's lockd will
+	 * most likely end up on a different port.
+	 */
+	host->h_nextrebind = 0;
+	nlm_rebind_host(host);
+
+	/* First, reclaim all locks that have been granted. */
+	list_splice_init(&host->h_granted, &host->h_reclaim);
 	list_for_each_entry_safe(fl, next, &host->h_reclaim, fl_u.nfs_fl.list) {
 		list_del_init(&fl->fl_u.nfs_fl.list);
 
+		/* Why are we leaking memory here? --okir */
 		if (signalled())
 			continue;
 		if (nlmclnt_reclaim(host, fl) != 0)
@@ -219,11 +200,13 @@ restart:
 		list_add_tail(&fl->fl_u.nfs_fl.list, &host->h_granted);
 		if (host->h_nsmstate != nsmstate) {
 			/* Argh! The server rebooted again! */
-			list_splice_init(&host->h_granted, &host->h_reclaim);
 			goto restart;
 		}
 	}
-	nlmclnt_finish_reclaim(host);
+
+	host->h_reclaiming = 0;
+	up_write(&host->h_rwsem);
+	dprintk("NLM: done reclaiming locks for host %s", host->h_name);
 
 	/* Now, wake up all processes that sleep on a blocked lock */
 	list_for_each_entry(block, &nlm_blocked, b_list) {

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 17:01:00.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 17:02:03.000000000 +1000
@@ -290,28 +290,57 @@ void nlm_release_host(struct nlm_host *h
  * has rebooted.
  * Release all resources held by that peer.
  */
-void nlm_host_rebooted(const struct sockaddr_in *sin, const struct nlm_reboot *argp)
-{
-	struct nlm_host *host;
-	int server;
+void nlm_host_rebooted(const struct sockaddr_in *sin,
+				const char *hostname, int hostname_len,
+				u32 new_state)
+{
+	struct nsm_handle *nsm;
+	struct nlm_host	*host, **hp;
+	int		hash;
 
-	/* Obtain the host pointer for this NFS server and try to
-	 * reclaim all locks we hold on this server.
-	 */
-	server = (argp->proto & 1)? 1 : 0;
-	host = nlm_lookup_host(server, sin, argp->proto >> 1, argp->vers,
-			argp->mon, argp->len);
-	if (host == NULL)
+	dprintk("lockd: nlm_host_rebooted(%s, %u.%u.%u.%u)\n",
+			hostname, NIPQUAD(sin->sin_addr));
+
+	/* Find the NSM handle for this peer */
+	if (!(nsm = __nsm_find(sin, hostname, hostname_len, 0)))
 		return;
 
-	if (server == 0) {
-		/* We are client, he's the server: try to reclaim all locks. */
-		nlmclnt_recovery(host, argp->state);
-	} else {
-		/* He's the client, we're the server: delete all locks held by the client */
-		nlmsvc_free_host_resources(host);
+	/* When reclaiming locks on this peer, make sure that
+	 * we set up a new notification */
+	nsm->sm_monitored = 0;
+
+	/* Mark all hosts tied to this NSM state as having rebooted.
+	 * We run the loop repeatedly, because we drop the host table
+	 * lock for this.
+	 * To avoid processing a host several times, we match the nsmstate.
+	 */
+again:	mutex_lock(&nlm_host_mutex);
+	for (hash = 0; hash < NLM_HOST_NRHASH; hash++) {
+		for (hp = &nlm_hosts[hash]; (host = *hp); hp = &host->h_next) {
+			if (host->h_nsmhandle == nsm
+			 && host->h_nsmstate != new_state) {
+				host->h_nsmstate = new_state;
+				host->h_state++;
+
+				nlm_get_host(host);
+				mutex_unlock(&nlm_host_mutex);
+
+				if (host->h_server) {
+					/* We're server for this guy, just ditch
+					 * all the locks he held. */
+					nlmsvc_free_host_resources(host);
+				} else {
+					/* He's the server, initiate lock recovery. */
+					nlmclnt_recovery(host);
+				}
+
+				nlm_release_host(host);
+				goto again;
+			}
+		}
 	}
-	nlm_release_host(host);
+
+	mutex_unlock(&nlm_host_mutex);
 }
 
 /*

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-08-31 16:59:53.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-08-31 17:02:03.000000000 +1000
@@ -438,7 +438,7 @@ nlm4svc_proc_sm_notify(struct svc_rqst *
 	 */
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_addr.s_addr = argp->addr;
-	nlm_host_rebooted(&saddr, argp);
+	nlm_host_rebooted(&saddr, argp->mon, argp->len, argp->state);
 
 	return rpc_success;
 }

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-08-31 16:59:39.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-08-31 17:02:03.000000000 +1000
@@ -467,7 +467,7 @@ nlmsvc_proc_sm_notify(struct svc_rqst *r
 	 */
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_addr.s_addr = argp->addr;
-	nlm_host_rebooted(&saddr, argp);
+	nlm_host_rebooted(&saddr, argp->mon, argp->len, argp->state);
 
 	return rpc_success;
 }

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-08-31 17:00:03.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-08-31 17:02:03.000000000 +1000
@@ -164,7 +164,7 @@ struct nlm_wait * nlmclnt_prepare_block(
 void		  nlmclnt_finish_block(struct nlm_wait *block);
 int		  nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout);
 u32		  nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *);
-void		  nlmclnt_recovery(struct nlm_host *, u32);
+void		  nlmclnt_recovery(struct nlm_host *);
 int		  nlmclnt_reclaim(struct nlm_host *, struct file_lock *);
 
 /*
@@ -179,7 +179,7 @@ struct nlm_host * nlm_get_host(struct nl
 void		  nlm_release_host(struct nlm_host *);
 void		  nlm_shutdown_hosts(void);
 extern struct nlm_host *nlm_find_client(void);
-extern void	  nlm_host_rebooted(const struct sockaddr_in *, const struct nlm_reboot *);
+extern void	  nlm_host_rebooted(const struct sockaddr_in *, const char *, int, u32);
 struct nsm_handle *nsm_find(const struct sockaddr_in *, const char *, int);
 void		  nsm_release(struct nsm_handle *);
 

[PATCH 007 of 19] knfsd: lockd: make the nsm upcalls use the nsm_handle

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This converts the statd upcalls to use the nsm_handle

  This means that we only register each host once with
  statd, rather than registering each host/vers/protocol triple.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/mon.c                 |   30 ++++++++++++++++++------------
 ./include/linux/lockd/sm_inter.h |    1 -
 2 files changed, 18 insertions(+), 13 deletions(-)

diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
--- .prev/fs/lockd/mon.c	2006-08-31 17:00:03.000000000 +1000
+++ ./fs/lockd/mon.c	2006-08-31 17:21:30.000000000 +1000
@@ -30,7 +30,7 @@ u32				nsm_local_state;
  * Common procedure for SM_MON/SM_UNMON calls
  */
 static int
-nsm_mon_unmon(struct nlm_host *host, u32 proc, struct nsm_res *res)
+nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res)
 {
 	struct rpc_clnt	*clnt;
 	int		status;
@@ -46,10 +46,10 @@ nsm_mon_unmon(struct nlm_host *host, u32
 		goto out;
 	}
 
-	args.addr = host->h_addr.sin_addr.s_addr;
-	args.proto= (host->h_proto<<1) | host->h_server;
+	memset(&args, 0, sizeof(args));
+	args.addr = nsm->sm_addr.sin_addr.s_addr;
 	args.prog = NLM_PROGRAM;
-	args.vers = host->h_version;
+	args.vers = 3;
 	args.proc = NLMPROC_NSM_NOTIFY;
 	memset(res, 0, sizeof(*res));
 
@@ -80,7 +80,7 @@ nsm_monitor(struct nlm_host *host)
 	if (nsm->sm_monitored)
 		return 0;
 
-	status = nsm_mon_unmon(host, SM_MON, &res);
+	status = nsm_mon_unmon(nsm, SM_MON, &res);
 
 	if (status < 0 || res.status != 0)
 		printk(KERN_NOTICE "lockd: cannot monitor %s\n", host->h_name);
@@ -99,16 +99,20 @@ nsm_unmonitor(struct nlm_host *host)
 	struct nsm_res	res;
 	int		status = 0;
 
-	dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
 	if (nsm == NULL)
 		return 0;
 	host->h_nsmhandle = NULL;
 
-	if (!host->h_killed) {
-		status = nsm_mon_unmon(host, SM_UNMON, &res);
+	if (atomic_read(&nsm->sm_count) == 1
+	 && nsm->sm_monitored && !nsm->sm_sticky) {
+		dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name);
+
+		status = nsm_mon_unmon(nsm, SM_UNMON, &res);
 		if (status < 0)
-			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", host->h_name);
-		nsm->sm_monitored = 0;
+			printk(KERN_NOTICE "lockd: cannot unmonitor %s\n",
+					host->h_name);
+		else
+			nsm->sm_monitored = 0;
 	}
 	nsm_release(nsm);
 	return status;
@@ -171,9 +175,11 @@ xdr_encode_mon(struct rpc_rqst *rqstp, u
 	p = xdr_encode_common(rqstp, p, argp);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
+
+	/* Surprise - there may even be room for an IPv6 address now */
 	*p++ = argp->addr;
-	*p++ = argp->vers;
-	*p++ = argp->proto;
+	*p++ = 0;
+	*p++ = 0;
 	*p++ = 0;
 	rqstp->rq_slen = xdr_adjust_iovec(rqstp->rq_svec, p);
 	return 0;

diff .prev/include/linux/lockd/sm_inter.h ./include/linux/lockd/sm_inter.h
--- .prev/include/linux/lockd/sm_inter.h	2006-08-31 17:21:30.000000000 +1000
+++ ./include/linux/lockd/sm_inter.h	2006-08-31 17:28:45.000000000 +1000
@@ -28,7 +28,6 @@ struct nsm_args {
 	u32		prog;		/* RPC callback info */
 	u32		vers;
 	u32		proc;
-	u32		proto;		/* protocol (udp/tcp) plus server/client flag */
 };
 
 /*

[PATCH 008 of 19] knfsd: lockd: make the hash chains use a hlist_node

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  Get rid of the home-grown singly linked lists for the
  nlm_host hash table.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c             |   71 +++++++++++++++++++++++-------------------
 ./include/linux/lockd/lockd.h |    2 -
 2 files changed, 40 insertions(+), 33 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 10:42:33.000000000 +1000
+++ ./fs/lockd/host.c	2006-08-31 17:32:46.000000000 +1000
@@ -27,7 +27,7 @@
 #define NLM_HOST_EXPIRE		((nrhosts > NLM_HOST_MAX)? 300 * HZ : 120 * HZ)
 #define NLM_HOST_COLLECT	((nrhosts > NLM_HOST_MAX)? 120 * HZ :  60 * HZ)
 
-static struct nlm_host *	nlm_hosts[NLM_HOST_NRHASH];
+static struct hlist_head	nlm_hosts[NLM_HOST_NRHASH];
 static unsigned long		next_gc;
 static int			nrhosts;
 static DEFINE_MUTEX(nlm_host_mutex);
@@ -69,7 +69,9 @@ nlm_lookup_host(int server, const struct
 					const char *hostname,
 					int hostname_len)
 {
-	struct nlm_host	*host, **hp;
+	struct hlist_head *chain;
+	struct hlist_node *pos;
+	struct nlm_host	*host;
 	struct nsm_handle *nsm = NULL;
 	int		hash;
 
@@ -95,7 +97,8 @@ nlm_lookup_host(int server, const struct
 	 * different NLM rpc_clients into one single nlm_host object.
 	 * This would allow us to have one nlm_host per address.
 	 */
-	for (hp = &nlm_hosts[hash]; (host = *hp) != 0; hp = &host->h_next) {
+	chain = &nlm_hosts[hash];
+	hlist_for_each_entry(host, pos, chain, h_hash) {
 		if (!nlm_cmp_addr(&host->h_addr, sin))
 			continue;
 
@@ -110,15 +113,16 @@ nlm_lookup_host(int server, const struct
 		if (host->h_server != server)
 			continue;
 
-		if (hp != nlm_hosts + hash) {
-			*hp = host->h_next;
-			host->h_next = nlm_hosts[hash];
-			nlm_hosts[hash] = host;
-		}
+		/* Move to head of hash chain. */
+		hlist_del(&host->h_hash);
+		hlist_add_head(&host->h_hash, chain);
+
 		nlm_get_host(host);
 		goto out;
 	}
 
+	host = NULL;
+
 	/* Sadly, the host isn't in our hash table yet. See if
 	 * we have an NSM handle for it. If not, create one.
 	 */
@@ -146,8 +150,7 @@ nlm_lookup_host(int server, const struct
 	host->h_nsmstate   = 0;			/* real NSM state */
 	host->h_nsmhandle  = nsm;
 	host->h_server	   = server;
-	host->h_next       = nlm_hosts[hash];
-	nlm_hosts[hash]    = host;
+	hlist_add_head(&host->h_hash, chain);
 	INIT_LIST_HEAD(&host->h_lockowners);
 	spin_lock_init(&host->h_lock);
 	INIT_LIST_HEAD(&host->h_granted);
@@ -164,14 +167,17 @@ out:
 struct nlm_host *
 nlm_find_client(void)
 {
+	struct hlist_head *chain;
+	struct hlist_node *pos;
+
 	/* find a nlm_host for a client for which h_killed == 0.
 	 * and return it
 	 */
-	int hash;
 	mutex_lock(&nlm_host_mutex);
-	for (hash = 0 ; hash < NLM_HOST_NRHASH; hash++) {
-		struct nlm_host *host, **hp;
-		for (hp = &nlm_hosts[hash]; (host = *hp) != 0; hp = &host->h_next) {
+	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+		struct nlm_host *host;
+
+		hlist_for_each_entry(host, pos, chain, h_hash) {
 			if (host->h_server &&
 			    host->h_killed == 0) {
 				nlm_get_host(host);
@@ -294,9 +300,10 @@ void nlm_host_rebooted(const struct sock
 				const char *hostname, int hostname_len,
 				u32 new_state)
 {
+	struct hlist_head *chain;
+	struct hlist_node *pos;
 	struct nsm_handle *nsm;
-	struct nlm_host	*host, **hp;
-	int		hash;
+	struct nlm_host	*host;
 
 	dprintk("lockd: nlm_host_rebooted(%s, %u.%u.%u.%u)\n",
 			hostname, NIPQUAD(sin->sin_addr));
@@ -315,8 +322,8 @@ void nlm_host_rebooted(const struct sock
 	 * To avoid processing a host several times, we match the nsmstate.
 	 */
 again:	mutex_lock(&nlm_host_mutex);
-	for (hash = 0; hash < NLM_HOST_NRHASH; hash++) {
-		for (hp = &nlm_hosts[hash]; (host = *hp); hp = &host->h_next) {
+	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+		hlist_for_each_entry(host, pos, chain, h_hash) {
 			if (host->h_nsmhandle == nsm
 			 && host->h_nsmstate != new_state) {
 				host->h_nsmstate = new_state;
@@ -350,16 +357,17 @@ again:	mutex_lock(&nlm_host_mutex);
 void
 nlm_shutdown_hosts(void)
 {
+	struct hlist_head *chain;
+	struct hlist_node *pos;
 	struct nlm_host	*host;
-	int		i;
 
 	dprintk("lockd: shutting down host module\n");
 	mutex_lock(&nlm_host_mutex);
 
 	/* First, make all hosts eligible for gc */
 	dprintk("lockd: nuking all hosts...\n");
-	for (i = 0; i < NLM_HOST_NRHASH; i++) {
-		for (host = nlm_hosts[i]; host; host = host->h_next)
+	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+		hlist_for_each_entry(host, pos, chain, h_hash)
 			host->h_expires = jiffies - 1;
 	}
 
@@ -371,8 +379,8 @@ nlm_shutdown_hosts(void)
 	if (nrhosts) {
 		printk(KERN_WARNING "lockd: couldn't shutdown host module!\n");
 		dprintk("lockd: %d hosts left:\n", nrhosts);
-		for (i = 0; i < NLM_HOST_NRHASH; i++) {
-			for (host = nlm_hosts[i]; host; host = host->h_next) {
+		for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+			hlist_for_each_entry(host, pos, chain, h_hash) {
 				dprintk("       %s (cnt %d use %d exp %ld)\n",
 					host->h_name, atomic_read(&host->h_count),
 					host->h_inuse, host->h_expires);
@@ -389,32 +397,31 @@ nlm_shutdown_hosts(void)
 static void
 nlm_gc_hosts(void)
 {
-	struct nlm_host	**q, *host;
+	struct hlist_head *chain;
+	struct hlist_node *pos, *next;
+	struct nlm_host	*host;
 	struct rpc_clnt	*clnt;
-	int		i;
 
 	dprintk("lockd: host garbage collection\n");
-	for (i = 0; i < NLM_HOST_NRHASH; i++) {
-		for (host = nlm_hosts[i]; host; host = host->h_next)
+	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+		hlist_for_each_entry(host, pos, chain, h_hash)
 			host->h_inuse = 0;
 	}
 
 	/* Mark all hosts that hold locks, blocks or shares */
 	nlmsvc_mark_resources();
 
-	for (i = 0; i < NLM_HOST_NRHASH; i++) {
-		q = &nlm_hosts[i];
-		while ((host = *q) != NULL) {
+	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
+		hlist_for_each_entry_safe(host, pos, next, chain, h_hash) {
 			if (atomic_read(&host->h_count) || host->h_inuse
 			 || time_before(jiffies, host->h_expires)) {
 				dprintk("nlm_gc_hosts skipping %s (cnt %d use %d exp %ld)\n",
 					host->h_name, atomic_read(&host->h_count),
 					host->h_inuse, host->h_expires);
-				q = &host->h_next;
 				continue;
 			}
 			dprintk("lockd: delete host %s\n", host->h_name);
-			*q = host->h_next;
+			hlist_del_init(&host->h_hash);
 
 			/*
 			 * Unmonitor unless host was invalidated (i.e. lockd restarted)

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 10:42:33.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 10:42:31.000000000 +1000
@@ -37,7 +37,7 @@
  * Lockd host handle (used both by the client and server personality).
  */
 struct nlm_host {
-	struct nlm_host *	h_next;		/* linked list (hash table) */
+	struct hlist_node	h_hash;		/* doubly linked list */
 	struct sockaddr_in	h_addr;		/* peer address */
 	struct rpc_clnt	*	h_rpcclnt;	/* RPC client to talk to peer */
 	char *			h_name;		/* remote hostname */

[PATCH 009 of 19] knfsd: lockd: Change list of blocked list to list_node

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

 This patch changes the nlm_blocked list to use a list_node
 instead of homegrown linked list handling.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/svclock.c          |  119 +++++++++++++++++++-----------------------
 ./fs/lockd/svcsubs.c          |    5 +
 ./include/linux/lockd/lockd.h |    7 +-
 3 files changed, 62 insertions(+), 69 deletions(-)

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-09-01 10:42:31.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-09-01 10:44:39.000000000 +1000
@@ -40,7 +40,7 @@
 
 static void nlmsvc_release_block(struct nlm_block *block);
 static void	nlmsvc_insert_block(struct nlm_block *block, unsigned long);
-static int	nlmsvc_remove_block(struct nlm_block *block);
+static void	nlmsvc_remove_block(struct nlm_block *block);
 
 static int nlmsvc_setgrantargs(struct nlm_rqst *call, struct nlm_lock *lock);
 static void nlmsvc_freegrantargs(struct nlm_rqst *call);
@@ -49,7 +49,7 @@ static const struct rpc_call_ops nlmsvc_
 /*
  * The list of blocked locks to retry
  */
-static struct nlm_block *	nlm_blocked;
+static LIST_HEAD(nlm_blocked);
 
 /*
  * Insert a blocked lock into the global list
@@ -57,48 +57,44 @@ static struct nlm_block *	nlm_blocked;
 static void
 nlmsvc_insert_block(struct nlm_block *block, unsigned long when)
 {
-	struct nlm_block **bp, *b;
+	struct nlm_block *b;
+	struct list_head *pos;
 
 	dprintk("lockd: nlmsvc_insert_block(%p, %ld)\n", block, when);
-	kref_get(&block->b_count);
-	if (block->b_queued)
-		nlmsvc_remove_block(block);
-	bp = &nlm_blocked;
+	if (list_empty(&block->b_list)) {
+		kref_get(&block->b_count);
+	} else {
+		list_del_init(&block->b_list);
+	}
+
+	pos = &nlm_blocked;
 	if (when != NLM_NEVER) {
 		if ((when += jiffies) == NLM_NEVER)
 			when ++;
-		while ((b = *bp) && time_before_eq(b->b_when,when) && b->b_when != NLM_NEVER)
-			bp = &b->b_next;
-	} else
-		while ((b = *bp) != 0)
-			bp = &b->b_next;
+		list_for_each(pos, &nlm_blocked) {
+			b = list_entry(pos, struct nlm_block, b_list);
+			if (time_after(b->b_when,when) || b->b_when == NLM_NEVER)
+				break;
+		}
+		/* On normal exit from the loop, pos == &nlm_blocked,
+		 * so we will be adding to the end of the list - good
+		 */
+	}
 
-	block->b_queued = 1;
+	list_add_tail(&block->b_list, pos);
 	block->b_when = when;
-	block->b_next = b;
-	*bp = block;
 }
 
 /*
  * Remove a block from the global list
  */
-static int
+static inline void
 nlmsvc_remove_block(struct nlm_block *block)
 {
-	struct nlm_block **bp, *b;
-
-	if (!block->b_queued)
-		return 1;
-	for (bp = &nlm_blocked; (b = *bp) != 0; bp = &b->b_next) {
-		if (b == block) {
-			*bp = block->b_next;
-			block->b_queued = 0;
-			nlmsvc_release_block(block);
-			return 1;
-		}
+	if (!list_empty(&block->b_list)) {
+		list_del_init(&block->b_list);
+		nlmsvc_release_block(block);
 	}
-
-	return 0;
 }
 
 /*
@@ -107,14 +103,14 @@ nlmsvc_remove_block(struct nlm_block *bl
 static struct nlm_block *
 nlmsvc_lookup_block(struct nlm_file *file, struct nlm_lock *lock)
 {
-	struct nlm_block	**head, *block;
+	struct nlm_block	*block;
 	struct file_lock	*fl;
 
 	dprintk("lockd: nlmsvc_lookup_block f=%p pd=%d %Ld-%Ld ty=%d\n",
 				file, lock->fl.fl_pid,
 				(long long)lock->fl.fl_start,
 				(long long)lock->fl.fl_end, lock->fl.fl_type);
-	for (head = &nlm_blocked; (block = *head) != 0; head = &block->b_next) {
+	list_for_each_entry(block, &nlm_blocked, b_list) {
 		fl = &block->b_call->a_args.lock.fl;
 		dprintk("lockd: check f=%p pd=%d %Ld-%Ld ty=%d cookie=%s\n",
 				block->b_file, fl->fl_pid,
@@ -147,16 +143,16 @@ nlmsvc_find_block(struct nlm_cookie *coo
 {
 	struct nlm_block *block;
 
-	for (block = nlm_blocked; block; block = block->b_next) {
-		dprintk("cookie: head of blocked queue %p, block %p\n", 
-			nlm_blocked, block);
+	list_for_each_entry(block, &nlm_blocked, b_list) {
 		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie)
 				&& nlm_cmp_addr(sin, &block->b_host->h_addr))
-			break;
+			goto found;
 	}
 
-	if (block != NULL)
-		kref_get(&block->b_count);
+	return NULL;
+
+found:
+	kref_get(&block->b_count);
 	return block;
 }
 
@@ -192,6 +188,8 @@ nlmsvc_create_block(struct svc_rqst *rqs
 	if (block == NULL)
 		goto failed;
 	kref_init(&block->b_count);
+	INIT_LIST_HEAD(&block->b_list);
+	INIT_LIST_HEAD(&block->b_flist);
 
 	if (!nlmsvc_setgrantargs(call, lock))
 		goto failed_free;
@@ -210,8 +208,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
 	file->f_count++;
 
 	/* Add to file's list of blocks */
-	block->b_fnext  = file->f_blocks;
-	file->f_blocks  = block;
+	list_add(&block->b_flist, &file->f_blocks);
 
 	/* Set up RPC arguments for callback */
 	block->b_call = call;
@@ -248,18 +245,12 @@ static void nlmsvc_free_block(struct kre
 {
 	struct nlm_block *block = container_of(kref, struct nlm_block, b_count);
 	struct nlm_file		*file = block->b_file;
-	struct nlm_block	**bp;
 
 	dprintk("lockd: freeing block %p...\n", block);
 
-	down(&file->f_sema);
 	/* Remove block from file's list of blocks */
-	for (bp = &file->f_blocks; *bp; bp = &(*bp)->b_fnext) {
-		if (*bp == block) {
-			*bp = block->b_fnext;
-			break;
-		}
-	}
+	down(&file->f_sema);
+	list_del_init(&block->b_flist);
 	up(&file->f_sema);
 
 	nlmsvc_freegrantargs(block->b_call);
@@ -279,21 +270,23 @@ static void nlmsvc_act_mark(struct nlm_h
 	struct nlm_block *block;
 
 	down(&file->f_sema);
-	for (block = file->f_blocks; block != NULL; block = block->b_fnext)
+	list_for_each_entry(block, &file->f_blocks, b_flist)
 		block->b_host->h_inuse = 1;
 	up(&file->f_sema);
 }
 
 static void nlmsvc_act_unlock(struct nlm_host *host, struct nlm_file *file)
 {
-	struct nlm_block *block;
+	struct nlm_block *block, *next;
 
 restart:
 	down(&file->f_sema);
-	for (block = file->f_blocks; block != NULL; block = block->b_fnext) {
+	list_for_each_entry_safe(block, next, &file->f_blocks, b_flist) {
 		if (host != NULL && host != block->b_host)
 			continue;
-		if (!block->b_queued)
+		/* Do not destroy blocks that are not on
+		 * the global retry list - why? */
+		if (list_empty(&block->b_list))
 			continue;
 		kref_get(&block->b_count);
 		up(&file->f_sema);
@@ -528,10 +521,10 @@ nlmsvc_cancel_blocked(struct nlm_file *f
 static void
 nlmsvc_notify_blocked(struct file_lock *fl)
 {
-	struct nlm_block	**bp, *block;
+	struct nlm_block	*block;
 
 	dprintk("lockd: VFS unblock notification for block %p\n", fl);
-	for (bp = &nlm_blocked; (block = *bp) != 0; bp = &block->b_next) {
+	list_for_each_entry(block, &nlm_blocked, b_list) {
 		if (nlm_compare_locks(&block->b_call->a_args.lock.fl, fl)) {
 			nlmsvc_insert_block(block, 0);
 			svc_wake_up(block->b_daemon);
@@ -697,16 +690,19 @@ nlmsvc_grant_reply(struct svc_rqst *rqst
 unsigned long
 nlmsvc_retry_blocked(void)
 {
-	struct nlm_block	*block;
+	unsigned long	timeout = MAX_SCHEDULE_TIMEOUT;
+	struct nlm_block *block;
+
+	while (!list_empty(&nlm_blocked)) {
+		block = list_entry(nlm_blocked.next, struct nlm_block, b_list);
 
-	dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
-			nlm_blocked,
-			nlm_blocked? nlm_blocked->b_when : 0);
-	while ((block = nlm_blocked) != 0) {
 		if (block->b_when == NLM_NEVER)
 			break;
-	        if (time_after(block->b_when,jiffies))
+	        if (time_after(block->b_when,jiffies)) {
+			timeout = block->b_when - jiffies;
 			break;
+		}
+
 		dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
 			block, block->b_when);
 		kref_get(&block->b_count);
@@ -714,8 +710,5 @@ nlmsvc_retry_blocked(void)
 		nlmsvc_release_block(block);
 	}
 
-	if ((block = nlm_blocked) && block->b_when != NLM_NEVER)
-		return (block->b_when - jiffies);
-
-	return MAX_SCHEDULE_TIMEOUT;
+	return timeout;
 }

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-09-01 10:42:31.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-09-01 10:42:51.000000000 +1000
@@ -107,6 +107,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 	memcpy(&file->f_handle, f, sizeof(struct nfs_fh));
 	file->f_hash = hash;
 	init_MUTEX(&file->f_sema);
+	INIT_LIST_HEAD(&file->f_blocks);
 
 	/* Open the file. Note that this must not sleep for too long, else
 	 * we would lock up lockd:-) So no NFS re-exports, folks.
@@ -220,7 +221,7 @@ nlm_inspect_file(struct nlm_host *host, 
 {
 	if (action == NLM_ACT_CHECK) {
 		/* Fast path for mark and sweep garbage collection */
-		if (file->f_count || file->f_blocks || file->f_shares)
+		if (file->f_count || list_empty(&file->f_blocks) || file->f_shares)
 			return 1;
 	} else {
 		nlmsvc_traverse_blocks(host, file, action);
@@ -253,7 +254,7 @@ nlm_traverse_files(struct nlm_host *host
 			mutex_lock(&nlm_file_mutex);
 			file->f_count--;
 			/* No more references to this file. Let go of it. */
-			if (!file->f_blocks && !file->f_locks
+			if (list_empty(&file->f_blocks) && !file->f_locks
 			 && !file->f_shares && !file->f_count) {
 				*fp = file->f_next;
 				nlmsvc_ops->fclose(file->f_file);

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 10:42:31.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 10:42:51.000000000 +1000
@@ -109,7 +109,7 @@ struct nlm_file {
 	struct nfs_fh		f_handle;	/* NFS file handle */
 	struct file *		f_file;		/* VFS file pointer */
 	struct nlm_share *	f_shares;	/* DOS shares */
-	struct nlm_block *	f_blocks;	/* blocked locks */
+	struct list_head	f_blocks;	/* blocked locks */
 	unsigned int		f_locks;	/* guesstimate # of locks */
 	unsigned int		f_count;	/* reference count */
 	struct semaphore	f_sema;		/* avoid concurrent access */
@@ -123,14 +123,13 @@ struct nlm_file {
 #define NLM_NEVER		(~(unsigned long) 0)
 struct nlm_block {
 	struct kref		b_count;	/* Reference count */
-	struct nlm_block *	b_next;		/* linked list (all blocks) */
-	struct nlm_block *	b_fnext;	/* linked list (per file) */
+	struct list_head	b_list;		/* linked list of all blocks */
+	struct list_head	b_flist;	/* linked list (per file) */
 	struct nlm_rqst	*	b_call;		/* RPC args & callback info */
 	struct svc_serv *	b_daemon;	/* NLM service */
 	struct nlm_host *	b_host;		/* host handle for RPC clnt */
 	unsigned long		b_when;		/* next re-xmit */
 	unsigned int		b_id;		/* block id */
-	unsigned char		b_queued;	/* re-queued */
 	unsigned char		b_granted;	/* VFS granted lock */
 	struct nlm_file *	b_file;		/* file in question */
 };

[PATCH 010 of 19] knfsd: Change nlm_file to use a hlist

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This changes struct nlm_file and the nlm_files hash table
  to use a hlist instead of the home-grown lists.

  This allows us to remove f_hash which was only used to find the right
  hash chain to delete an entry from.

  It also increases the size of the nlm_files hash table from
  32 to 128.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/svcsubs.c          |   42 ++++++++++++++++--------------------------
 ./include/linux/lockd/lockd.h |    3 +--
 2 files changed, 17 insertions(+), 28 deletions(-)

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-09-01 10:42:51.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-09-01 10:45:16.000000000 +1000
@@ -25,9 +25,9 @@
 /*
  * Global file hash table
  */
-#define FILE_HASH_BITS		5
+#define FILE_HASH_BITS		7
 #define FILE_NRHASH		(1<<FILE_HASH_BITS)
-static struct nlm_file *	nlm_files[FILE_NRHASH];
+static struct hlist_head	nlm_files[FILE_NRHASH];
 static DEFINE_MUTEX(nlm_file_mutex);
 
 #ifdef NFSD_DEBUG
@@ -82,6 +82,7 @@ u32
 nlm_lookup_file(struct svc_rqst *rqstp, struct nlm_file **result,
 					struct nfs_fh *f)
 {
+	struct hlist_node *pos;
 	struct nlm_file	*file;
 	unsigned int	hash;
 	u32		nfserr;
@@ -93,7 +94,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 	/* Lock file table */
 	mutex_lock(&nlm_file_mutex);
 
-	for (file = nlm_files[hash]; file; file = file->f_next)
+	hlist_for_each_entry(file, pos, &nlm_files[hash], f_list)
 		if (!nfs_compare_fh(&file->f_handle, f))
 			goto found;
 
@@ -105,8 +106,8 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 		goto out_unlock;
 
 	memcpy(&file->f_handle, f, sizeof(struct nfs_fh));
-	file->f_hash = hash;
 	init_MUTEX(&file->f_sema);
+	INIT_HLIST_NODE(&file->f_list);
 	INIT_LIST_HEAD(&file->f_blocks);
 
 	/* Open the file. Note that this must not sleep for too long, else
@@ -120,8 +121,7 @@ nlm_lookup_file(struct svc_rqst *rqstp, 
 		goto out_free;
 	}
 
-	file->f_next = nlm_files[hash];
-	nlm_files[hash] = file;
+	hlist_add_head(&file->f_list, &nlm_files[hash]);
 
 found:
 	dprintk("lockd: found file %p (count %d)\n", file, file->f_count);
@@ -150,22 +150,14 @@ out_free:
 static inline void
 nlm_delete_file(struct nlm_file *file)
 {
-	struct nlm_file	**fp, *f;
-
 	nlm_debug_print_file("closing file", file);
-
-	fp = nlm_files + file->f_hash;
-	while ((f = *fp) != NULL) {
-		if (f == file) {
-			*fp = file->f_next;
-			nlmsvc_ops->fclose(file->f_file);
-			kfree(file);
-			return;
-		}
-		fp = &f->f_next;
+	if (!hlist_unhashed(&file->f_list)) {
+		hlist_del(&file->f_list);
+		nlmsvc_ops->fclose(file->f_file);
+		kfree(file);
+	} else {
+		printk(KERN_WARNING "lockd: attempt to release unknown file!\n");
 	}
-
-	printk(KERN_WARNING "lockd: attempt to release unknown file!\n");
 }
 
 /*
@@ -236,13 +228,13 @@ nlm_inspect_file(struct nlm_host *host, 
 static int
 nlm_traverse_files(struct nlm_host *host, int action)
 {
-	struct nlm_file	*file, **fp;
+	struct hlist_node *pos, *next;
+	struct nlm_file	*file;
 	int i, ret = 0;
 
 	mutex_lock(&nlm_file_mutex);
 	for (i = 0; i < FILE_NRHASH; i++) {
-		fp = nlm_files + i;
-		while ((file = *fp) != NULL) {
+		hlist_for_each_entry_safe(file, pos, next, &nlm_files[i], f_list) {
 			file->f_count++;
 			mutex_unlock(&nlm_file_mutex);
 
@@ -256,11 +248,9 @@ nlm_traverse_files(struct nlm_host *host
 			/* No more references to this file. Let go of it. */
 			if (list_empty(&file->f_blocks) && !file->f_locks
 			 && !file->f_shares && !file->f_count) {
-				*fp = file->f_next;
+				hlist_del(&file->f_list);
 				nlmsvc_ops->fclose(file->f_file);
 				kfree(file);
-			} else {
-				fp = &file->f_next;
 			}
 		}
 	}

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 10:42:51.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 10:45:16.000000000 +1000
@@ -105,7 +105,7 @@ struct nlm_rqst {
  * an NFS client.
  */
 struct nlm_file {
-	struct nlm_file *	f_next;		/* linked list */
+	struct hlist_node	f_list;		/* linked list */
 	struct nfs_fh		f_handle;	/* NFS file handle */
 	struct file *		f_file;		/* VFS file pointer */
 	struct nlm_share *	f_shares;	/* DOS shares */
@@ -113,7 +113,6 @@ struct nlm_file {
 	unsigned int		f_locks;	/* guesstimate # of locks */
 	unsigned int		f_count;	/* reference count */
 	struct semaphore	f_sema;		/* avoid concurrent access */
-	int		       	f_hash;		/* hash of f_handle */
 };
 
 /*

[PATCH 011 of 19] knfsd: lockd: make nlm_traverse_* more flexible

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch makes nlm_traverse{locks,blocks,shares} and friends
  use a function pointer rather than a "action" enum.

  This function pointer is given two nlm_hosts (one given by the
  caller, the other taken from the lock/block/share currently
  visited), and is free to do with them as it wants. If it returns a
  non-zero value, the lockd/block/share is released.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/svclock.c          |   33 +++----------
 ./fs/lockd/svcshare.c         |   20 +++-----
 ./fs/lockd/svcsubs.c          |  105 ++++++++++++++++++++++++++++++------------
 ./include/linux/lockd/lockd.h |   15 ++----
 ./include/linux/lockd/share.h |    3 -
 5 files changed, 102 insertions(+), 74 deletions(-)

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-09-01 10:44:39.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-09-01 10:59:37.000000000 +1000
@@ -265,24 +265,20 @@ static void nlmsvc_release_block(struct 
 		kref_put(&block->b_count, nlmsvc_free_block);
 }
 
-static void nlmsvc_act_mark(struct nlm_host *host, struct nlm_file *file)
-{
-	struct nlm_block *block;
-
-	down(&file->f_sema);
-	list_for_each_entry(block, &file->f_blocks, b_flist)
-		block->b_host->h_inuse = 1;
-	up(&file->f_sema);
-}
-
-static void nlmsvc_act_unlock(struct nlm_host *host, struct nlm_file *file)
+/*
+ * Loop over all blocks and delete blocks held by
+ * a matching host.
+ */
+void nlmsvc_traverse_blocks(struct nlm_host *host,
+			struct nlm_file *file,
+			nlm_host_match_fn_t match)
 {
 	struct nlm_block *block, *next;
 
 restart:
 	down(&file->f_sema);
 	list_for_each_entry_safe(block, next, &file->f_blocks, b_flist) {
-		if (host != NULL && host != block->b_host)
+		if (!match(block->b_host, host))
 			continue;
 		/* Do not destroy blocks that are not on
 		 * the global retry list - why? */
@@ -298,19 +294,6 @@ restart:
 }
 
 /*
- * Loop over all blocks and perform the action specified.
- * (NLM_ACT_CHECK handled by nlmsvc_inspect_file).
- */
-void
-nlmsvc_traverse_blocks(struct nlm_host *host, struct nlm_file *file, int action)
-{
-	if (action == NLM_ACT_MARK)
-		nlmsvc_act_mark(host, file);
-	else
-		nlmsvc_act_unlock(host, file);
-}
-
-/*
  * Initialize arguments for GRANTED call. The nlm_rqst structure
  * has been cleared already.
  */

diff .prev/fs/lockd/svcshare.c ./fs/lockd/svcshare.c
--- .prev/fs/lockd/svcshare.c	2006-09-01 10:39:06.000000000 +1000
+++ ./fs/lockd/svcshare.c	2006-09-01 10:58:38.000000000 +1000
@@ -85,24 +85,20 @@ nlmsvc_unshare_file(struct nlm_host *hos
 }
 
 /*
- * Traverse all shares for a given file (and host).
- * NLM_ACT_CHECK is handled by nlmsvc_inspect_file.
+ * Traverse all shares for a given file, and delete
+ * those owned by the given (type of) host
  */
-void
-nlmsvc_traverse_shares(struct nlm_host *host, struct nlm_file *file, int action)
+void nlmsvc_traverse_shares(struct nlm_host *host, struct nlm_file *file,
+		nlm_host_match_fn_t match)
 {
 	struct nlm_share	*share, **shpp;
 
 	shpp = &file->f_shares;
 	while ((share = *shpp) !=  NULL) {
-		if (action == NLM_ACT_MARK)
-			share->s_host->h_inuse = 1;
-		else if (action == NLM_ACT_UNLOCK) {
-			if (host == NULL || host == share->s_host) {
-				*shpp = share->s_next;
-				kfree(share);
-				continue;
-			}
+		if (match(share->s_host, host)) {
+			*shpp = share->s_next;
+			kfree(share);
+			continue;
 		}
 		shpp = &share->s_next;
 	}

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-09-01 10:45:16.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-09-01 11:06:21.000000000 +1000
@@ -165,7 +165,8 @@ nlm_delete_file(struct nlm_file *file)
  * action.
  */
 static int
-nlm_traverse_locks(struct nlm_host *host, struct nlm_file *file, int action)
+nlm_traverse_locks(struct nlm_host *host, struct nlm_file *file,
+			nlm_host_match_fn_t match)
 {
 	struct inode	 *inode = nlmsvc_file_inode(file);
 	struct file_lock *fl;
@@ -179,17 +180,11 @@ again:
 
 		/* update current lock count */
 		file->f_locks++;
+
 		lockhost = (struct nlm_host *) fl->fl_owner;
-		if (action == NLM_ACT_MARK)
-			lockhost->h_inuse = 1;
-		else if (action == NLM_ACT_CHECK)
-			return 1;
-		else if (action == NLM_ACT_UNLOCK) {
+		if (match(lockhost, host)) {
 			struct file_lock lock = *fl;
 
-			if (host && lockhost != host)
-				continue;
-
 			lock.fl_type  = F_UNLCK;
 			lock.fl_start = 0;
 			lock.fl_end   = OFFSET_MAX;
@@ -206,27 +201,42 @@ again:
 }
 
 /*
- * Operate on a single file
+ * Inspect a single file
+ */
+static inline int
+nlm_inspect_file(struct nlm_host *host, struct nlm_file *file, nlm_host_match_fn_t match)
+{
+	nlmsvc_traverse_blocks(host, file, match);
+	nlmsvc_traverse_shares(host, file, match);
+	return nlm_traverse_locks(host, file, match);
+}
+
+/*
+ * Quick check whether there are still any locks, blocks or
+ * shares on a given file.
  */
 static inline int
-nlm_inspect_file(struct nlm_host *host, struct nlm_file *file, int action)
+nlm_file_inuse(struct nlm_file *file)
 {
-	if (action == NLM_ACT_CHECK) {
-		/* Fast path for mark and sweep garbage collection */
-		if (file->f_count || list_empty(&file->f_blocks) || file->f_shares)
+	struct inode	 *inode = nlmsvc_file_inode(file);
+	struct file_lock *fl;
+
+	if (file->f_count || !list_empty(&file->f_blocks) || file->f_shares)
+		return 1;
+
+	for (fl = inode->i_flock; fl; fl = fl->fl_next) {
+		if (fl->fl_lmops == &nlmsvc_lock_operations)
 			return 1;
-	} else {
-		nlmsvc_traverse_blocks(host, file, action);
-		nlmsvc_traverse_shares(host, file, action);
 	}
-	return nlm_traverse_locks(host, file, action);
+	file->f_locks = 0;
+	return 0;
 }
 
 /*
  * Loop over all files in the file table.
  */
 static int
-nlm_traverse_files(struct nlm_host *host, int action)
+nlm_traverse_files(struct nlm_host *host, nlm_host_match_fn_t match)
 {
 	struct hlist_node *pos, *next;
 	struct nlm_file	*file;
@@ -240,7 +250,7 @@ nlm_traverse_files(struct nlm_host *host
 
 			/* Traverse locks, blocks and shares of this file
 			 * and update file->f_locks count */
-			if (nlm_inspect_file(host, file, action))
+			if (nlm_inspect_file(host, file, match))
 				ret = 1;
 
 			mutex_lock(&nlm_file_mutex);
@@ -277,23 +287,54 @@ nlm_release_file(struct nlm_file *file)
 	mutex_lock(&nlm_file_mutex);
 
 	/* If there are no more locks etc, delete the file */
-	if(--file->f_count == 0) {
-		if(!nlm_inspect_file(NULL, file, NLM_ACT_CHECK))
-			nlm_delete_file(file);
-	}
+	if (--file->f_count == 0 && !nlm_file_inuse(file))
+		nlm_delete_file(file);
 
 	mutex_unlock(&nlm_file_mutex);
 }
 
 /*
+ * Helpers function for resource traversal
+ *
+ * nlmsvc_mark_host:
+ *	used by the garbage collector; simply sets h_inuse.
+ *	Always returns 0.
+ *
+ * nlmsvc_same_host:
+ *	returns 1 iff the two hosts match. Used to release
+ *	all resources bound to a specific host.
+ *
+ * nlmsvc_is_client:
+ *	returns 1 iff the host is a client.
+ *	Used by nlmsvc_invalidate_all
+ */
+static int
+nlmsvc_mark_host(struct nlm_host *host, struct nlm_host *dummy)
+{
+	host->h_inuse = 1;
+	return 0;
+}
+
+static int
+nlmsvc_same_host(struct nlm_host *host, struct nlm_host *other)
+{
+	return host == other;
+}
+
+static int
+nlmsvc_is_client(struct nlm_host *host, struct nlm_host *dummy)
+{
+	return host->h_server;
+}
+
+/*
  * Mark all hosts that still hold resources
  */
 void
 nlmsvc_mark_resources(void)
 {
 	dprintk("lockd: nlmsvc_mark_resources\n");
-
-	nlm_traverse_files(NULL, NLM_ACT_MARK);
+	nlm_traverse_files(NULL, nlmsvc_mark_host);
 }
 
 /*
@@ -304,7 +345,7 @@ nlmsvc_free_host_resources(struct nlm_ho
 {
 	dprintk("lockd: nlmsvc_free_host_resources\n");
 
-	if (nlm_traverse_files(host, NLM_ACT_UNLOCK)) {
+	if (nlm_traverse_files(host, nlmsvc_same_host)) {
 		printk(KERN_WARNING
 			"lockd: couldn't remove all locks held by %s\n",
 			host->h_name);
@@ -319,8 +360,16 @@ void
 nlmsvc_invalidate_all(void)
 {
 	struct nlm_host *host;
+
+	/* Release all locks held by NFS clients.
+	 * Previously, the code would call
+	 * nlmsvc_free_host_resources for each client in
+	 * turn, which is about as inefficient as it gets.
+	 * Now we just do it once in nlm_traverse_files.
+	 */
+	nlm_traverse_files(NULL, nlmsvc_is_client);
+
 	while ((host = nlm_find_client()) != NULL) {
-		nlmsvc_free_host_resources(host);
 		host->h_expires = 0;
 		host->h_killed = 1;
 		nlm_release_host(host);

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 10:45:16.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 10:58:38.000000000 +1000
@@ -134,13 +134,6 @@ struct nlm_block {
 };
 
 /*
- * Valid actions for nlmsvc_traverse_files
- */
-#define NLM_ACT_CHECK		0		/* check for locks */
-#define NLM_ACT_MARK		1		/* mark & sweep */
-#define NLM_ACT_UNLOCK		2		/* release all locks */
-
-/*
  * Global variables
  */
 extern struct rpc_program	nlm_program;
@@ -183,6 +176,12 @@ void		  nsm_release(struct nsm_handle *)
 
 
 /*
+ * This is used in garbage collection and resource reclaim
+ * A return value != 0 means destroy the lock/block/share
+ */
+typedef int	  (*nlm_host_match_fn_t)(struct nlm_host *cur, struct nlm_host *ref);
+
+/*
  * Server-side lock handling
  */
 u32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
@@ -193,7 +192,7 @@ u32		  nlmsvc_testlock(struct nlm_file *
 u32		  nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
 unsigned long	  nlmsvc_retry_blocked(void);
 void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
-					int action);
+					nlm_host_match_fn_t match);
 void	  nlmsvc_grant_reply(struct svc_rqst *, struct nlm_cookie *, u32);
 
 /*

diff .prev/include/linux/lockd/share.h ./include/linux/lockd/share.h
--- .prev/include/linux/lockd/share.h	2006-09-01 10:39:06.000000000 +1000
+++ ./include/linux/lockd/share.h	2006-09-01 10:58:38.000000000 +1000
@@ -25,6 +25,7 @@ u32	nlmsvc_share_file(struct nlm_host *,
 					       struct nlm_args *);
 u32	nlmsvc_unshare_file(struct nlm_host *, struct nlm_file *,
 					       struct nlm_args *);
-void	nlmsvc_traverse_shares(struct nlm_host *, struct nlm_file *, int);
+void	nlmsvc_traverse_shares(struct nlm_host *, struct nlm_file *,
+					       nlm_host_match_fn_t);
 
 #endif /* LINUX_LOCKD_SHARE_H */

[PATCH 012 of 19] knfsd: lockd: Add nlm_destroy_host

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch moves the host destruction code out of nlm_host_gc
  into a function of its own.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c |   45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-08-31 17:32:46.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-01 11:07:19.000000000 +1000
@@ -164,6 +164,34 @@ out:
 	return host;
 }
 
+/*
+ * Destroy a host
+ */
+static void
+nlm_destroy_host(struct nlm_host *host)
+{
+	struct rpc_clnt	*clnt;
+
+	BUG_ON(!list_empty(&host->h_lockowners));
+	BUG_ON(atomic_read(&host->h_count));
+
+	/*
+	 * Release NSM handle and unmonitor host.
+	 */
+	nsm_unmonitor(host);
+
+	if ((clnt = host->h_rpcclnt) != NULL) {
+		if (atomic_read(&clnt->cl_users)) {
+			printk(KERN_WARNING
+				"lockd: active RPC handle\n");
+			clnt->cl_dead = 1;
+		} else {
+			rpc_destroy_client(host->h_rpcclnt);
+		}
+	}
+	kfree(host);
+}
+
 struct nlm_host *
 nlm_find_client(void)
 {
@@ -400,7 +428,6 @@ nlm_gc_hosts(void)
 	struct hlist_head *chain;
 	struct hlist_node *pos, *next;
 	struct nlm_host	*host;
-	struct rpc_clnt	*clnt;
 
 	dprintk("lockd: host garbage collection\n");
 	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
@@ -423,21 +450,7 @@ nlm_gc_hosts(void)
 			dprintk("lockd: delete host %s\n", host->h_name);
 			hlist_del_init(&host->h_hash);
 
-			/*
-			 * Unmonitor unless host was invalidated (i.e. lockd restarted)
-			 */
-			nsm_unmonitor(host);
-
-			if ((clnt = host->h_rpcclnt) != NULL) {
-				if (atomic_read(&clnt->cl_users)) {
-					printk(KERN_WARNING
-						"lockd: active RPC handle\n");
-					clnt->cl_dead = 1;
-				} else {
-					rpc_destroy_client(host->h_rpcclnt);
-				}
-			}
-			kfree(host);
+			nlm_destroy_host(host);
 			nrhosts--;
 		}
 	}

[PATCH 013 of 19] knfsd: Simplify nlmsvc_invalidate_all

[NeilBrown]

  As a result of previous patches, the loop in nlmsvc_invalidate_all
  just sets h_expires for all client/hosts to 0 (though does it in a
  very complicated way).

  This was possibly meant to trigger early garbage collection but
  half the time '0' is in the future and so it infact delays garbage
  collection.

  Pre-aging the 'hosts' is not really needed at this point anyway
  so we throw out the loop and nlm_find_client which is no longer
  needed.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c             |   27 ---------------------------
 ./fs/lockd/svcsubs.c          |   10 +---------
 ./include/linux/lockd/lockd.h |    4 +---
 3 files changed, 2 insertions(+), 39 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 11:07:19.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-01 11:37:34.000000000 +1000
@@ -192,33 +192,6 @@ nlm_destroy_host(struct nlm_host *host)
 	kfree(host);
 }
 
-struct nlm_host *
-nlm_find_client(void)
-{
-	struct hlist_head *chain;
-	struct hlist_node *pos;
-
-	/* find a nlm_host for a client for which h_killed == 0.
-	 * and return it
-	 */
-	mutex_lock(&nlm_host_mutex);
-	for (chain = nlm_hosts; chain < nlm_hosts + NLM_HOST_NRHASH; ++chain) {
-		struct nlm_host *host;
-
-		hlist_for_each_entry(host, pos, chain, h_hash) {
-			if (host->h_server &&
-			    host->h_killed == 0) {
-				nlm_get_host(host);
-				mutex_unlock(&nlm_host_mutex);
-				return host;
-			}
-		}
-	}
-	mutex_unlock(&nlm_host_mutex);
-	return NULL;
-}
-
-				
 /*
  * Create the NLM RPC client for an NLM peer
  */

diff .prev/fs/lockd/svcsubs.c ./fs/lockd/svcsubs.c
--- .prev/fs/lockd/svcsubs.c	2006-09-01 11:06:21.000000000 +1000
+++ ./fs/lockd/svcsubs.c	2006-09-01 11:38:14.000000000 +1000
@@ -354,13 +354,11 @@ nlmsvc_free_host_resources(struct nlm_ho
 }
 
 /*
- * delete all hosts structs for clients
+ * Remove all locks held for clients
  */
 void
 nlmsvc_invalidate_all(void)
 {
-	struct nlm_host *host;
-
 	/* Release all locks held by NFS clients.
 	 * Previously, the code would call
 	 * nlmsvc_free_host_resources for each client in
@@ -368,10 +366,4 @@ nlmsvc_invalidate_all(void)
 	 * Now we just do it once in nlm_traverse_files.
 	 */
 	nlm_traverse_files(NULL, nlmsvc_is_client);
-
-	while ((host = nlm_find_client()) != NULL) {
-		host->h_expires = 0;
-		host->h_killed = 1;
-		nlm_release_host(host);
-	}
 }

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 10:58:38.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 11:38:57.000000000 +1000
@@ -45,8 +45,7 @@ struct nlm_host {
 	unsigned short		h_proto;	/* transport proto */
 	unsigned short		h_reclaiming : 1,
 				h_server     : 1, /* server side, not client side */
-				h_inuse      : 1,
-				h_killed     : 1;
+				h_inuse      : 1;
 	wait_queue_head_t	h_gracewait;	/* wait while reclaiming */
 	struct rw_semaphore	h_rwsem;	/* Reboot recovery lock */
 	u32			h_state;	/* pseudo-state counter */
@@ -169,7 +168,6 @@ void		  nlm_rebind_host(struct nlm_host 
 struct nlm_host * nlm_get_host(struct nlm_host *);
 void		  nlm_release_host(struct nlm_host *);
 void		  nlm_shutdown_hosts(void);
-extern struct nlm_host *nlm_find_client(void);
 extern void	  nlm_host_rebooted(const struct sockaddr_in *, const char *, int, u32);
 struct nsm_handle *nsm_find(const struct sockaddr_in *, const char *, int);
 void		  nsm_release(struct nsm_handle *);

[PATCH 014 of 19] knfsd: lockd: optionally use hostnames for identifying peers

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  This patch adds the nsm_use_hostnames sysctl and module param. If
  set, lockd will use the client's name (as given in the NLM
  arguments) to find the NSM handle. This makes recovery work when the
  NFS peer is multi-homed, and the reboot notification arrives from a
  different IP than the original lock calls.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/host.c                |    6 +++++-
 ./fs/lockd/mon.c                 |   12 +++++++++---
 ./fs/lockd/svc.c                 |   10 ++++++++++
 ./include/linux/lockd/lockd.h    |    1 +
 ./include/linux/lockd/sm_inter.h |    2 ++
 5 files changed, 27 insertions(+), 4 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-09-01 12:11:07.000000000 +1000
+++ ./fs/lockd/host.c	2006-09-01 12:11:16.000000000 +1000
@@ -462,7 +462,11 @@ __nsm_find(const struct sockaddr_in *sin
 	list_for_each(pos, &nsm_handles) {
 		nsm = list_entry(pos, struct nsm_handle, sm_link);
 
-		if (!nlm_cmp_addr(&nsm->sm_addr, sin))
+		if (hostname && nsm_use_hostnames) {
+			if (strlen(nsm->sm_name) != hostname_len
+			 || memcmp(nsm->sm_name, hostname, hostname_len))
+				continue;
+		} else if (!nlm_cmp_addr(&nsm->sm_addr, sin))
 			continue;
 		atomic_inc(&nsm->sm_count);
 		goto out;

diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
--- .prev/fs/lockd/mon.c	2006-09-01 12:11:07.000000000 +1000
+++ ./fs/lockd/mon.c	2006-09-01 12:11:16.000000000 +1000
@@ -47,6 +47,7 @@ nsm_mon_unmon(struct nsm_handle *nsm, u3
 	}
 
 	memset(&args, 0, sizeof(args));
+	args.mon_name = nsm->sm_name;
 	args.addr = nsm->sm_addr.sin_addr.s_addr;
 	args.prog = NLM_PROGRAM;
 	args.vers = 3;
@@ -150,7 +151,7 @@ nsm_create(void)
 static u32 *
 xdr_encode_common(struct rpc_rqst *rqstp, u32 *p, struct nsm_args *argp)
 {
-	char	buffer[20];
+	char	buffer[20], *name;
 
 	/*
 	 * Use the dotted-quad IP address of the remote host as
@@ -158,8 +159,13 @@ xdr_encode_common(struct rpc_rqst *rqstp
 	 * hostname first for whatever remote hostname it receives,
 	 * so this works alright.
 	 */
-	sprintf(buffer, "%u.%u.%u.%u", NIPQUAD(argp->addr));
-	if (!(p = xdr_encode_string(p, buffer))
+	if (nsm_use_hostnames) {
+		name = argp->mon_name;
+	} else {
+		sprintf(buffer, "%u.%u.%u.%u", NIPQUAD(argp->addr));
+		name = buffer;
+	}
+	if (!(p = xdr_encode_string(p, name))
 	 || !(p = xdr_encode_string(p, utsname()->nodename)))
 		return ERR_PTR(-EIO);
 	*p++ = htonl(argp->prog);

diff .prev/fs/lockd/svc.c ./fs/lockd/svc.c
--- .prev/fs/lockd/svc.c	2006-09-01 12:11:07.000000000 +1000
+++ ./fs/lockd/svc.c	2006-09-01 12:15:24.000000000 +1000
@@ -61,6 +61,7 @@ static DECLARE_WAIT_QUEUE_HEAD(lockd_exi
 static unsigned long		nlm_grace_period;
 static unsigned long		nlm_timeout = LOCKD_DFLT_TIMEO;
 static int			nlm_udpport, nlm_tcpport;
+int				nsm_use_hostnames = 0;
 
 /*
  * Constants needed for the sysctl interface.
@@ -395,6 +396,14 @@ static ctl_table nlm_sysctls[] = {
 		.extra1		= (int *) &nlm_port_min,
 		.extra2		= (int *) &nlm_port_max,
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "nsm_use_hostnames",
+		.data		= &nsm_use_hostnames,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
 	{ .ctl_name = 0 }
 };
 
@@ -483,6 +492,7 @@ module_param_call(nlm_udpport, param_set
 		  &nlm_udpport, 0644);
 module_param_call(nlm_tcpport, param_set_port, param_get_int,
 		  &nlm_tcpport, 0644);
+module_param(nsm_use_hostnames, bool, 0644);
 
 /*
  * Initialising and terminating the module.

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 12:11:05.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 12:16:06.000000000 +1000
@@ -142,6 +142,7 @@ extern struct svc_procedure	nlmsvc_proce
 #endif
 extern int			nlmsvc_grace_period;
 extern unsigned long		nlmsvc_timeout;
+extern int			nsm_use_hostnames;
 
 /*
  * Lockd client functions

diff .prev/include/linux/lockd/sm_inter.h ./include/linux/lockd/sm_inter.h
--- .prev/include/linux/lockd/sm_inter.h	2006-09-01 12:11:07.000000000 +1000
+++ ./include/linux/lockd/sm_inter.h	2006-09-01 12:11:16.000000000 +1000
@@ -28,6 +28,8 @@ struct nsm_args {
 	u32		prog;		/* RPC callback info */
 	u32		vers;
 	u32		proc;
+
+	char *		mon_name;
 };
 
 /*

[PATCH 015 of 19] knfsd: make nlmclnt_next_cookie SMP safe

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  The way we incremented the NLM cookie in nlmclnt_next_cookie
  was not thread safe. This patch changes the counter to an
  atomic_t

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/clntproc.c         |   10 +++++-----
 ./include/linux/lockd/lockd.h |    1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff .prev/fs/lockd/clntproc.c ./fs/lockd/clntproc.c
--- .prev/fs/lockd/clntproc.c	2006-09-01 12:11:03.000000000 +1000
+++ ./fs/lockd/clntproc.c	2006-09-01 12:17:03.000000000 +1000
@@ -36,14 +36,14 @@ static const struct rpc_call_ops nlmclnt
 /*
  * Cookie counter for NLM requests
  */
-static u32	nlm_cookie = 0x1234;
+static atomic_t	nlm_cookie = ATOMIC_INIT(0x1234);
 
-static inline void nlmclnt_next_cookie(struct nlm_cookie *c)
+void nlmclnt_next_cookie(struct nlm_cookie *c)
 {
-	memcpy(c->data, &nlm_cookie, 4);
-	memset(c->data+4, 0, 4);
+	u32	cookie = atomic_inc_return(&nlm_cookie);
+
+	memcpy(c->data, &cookie, 4);
 	c->len=4;
-	nlm_cookie++;
 }
 
 static struct nlm_lockowner *nlm_get_lockowner(struct nlm_lockowner *lockowner)

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 12:16:06.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 12:17:03.000000000 +1000
@@ -157,6 +157,7 @@ int		  nlmclnt_block(struct nlm_wait *bl
 u32		  nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *);
 void		  nlmclnt_recovery(struct nlm_host *);
 int		  nlmclnt_reclaim(struct nlm_host *, struct file_lock *);
+void		  nlmclnt_next_cookie(struct nlm_cookie *);
 
 /*
  * Host cache

[PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

 When we send a GRANTED_MSG call, we current copy the NLM cookie
 provided in the original LOCK call - because in 1996, some broken
 clients seemed to rely on this bug. However, this means the cookies
 are not unique, so that when the client's GRANTED_RES message comes
 back, we cannot simply match it based on the cookie, but have to
 use the client's IP address in addition. Which breaks when you have
 a multi-homed NFS client.
 
 The X/Open spec explicitly mentions that clients should not expect the
 same cookie; so one may hope that any clients that were broken in 1996
 have either been fixed or rendered obsolete.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/svc4proc.c         |    2 +-
 ./fs/lockd/svclock.c          |   24 +++++++++++++-----------
 ./fs/lockd/svcproc.c          |    2 +-
 ./include/linux/lockd/lockd.h |    2 +-
 4 files changed, 16 insertions(+), 14 deletions(-)

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-09-01 12:17:21.000000000 +1000
@@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst
 
         dprintk("lockd: GRANTED_RES   called\n");
 
-        nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+        nlmsvc_grant_reply(&argp->cookie, argp->status);
         return rpc_success;
 }
 

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-09-01 12:17:21.000000000 +1000
@@ -139,19 +139,19 @@ static inline int nlm_cookie_match(struc
  * Find a block with a given NLM cookie.
  */
 static inline struct nlm_block *
-nlmsvc_find_block(struct nlm_cookie *cookie,  struct sockaddr_in *sin)
+nlmsvc_find_block(struct nlm_cookie *cookie)
 {
 	struct nlm_block *block;
 
 	list_for_each_entry(block, &nlm_blocked, b_list) {
-		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie)
-				&& nlm_cmp_addr(sin, &block->b_host->h_addr))
+		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie))
 			goto found;
 	}
 
 	return NULL;
 
 found:
+	dprintk("nlmsvc_find_block(%s): block=%p\n", nlmdbg_cookie2a(cookie), block);
 	kref_get(&block->b_count);
 	return block;
 }
@@ -165,6 +165,11 @@ found:
  * request, but (as I found out later) that's because some implementations
  * do just this. Never mind the standards comittees, they support our
  * logging industries.
+ *
+ * 10 years later: I hope we can safely ignore these old and broken
+ * clients by now. Let's fix this so we can uniquely identify an incoming
+ * GRANTED_RES message by cookie, without having to rely on the client's IP
+ * address. --okir
  */
 static inline struct nlm_block *
 nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
@@ -197,7 +202,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
 	/* Set notifier function for VFS, and init args */
 	call->a_args.lock.fl.fl_flags |= FL_SLEEP;
 	call->a_args.lock.fl.fl_lmops = &nlmsvc_lock_operations;
-	call->a_args.cookie = *cookie;	/* see above */
+	nlmclnt_next_cookie(&call->a_args.cookie);
 
 	dprintk("lockd: created block %p...\n", block);
 
@@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_
  * block.
  */
 void
-nlmsvc_grant_reply(struct svc_rqst *rqstp, struct nlm_cookie *cookie, u32 status)
+nlmsvc_grant_reply(struct nlm_cookie *cookie, u32 status)
 {
 	struct nlm_block	*block;
-	struct nlm_file		*file;
 
-	dprintk("grant_reply: looking for cookie %x, host (%08x), s=%d \n", 
-		*(unsigned int *)(cookie->data), 
-		ntohl(rqstp->rq_addr.sin_addr.s_addr), status);
-	if (!(block = nlmsvc_find_block(cookie, &rqstp->rq_addr)))
+	dprintk("grant_reply: looking for cookie %x, s=%d \n",
+		*(unsigned int *)(cookie->data), status);
+	if (!(block = nlmsvc_find_block(cookie)))
 		return;
-	file = block->b_file;
 
 	if (block) {
 		if (status == NLM_LCK_DENIED_GRACE_PERIOD) {

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-09-01 12:11:01.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-09-01 12:17:21.000000000 +1000
@@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst 
 
 	dprintk("lockd: GRANTED_RES   called\n");
 
-	nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
+	nlmsvc_grant_reply(&argp->cookie, argp->status);
 	return rpc_success;
 }
 

diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
--- .prev/include/linux/lockd/lockd.h	2006-09-01 12:17:03.000000000 +1000
+++ ./include/linux/lockd/lockd.h	2006-09-01 12:17:21.000000000 +1000
@@ -193,7 +193,7 @@ u32		  nlmsvc_cancel_blocked(struct nlm_
 unsigned long	  nlmsvc_retry_blocked(void);
 void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
 					nlm_host_match_fn_t match);
-void	  nlmsvc_grant_reply(struct svc_rqst *, struct nlm_cookie *, u32);
+void		  nlmsvc_grant_reply(struct nlm_cookie *, u32);
 
 /*
  * File handling for the server personality

Re: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies

[Trond Myklebust]

On Fri, 2006-09-01 at 14:39 +1000, NeilBrown wrote:
> From: Olaf Kirch <okir@suse.de>
> 
>  When we send a GRANTED_MSG call, we current copy the NLM cookie
>  provided in the original LOCK call - because in 1996, some broken
>  clients seemed to rely on this bug. However, this means the cookies
>  are not unique, so that when the client's GRANTED_RES message comes
>  back, we cannot simply match it based on the cookie, but have to
>  use the client's IP address in addition. Which breaks when you have
>  a multi-homed NFS client.
>  
>  The X/Open spec explicitly mentions that clients should not expect the
>  same cookie; so one may hope that any clients that were broken in 1996
>  have either been fixed or rendered obsolete.

Vetoed. The reason why we need the client's IP address as an argument
for nlmsvc_find_block() is 'cos the cookie value is unique to the
_client_ only.

IOW: when we go searching a global list of blocks for a given cookie
value that was sent to us by a given client, then we want to know that
we're only searching through that particular client's blocks.

A better way, BTW, would be to get rid of the global list nlm_blocked,
and just move the list of blocks into a field in the nlm_host for each
client.

> Signed-off-by: Olaf Kirch <okir@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
> 
> ### Diffstat output
>  ./fs/lockd/svc4proc.c         |    2 +-
>  ./fs/lockd/svclock.c          |   24 +++++++++++++-----------
>  ./fs/lockd/svcproc.c          |    2 +-
>  ./include/linux/lockd/lockd.h |    2 +-
>  4 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
> --- .prev/fs/lockd/svc4proc.c	2006-09-01 12:11:01.000000000 +1000
> +++ ./fs/lockd/svc4proc.c	2006-09-01 12:17:21.000000000 +1000
> @@ -455,7 +455,7 @@ nlm4svc_proc_granted_res(struct svc_rqst
>  
>          dprintk("lockd: GRANTED_RES   called\n");
>  
> -        nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
> +        nlmsvc_grant_reply(&argp->cookie, argp->status);
>          return rpc_success;
>  }
>  
> 
> diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
> --- .prev/fs/lockd/svclock.c	2006-09-01 12:11:01.000000000 +1000
> +++ ./fs/lockd/svclock.c	2006-09-01 12:17:21.000000000 +1000
> @@ -139,19 +139,19 @@ static inline int nlm_cookie_match(struc
>   * Find a block with a given NLM cookie.
>   */
>  static inline struct nlm_block *
> -nlmsvc_find_block(struct nlm_cookie *cookie,  struct sockaddr_in *sin)
> +nlmsvc_find_block(struct nlm_cookie *cookie)
>  {
>  	struct nlm_block *block;
>  
>  	list_for_each_entry(block, &nlm_blocked, b_list) {
> -		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie)
> -				&& nlm_cmp_addr(sin, &block->b_host->h_addr))
> +		if (nlm_cookie_match(&block->b_call->a_args.cookie,cookie))
>  			goto found;
>  	}
>  
>  	return NULL;
>  
>  found:
> +	dprintk("nlmsvc_find_block(%s): block=%p\n", nlmdbg_cookie2a(cookie), block);
>  	kref_get(&block->b_count);
>  	return block;
>  }
> @@ -165,6 +165,11 @@ found:
>   * request, but (as I found out later) that's because some implementations
>   * do just this. Never mind the standards comittees, they support our
>   * logging industries.
> + *
> + * 10 years later: I hope we can safely ignore these old and broken
> + * clients by now. Let's fix this so we can uniquely identify an incoming
> + * GRANTED_RES message by cookie, without having to rely on the client's IP
> + * address. --okir
>   */
>  static inline struct nlm_block *
>  nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_file *file,
> @@ -197,7 +202,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
>  	/* Set notifier function for VFS, and init args */
>  	call->a_args.lock.fl.fl_flags |= FL_SLEEP;
>  	call->a_args.lock.fl.fl_lmops = &nlmsvc_lock_operations;
> -	call->a_args.cookie = *cookie;	/* see above */
> +	nlmclnt_next_cookie(&call->a_args.cookie);
>  
>  	dprintk("lockd: created block %p...\n", block);
>  
> @@ -640,17 +645,14 @@ static const struct rpc_call_ops nlmsvc_
>   * block.
>   */
>  void
> -nlmsvc_grant_reply(struct svc_rqst *rqstp, struct nlm_cookie *cookie, u32 status)
> +nlmsvc_grant_reply(struct nlm_cookie *cookie, u32 status)
>  {
>  	struct nlm_block	*block;
> -	struct nlm_file		*file;
>  
> -	dprintk("grant_reply: looking for cookie %x, host (%08x), s=%d \n", 
> -		*(unsigned int *)(cookie->data), 
> -		ntohl(rqstp->rq_addr.sin_addr.s_addr), status);
> -	if (!(block = nlmsvc_find_block(cookie, &rqstp->rq_addr)))
> +	dprintk("grant_reply: looking for cookie %x, s=%d \n",
> +		*(unsigned int *)(cookie->data), status);
> +	if (!(block = nlmsvc_find_block(cookie)))
>  		return;
> -	file = block->b_file;
>  
>  	if (block) {
>  		if (status == NLM_LCK_DENIED_GRACE_PERIOD) {
> 
> diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
> --- .prev/fs/lockd/svcproc.c	2006-09-01 12:11:01.000000000 +1000
> +++ ./fs/lockd/svcproc.c	2006-09-01 12:17:21.000000000 +1000
> @@ -484,7 +484,7 @@ nlmsvc_proc_granted_res(struct svc_rqst 
>  
>  	dprintk("lockd: GRANTED_RES   called\n");
>  
> -	nlmsvc_grant_reply(rqstp, &argp->cookie, argp->status);
> +	nlmsvc_grant_reply(&argp->cookie, argp->status);
>  	return rpc_success;
>  }
>  
> 
> diff .prev/include/linux/lockd/lockd.h ./include/linux/lockd/lockd.h
> --- .prev/include/linux/lockd/lockd.h	2006-09-01 12:17:03.000000000 +1000
> +++ ./include/linux/lockd/lockd.h	2006-09-01 12:17:21.000000000 +1000
> @@ -193,7 +193,7 @@ u32		  nlmsvc_cancel_blocked(struct nlm_
>  unsigned long	  nlmsvc_retry_blocked(void);
>  void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
>  					nlm_host_match_fn_t match);
> -void	  nlmsvc_grant_reply(struct svc_rqst *, struct nlm_cookie *, u32);
> +void		  nlmsvc_grant_reply(struct nlm_cookie *, u32);
>  
>  /*
>   * File handling for the server personality
> 
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs

Re: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies

[Olaf Kirch]

On Fri, Sep 01, 2006 at 12:03:38PM -0400, Trond Myklebust wrote:
> Vetoed. The reason why we need the client's IP address as an argument
> for nlmsvc_find_block() is 'cos the cookie value is unique to the
> _client_ only.

In NLM, a cookie can be used to identify the asynchronous reply to the
original request. Previously, there was a hack in the code that
sends GRANT replies to reuse the original cookie from the client's
LOCK request. The protocol spec explicitly says you must not rely
on this behavior; the only reason I added this kludge was that some
HPUX and SunOS versions did just that.

The down side of that kludge is that we are using a client-provided cookie
in one of our calls - which means we keep our fingers crossed it doesn't
collide with the cookie we generate ourselves. And to reduce the risk,
we also check the client IP when searching the nlm_blocked list. But it
is incorrect, and a kludge, and the longer I look at this code the
more I'm amazed it hasn't blown up.

This patch changes the code so that the only cookies we ever use
to look up something are those we generate ourselves, so they
are globally unique. As a consequence, we can stop using the client's
IP when searching the list.

> IOW: when we go searching a global list of blocks for a given cookie
> value that was sent to us by a given client, then we want to know that
> we're only searching through that particular client's blocks.

This is no longer true after applying this change.

> A better way, BTW, would be to get rid of the global list nlm_blocked,
> and just move the list of blocks into a field in the nlm_host for each
> client.

Given an incoming NLM_GRANTED_RES, how can you look up the nlm_host
and the pending NLM_GRANTED_MSG?
The reply may not come from any IP address you know of. The whole
reason for introducing this cookie nonsense in the NLM specification
was that the RPC layer doesn't always give you enough clues to
match a callback to the original message.

So this is really a bugfix which you *do* want to apply.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

-- 
VGER BF report: H 0.149416

Re: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies

[Trond Myklebust]

On Mon, 2006-09-04 at 11:09 +0200, Olaf Kirch wrote:
> On Fri, Sep 01, 2006 at 12:03:38PM -0400, Trond Myklebust wrote:
> > Vetoed. The reason why we need the client's IP address as an argument
> > for nlmsvc_find_block() is 'cos the cookie value is unique to the
> > _client_ only.
> 
> In NLM, a cookie can be used to identify the asynchronous reply to the
> original request. Previously, there was a hack in the code that
> sends GRANT replies to reuse the original cookie from the client's
> LOCK request. The protocol spec explicitly says you must not rely
> on this behavior; the only reason I added this kludge was that some
> HPUX and SunOS versions did just that.
> 
> The down side of that kludge is that we are using a client-provided cookie
> in one of our calls - which means we keep our fingers crossed it doesn't
> collide with the cookie we generate ourselves. And to reduce the risk,
> we also check the client IP when searching the nlm_blocked list. But it
> is incorrect, and a kludge, and the longer I look at this code the
> more I'm amazed it hasn't blown up.
> 
> This patch changes the code so that the only cookies we ever use
> to look up something are those we generate ourselves, so they
> are globally unique. As a consequence, we can stop using the client's
> IP when searching the list.
> 
> > IOW: when we go searching a global list of blocks for a given cookie
> > value that was sent to us by a given client, then we want to know that
> > we're only searching through that particular client's blocks.
> 
> This is no longer true after applying this change.

Sorry, I missed that change. I see your point now.

> > A better way, BTW, would be to get rid of the global list nlm_blocked,
> > and just move the list of blocks into a field in the nlm_host for each
> > client.
> 
> Given an incoming NLM_GRANTED_RES, how can you look up the nlm_host
> and the pending NLM_GRANTED_MSG?
> The reply may not come from any IP address you know of. The whole
> reason for introducing this cookie nonsense in the NLM specification
> was that the RPC layer doesn't always give you enough clues to
> match a callback to the original message.

Right. The question is how many clients out there do still rely on the
current behaviour?

Looking at Brent's 'NFS Illustrated', I see that he notes that for
NLM_GRANTED, the cookie is "An opaque value that is normally the same as
the client sent in the LOCK request, though the client cannot depend on
it". Which sounds like weasel words for "some clients still do depend on
it".

Cheers
  Trond

Re: [NFS] [PATCH 016 of 19] knfsd: match GRANTED_RES replies using cookies

[Olaf Kirch]

On Tue, Sep 05, 2006 at 12:12:30PM -0400, Trond Myklebust wrote:
> Right. The question is how many clients out there do still rely on the
> current behaviour?

Right. I know that old SunOS releases did, as did HPUX-6.5 or whatever
it was. But that was ages ago.

Suse has had this code change for a while now in SL10.1 (6 months) as
well as SLES10, and I haven't heard of any interoperability problems,
neither from partners or users, nor from our own QA.

> Looking at Brent's 'NFS Illustrated', I see that he notes that for
> NLM_GRANTED, the cookie is "An opaque value that is normally the same as
> the client sent in the LOCK request, though the client cannot depend on
> it". Which sounds like weasel words for "some clients still do depend on
> it".

The spec says the same (except it doesn't mention that the cookie is
"normally the same", it just explicitly states that the client must not
rely on the cookie in the GRANT call being the same as the one in the
LOCK call).

My guess is that this was an implementation "shortcut" in the original
Sun code that metastased into all the derived implementations, and when
they discovered it was a bad bug that could lead to lock mix-up, this
sloppiness had spread all over the Unix landscape and they needed to
put stern words into the standard...

In summary, I think more than 10 years after the publication of the
X/Open NLM spec it is acceptable to remove that kludge.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

[PATCH 017 of 19] knfsd: Export nsm_local_state to user space via sysctl

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  Every NLM call includes the client's NSM state. Currently,
  the Linux client always reports 0 - which seems not to cause
  any problems, but is not what the protocol says.

  This patch exposes the kernel's internal variable to user
  space via a sysctl, which can be set at system boot time
  by statd.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/mon.c                 |    2 +-
 ./fs/lockd/svc.c                 |    9 +++++++++
 ./include/linux/lockd/sm_inter.h |    2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff .prev/fs/lockd/mon.c ./fs/lockd/mon.c
--- .prev/fs/lockd/mon.c	2006-09-01 12:11:16.000000000 +1000
+++ ./fs/lockd/mon.c	2006-09-01 12:17:47.000000000 +1000
@@ -24,7 +24,7 @@ static struct rpc_program	nsm_program;
 /*
  * Local NSM state
  */
-u32				nsm_local_state;
+int				nsm_local_state;
 
 /*
  * Common procedure for SM_MON/SM_UNMON calls

diff .prev/fs/lockd/svc.c ./fs/lockd/svc.c
--- .prev/fs/lockd/svc.c	2006-09-01 12:15:24.000000000 +1000
+++ ./fs/lockd/svc.c	2006-09-01 12:19:26.000000000 +1000
@@ -33,6 +33,7 @@
 #include <linux/sunrpc/svcsock.h>
 #include <net/ip.h>
 #include <linux/lockd/lockd.h>
+#include <linux/lockd/sm_inter.h>
 #include <linux/nfs.h>
 
 #define NLMDBG_FACILITY		NLMDBG_SVC
@@ -404,6 +405,14 @@ static ctl_table nlm_sysctls[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "nsm_local_state",
+		.data		= &nsm_local_state,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
 	{ .ctl_name = 0 }
 };
 

diff .prev/include/linux/lockd/sm_inter.h ./include/linux/lockd/sm_inter.h
--- .prev/include/linux/lockd/sm_inter.h	2006-09-01 12:11:16.000000000 +1000
+++ ./include/linux/lockd/sm_inter.h	2006-09-01 12:17:47.000000000 +1000
@@ -42,6 +42,6 @@ struct nsm_res {
 
 int		nsm_monitor(struct nlm_host *);
 int		nsm_unmonitor(struct nlm_host *);
-extern u32	nsm_local_state;
+extern int	nsm_local_state;
 
 #endif /* LINUX_LOCKD_SM_INTER_H */

[PATCH 018 of 19] knfsd: lockd: fix use of h_nextrebind

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  nlmclnt_recovery would try to force a portmap rebind by setting
  host->h_nextrebind to 0. The right thing to do here is to set it
  to the current time.

Signed-off-by: okir@suse.de
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/lockd/clntlock.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
--- .prev/fs/lockd/clntlock.c	2006-08-31 17:02:23.000000000 +1000
+++ ./fs/lockd/clntlock.c	2006-09-01 12:19:55.000000000 +1000
@@ -184,7 +184,7 @@ restart:
 	/* Force a portmap getport - the peer's lockd will
 	 * most likely end up on a different port.
 	 */
-	host->h_nextrebind = 0;
+	host->h_nextrebind = jiffies;
 	nlm_rebind_host(host);
 
 	/* First, reclaim all locks that have been granted. */

Re: [NFS] [PATCH 018 of 19] knfsd: lockd: fix use of h_nextrebind

[Trond Myklebust]

On Fri, 2006-09-01 at 14:39 +1000, NeilBrown wrote:
> From: Olaf Kirch <okir@suse.de>
> 
>   nlmclnt_recovery would try to force a portmap rebind by setting
>   host->h_nextrebind to 0. The right thing to do here is to set it
>   to the current time.

Could we instead just add a routine nlm_force_rebind_host() into host.c?

> Signed-off-by: okir@suse.de
> Signed-off-by: Neil Brown <neilb@suse.de>
> 
> ### Diffstat output
>  ./fs/lockd/clntlock.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff .prev/fs/lockd/clntlock.c ./fs/lockd/clntlock.c
> --- .prev/fs/lockd/clntlock.c	2006-08-31 17:02:23.000000000 +1000
> +++ ./fs/lockd/clntlock.c	2006-09-01 12:19:55.000000000 +1000
> @@ -184,7 +184,7 @@ restart:
>  	/* Force a portmap getport - the peer's lockd will
>  	 * most likely end up on a different port.
>  	 */
> -	host->h_nextrebind = 0;
> +	host->h_nextrebind = jiffies;
>  	nlm_rebind_host(host);
>  
>  	/* First, reclaim all locks that have been granted. */
> 
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs

[PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[NeilBrown]

From: Olaf Kirch <okir@suse.de>

  The NFSACL patches introduced support for multiple RPC services
  listening on the same transport. However, only the first of these
  services was registered with portmapper. This was perfectly fine
  for nfsacl, as you traditionally do not want these to show up in a
  portmapper listing.

  The patch below changes the default behavior to always register
  all services listening on a given transport, but retains the
  old behavior for nfsacl services.

Signed-off-by: Olaf Kirch <okir@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/nfsd/nfs2acl.c          |    1 +
 ./fs/nfsd/nfs3acl.c          |    1 +
 ./include/linux/sunrpc/svc.h |    3 +++
 ./net/sunrpc/svc.c           |   37 +++++++++++++++++++++++--------------
 4 files changed, 28 insertions(+), 14 deletions(-)

diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
--- .prev/fs/nfsd/nfs2acl.c	2006-09-01 12:22:10.000000000 +1000
+++ ./fs/nfsd/nfs2acl.c	2006-09-01 12:22:11.000000000 +1000
@@ -333,4 +333,5 @@ struct svc_version	nfsd_acl_version2 = {
 		.vs_proc	= nfsd_acl_procedures2,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
+		.vs_hidden	= 1,
 };

diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
--- .prev/fs/nfsd/nfs3acl.c	2006-09-01 12:22:10.000000000 +1000
+++ ./fs/nfsd/nfs3acl.c	2006-09-01 12:22:11.000000000 +1000
@@ -263,5 +263,6 @@ struct svc_version	nfsd_acl_version3 = {
 		.vs_proc	= nfsd_acl_procedures3,
 		.vs_dispatch	= nfsd_dispatch,
 		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
+		.vs_hidden	= 1,
 };
 

diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
--- .prev/include/linux/sunrpc/svc.h	2006-09-01 12:22:10.000000000 +1000
+++ ./include/linux/sunrpc/svc.h	2006-09-01 12:22:11.000000000 +1000
@@ -304,6 +304,9 @@ struct svc_version {
 	struct svc_procedure *	vs_proc;	/* per-procedure info */
 	u32			vs_xdrsize;	/* xdrsize needed for this version */
 
+	unsigned int		vs_hidden : 1;	/* Don't register with portmapper.
+						 * Only used for nfsacl so far. */
+
 	/* Override dispatch function (e.g. when caching replies).
 	 * A return value of 0 means drop the request. 
 	 * vs_dispatch == NULL means use default dispatcher.

diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
--- .prev/net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
+++ ./net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
@@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int 
 	unsigned long		flags;
 	int			i, error = 0, dummy;
 
-	progp = serv->sv_program;
-
-	dprintk("RPC: svc_register(%s, %s, %d)\n",
-		progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
-
 	if (!port)
 		clear_thread_flag(TIF_SIGPENDING);
 
-	for (i = 0; i < progp->pg_nvers; i++) {
-		if (progp->pg_vers[i] == NULL)
-			continue;
-		error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
-		if (error < 0)
-			break;
-		if (port && !dummy) {
-			error = -EACCES;
-			break;
+	for (progp = serv->sv_program; progp; progp = progp->pg_next) {
+		for (i = 0; i < progp->pg_nvers; i++) {
+			if (progp->pg_vers[i] == NULL)
+				continue;
+
+			dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
+					progp->pg_name,
+					proto == IPPROTO_UDP?  "udp" : "tcp",
+					port,
+					i,
+					progp->pg_vers[i]->vs_hidden?
+						" (but not telling portmap)" : "");
+
+			if (progp->pg_vers[i]->vs_hidden)
+				continue;
+
+			error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
+			if (error < 0)
+				break;
+			if (port && !dummy) {
+				error = -EACCES;
+				break;
+			}
 		}
 	}
 

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Peter Staubach]

NeilBrown wrote:
> From: Olaf Kirch <okir@suse.de>
>
>   The NFSACL patches introduced support for multiple RPC services
>   listening on the same transport. However, only the first of these
>   services was registered with portmapper. This was perfectly fine
>   for nfsacl, as you traditionally do not want these to show up in a
>   portmapper listing.
>
>   The patch below changes the default behavior to always register
>   all services listening on a given transport, but retains the
>   old behavior for nfsacl services.
>
> Signed-off-by: Olaf Kirch <okir@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
>
> ### Diffstat output
>  ./fs/nfsd/nfs2acl.c          |    1 +
>  ./fs/nfsd/nfs3acl.c          |    1 +
>  ./include/linux/sunrpc/svc.h |    3 +++
>  ./net/sunrpc/svc.c           |   37 +++++++++++++++++++++++--------------
>  4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c	2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c	2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version	nfsd_acl_version2 = {
>  		.vs_proc	= nfsd_acl_procedures2,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> +		.vs_hidden	= 1,
>  };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c	2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c	2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version	nfsd_acl_version3 = {
>  		.vs_proc	= nfsd_acl_procedures3,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> +		.vs_hidden	= 1,
>  };
>  
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h	2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h	2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
>  	struct svc_procedure *	vs_proc;	/* per-procedure info */
>  	u32			vs_xdrsize;	/* xdrsize needed for this version */
>  
> +	unsigned int		vs_hidden : 1;	/* Don't register with portmapper.
> +						 * Only used for nfsacl so far. */
> +
>  	/* Override dispatch function (e.g. when caching replies).
>  	 * A return value of 0 means drop the request. 
>  	 * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int 
>  	unsigned long		flags;
>  	int			i, error = 0, dummy;
>  
> -	progp = serv->sv_program;
> -
> -	dprintk("RPC: svc_register(%s, %s, %d)\n",
> -		progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
>  	if (!port)
>  		clear_thread_flag(TIF_SIGPENDING);
>  
> -	for (i = 0; i < progp->pg_nvers; i++) {
> -		if (progp->pg_vers[i] == NULL)
> -			continue;
> -		error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> -		if (error < 0)
> -			break;
> -		if (port && !dummy) {
> -			error = -EACCES;
> -			break;
> +	for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> +		for (i = 0; i < progp->pg_nvers; i++) {
> +			if (progp->pg_vers[i] == NULL)
> +				continue;
> +
> +			dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> +					progp->pg_name,
> +					proto == IPPROTO_UDP?  "udp" : "tcp",
> +					port,
> +					i,
> +					progp->pg_vers[i]->vs_hidden?
> +						" (but not telling portmap)" : "");
> +
> +			if (progp->pg_vers[i]->vs_hidden)
> +				continue;
> +
> +			error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> +			if (error < 0)
> +				break;
> +			if (port && !dummy) {
> +				error = -EACCES;
> +				break;
> +			}
>  		}
>  	}

Just out of curiosity, why does the dprintk say "svc_register", but the
code calls rpc_register?

    Thanx...

       ps

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Peter Staubach]

NeilBrown wrote:
> From: Olaf Kirch <okir@suse.de>
>
>   The NFSACL patches introduced support for multiple RPC services
>   listening on the same transport. However, only the first of these
>   services was registered with portmapper. This was perfectly fine
>   for nfsacl, as you traditionally do not want these to show up in a
>   portmapper listing.
>
>   The patch below changes the default behavior to always register
>   all services listening on a given transport, but retains the
>   old behavior for nfsacl services.
>
> Signed-off-by: Olaf Kirch <okir@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
>
> ### Diffstat output
>  ./fs/nfsd/nfs2acl.c          |    1 +
>  ./fs/nfsd/nfs3acl.c          |    1 +
>  ./include/linux/sunrpc/svc.h |    3 +++
>  ./net/sunrpc/svc.c           |   37 +++++++++++++++++++++++--------------
>  4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c	2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c	2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version	nfsd_acl_version2 = {
>  		.vs_proc	= nfsd_acl_procedures2,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> +		.vs_hidden	= 1,
>  };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c	2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c	2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version	nfsd_acl_version3 = {
>  		.vs_proc	= nfsd_acl_procedures3,
>  		.vs_dispatch	= nfsd_dispatch,
>  		.vs_xdrsize	= NFS3_SVC_XDRSIZE,
> +		.vs_hidden	= 1,
>  };
>  
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h	2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h	2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
>  	struct svc_procedure *	vs_proc;	/* per-procedure info */
>  	u32			vs_xdrsize;	/* xdrsize needed for this version */
>  
> +	unsigned int		vs_hidden : 1;	/* Don't register with portmapper.
> +						 * Only used for nfsacl so far. */
> +
>  	/* Override dispatch function (e.g. when caching replies).
>  	 * A return value of 0 means drop the request. 
>  	 * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c	2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int 
>  	unsigned long		flags;
>  	int			i, error = 0, dummy;
>  
> -	progp = serv->sv_program;
> -
> -	dprintk("RPC: svc_register(%s, %s, %d)\n",
> -		progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
>  	if (!port)
>  		clear_thread_flag(TIF_SIGPENDING);
>  
> -	for (i = 0; i < progp->pg_nvers; i++) {
> -		if (progp->pg_vers[i] == NULL)
> -			continue;
> -		error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> -		if (error < 0)
> -			break;
> -		if (port && !dummy) {
> -			error = -EACCES;
> -			break;
> +	for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> +		for (i = 0; i < progp->pg_nvers; i++) {
> +			if (progp->pg_vers[i] == NULL)
> +				continue;
> +
> +			dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> +					progp->pg_name,
> +					proto == IPPROTO_UDP?  "udp" : "tcp",
> +					port,
> +					i,
> +					progp->pg_vers[i]->vs_hidden?
> +						" (but not telling portmap)" : "");
> +
> +			if (progp->pg_vers[i]->vs_hidden)
> +				continue;
> +
> +			error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> +			if (error < 0)
> +				break;
> +			if (port && !dummy) {
> +				error = -EACCES;
> +				break;
> +			}
>  		}
>  	}

While I'm thinking about it a little more, why not register NFS_ACL with
portmap/rpcbind?  That would make it pingable via rpcinfo.

    Thanx...

       ps

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Olaf Kirch]

On Fri, Sep 01, 2006 at 09:29:01AM -0400, Peter Staubach wrote:
> While I'm thinking about it a little more, why not register NFS_ACL with
> portmap/rpcbind?  That would make it pingable via rpcinfo.

I can't say, but I dimly recall that this was intentional, and I
did not want to change this behavior. Andreas Gruenbacher should
know.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Chuck Lever]

On 9/1/06, NeilBrown <neilb@suse.de> wrote:
>
> From: Olaf Kirch <okir@suse.de>
>
>   The NFSACL patches introduced support for multiple RPC services
>   listening on the same transport. However, only the first of these
>   services was registered with portmapper. This was perfectly fine
>   for nfsacl, as you traditionally do not want these to show up in a
>   portmapper listing.
>
>   The patch below changes the default behavior to always register
>   all services listening on a given transport, but retains the
>   old behavior for nfsacl services.

I don't like this.  The idea that multiple RPC services are listening
on the same port is a total hack.  What other service might use this
besides NFSACL?

Does the reference implementation (Solaris) behave this way?

> Signed-off-by: Olaf Kirch <okir@suse.de>
> Signed-off-by: Neil Brown <neilb@suse.de>
>
> ### Diffstat output
>  ./fs/nfsd/nfs2acl.c          |    1 +
>  ./fs/nfsd/nfs3acl.c          |    1 +
>  ./include/linux/sunrpc/svc.h |    3 +++
>  ./net/sunrpc/svc.c           |   37 +++++++++++++++++++++++--------------
>  4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff .prev/fs/nfsd/nfs2acl.c ./fs/nfsd/nfs2acl.c
> --- .prev/fs/nfsd/nfs2acl.c     2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs2acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -333,4 +333,5 @@ struct svc_version  nfsd_acl_version2 = {
>                 .vs_proc        = nfsd_acl_procedures2,
>                 .vs_dispatch    = nfsd_dispatch,
>                 .vs_xdrsize     = NFS3_SVC_XDRSIZE,
> +               .vs_hidden      = 1,
>  };
>
> diff .prev/fs/nfsd/nfs3acl.c ./fs/nfsd/nfs3acl.c
> --- .prev/fs/nfsd/nfs3acl.c     2006-09-01 12:22:10.000000000 +1000
> +++ ./fs/nfsd/nfs3acl.c 2006-09-01 12:22:11.000000000 +1000
> @@ -263,5 +263,6 @@ struct svc_version  nfsd_acl_version3 = {
>                 .vs_proc        = nfsd_acl_procedures3,
>                 .vs_dispatch    = nfsd_dispatch,
>                 .vs_xdrsize     = NFS3_SVC_XDRSIZE,
> +               .vs_hidden      = 1,
>  };
>
>
> diff .prev/include/linux/sunrpc/svc.h ./include/linux/sunrpc/svc.h
> --- .prev/include/linux/sunrpc/svc.h    2006-09-01 12:22:10.000000000 +1000
> +++ ./include/linux/sunrpc/svc.h        2006-09-01 12:22:11.000000000 +1000
> @@ -304,6 +304,9 @@ struct svc_version {
>         struct svc_procedure *  vs_proc;        /* per-procedure info */
>         u32                     vs_xdrsize;     /* xdrsize needed for this version */
>
> +       unsigned int            vs_hidden : 1;  /* Don't register with portmapper.
> +                                                * Only used for nfsacl so far. */
> +
>         /* Override dispatch function (e.g. when caching replies).
>          * A return value of 0 means drop the request.
>          * vs_dispatch == NULL means use default dispatcher.
>
> diff .prev/net/sunrpc/svc.c ./net/sunrpc/svc.c
> --- .prev/net/sunrpc/svc.c      2006-09-01 12:22:11.000000000 +1000
> +++ ./net/sunrpc/svc.c  2006-09-01 12:22:11.000000000 +1000
> @@ -644,23 +644,32 @@ svc_register(struct svc_serv *serv, int
>         unsigned long           flags;
>         int                     i, error = 0, dummy;
>
> -       progp = serv->sv_program;
> -
> -       dprintk("RPC: svc_register(%s, %s, %d)\n",
> -               progp->pg_name, proto == IPPROTO_UDP? "udp" : "tcp", port);
> -
>         if (!port)
>                 clear_thread_flag(TIF_SIGPENDING);
>
> -       for (i = 0; i < progp->pg_nvers; i++) {
> -               if (progp->pg_vers[i] == NULL)
> -                       continue;
> -               error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> -               if (error < 0)
> -                       break;
> -               if (port && !dummy) {
> -                       error = -EACCES;
> -                       break;
> +       for (progp = serv->sv_program; progp; progp = progp->pg_next) {
> +               for (i = 0; i < progp->pg_nvers; i++) {
> +                       if (progp->pg_vers[i] == NULL)
> +                               continue;
> +
> +                       dprintk("RPC: svc_register(%s, %s, %d, %d)%s\n",
> +                                       progp->pg_name,
> +                                       proto == IPPROTO_UDP?  "udp" : "tcp",
> +                                       port,
> +                                       i,
> +                                       progp->pg_vers[i]->vs_hidden?
> +                                               " (but not telling portmap)" : "");
> +
> +                       if (progp->pg_vers[i]->vs_hidden)
> +                               continue;
> +
> +                       error = rpc_register(progp->pg_prog, i, proto, port, &dummy);
> +                       if (error < 0)
> +                               break;
> +                       if (port && !dummy) {
> +                               error = -EACCES;
> +                               break;
> +                       }
>                 }
>         }
>
>
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs
>


-- 
"We who cut mere stones must always be envisioning cathedrals"
   -- Quarry worker's creed

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Olaf Kirch]

On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
> I don't like this.  The idea that multiple RPC services are listening
> on the same port is a total hack.  What other service might use this
> besides NFSACL?

Why do you consider this a hack? I have always felt that librpc requiring
you to open separate ports for every program you register was a poor
design. The RPC header contains the program number, and the RPC code
is fully capable of demuxing incoming requests. So I do not think it is
a hack at all.

And yes, Solaris NFSACL resides on 2049 too.

Olaf
-- 
Olaf Kirch   |  --- o --- Nous sommes du soleil we love when we play
okir@suse.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Chuck Lever]

On 9/1/06, Olaf Kirch <okir@suse.de> wrote:
> On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
> > I don't like this.  The idea that multiple RPC services are listening
> > on the same port is a total hack.  What other service might use this
> > besides NFSACL?
>
> Why do you consider this a hack? I have always felt that librpc requiring
> you to open separate ports for every program you register was a poor
> design. The RPC header contains the program number, and the RPC code
> is fully capable of demuxing incoming requests. So I do not think it is
> a hack at all.
>
> And yes, Solaris NFSACL resides on 2049 too.

I meant "Does Solaris advertise NFSACL on 2049 via the portmapper?"

-- 
"We who cut mere stones must always be envisioning cathedrals"
   -- Quarry worker's creed

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Peter Staubach]

Chuck Lever wrote:
> On 9/1/06, Olaf Kirch <okir@suse.de> wrote:
>   
>> On Fri, Sep 01, 2006 at 11:31:25AM -0400, Chuck Lever wrote:
>>     
>>> I don't like this.  The idea that multiple RPC services are listening
>>> on the same port is a total hack.  What other service might use this
>>> besides NFSACL?
>>>       
>> Why do you consider this a hack? I have always felt that librpc requiring
>> you to open separate ports for every program you register was a poor
>> design. The RPC header contains the program number, and the RPC code
>> is fully capable of demuxing incoming requests. So I do not think it is
>> a hack at all.
>>
>> And yes, Solaris NFSACL resides on 2049 too.
>>     
>
> I meant "Does Solaris advertise NFSACL on 2049 via the portmapper?"

Yes, the Solaris server registers the NFS_ACL service with the rpcbind
daemon.  And, the NFS_ACL protocol is defined to use port 2049.  Please
see nfs_acl.x (/usr/include/rpcsvc/nfs_acl.x) for details.

       ps

Re: [NFS] [PATCH 019 of 19] knfsd: Register all RPC programs with portmapper by default

[Trond Myklebust]

On Fri, 2006-09-01 at 11:31 -0400, Chuck Lever wrote:

> I don't like this.  The idea that multiple RPC services are listening
> on the same port is a total hack.  What other service might use this
> besides NFSACL?

Ermm... Any RPC server process that doesn't want to register with the
portmapper. The NFSv4 callback channel is one obvious candidate.

Cheers,
  Trond