All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 000 of 4] Introduction Possibly patches for RPC client changes.
@ 2006-10-24  2:48 NeilBrown
  2006-10-24  2:48 ` [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client NeilBrown
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: NeilBrown @ 2006-10-24  2:48 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs

These are very much suggestions only.  They have at best been lightly
tested.

The commonality is to try to be uniform in setting the 'hard' and
'intr' flags on requests.

The primary goal is to get rid of the 30second uninterruptible pause
when making a lock request while statd isn't running.

Comments very welcome.

NeilBrown


 [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
 [PATCH 002 of 4] Make the initial RPC PING interruptible.
 [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.
 [PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests.

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-10-24  2:48 [PATCH 000 of 4] Introduction Possibly patches for RPC client changes NeilBrown
@ 2006-10-24  2:48 ` NeilBrown
  2006-10-24 23:05   ` Chuck Lever
  2006-10-24  2:48 ` [PATCH 002 of 4] Make the initial RPC PING interruptible NeilBrown
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: NeilBrown @ 2006-10-24  2:48 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs


When an RPC request needs to make a subordinate RPC request to portmap
to find the port number to communicate with, the 'soft' and 'intr'
flags should be copied from the original request to the subordinate request
to ensure consistent handling.

Currently the pmap client defaults to soft,nointr, so while it is
certain to timeout, it could still be 30 seconds without being
interruptible.

Note that copying the 'hard' flag down is not essential as if the pmap
request aborts, the parent request - being hard - will retry it
indefinitely.  However copying it is more obviously right.

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

### Diffstat output
 ./net/sunrpc/pmap_clnt.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff .prev/net/sunrpc/pmap_clnt.c ./net/sunrpc/pmap_clnt.c
--- .prev/net/sunrpc/pmap_clnt.c	2006-10-24 09:23:13.000000000 +1000
+++ ./net/sunrpc/pmap_clnt.c	2006-10-24 09:33:08.000000000 +1000
@@ -34,7 +34,8 @@ struct portmap_args {
 };
 
 static struct rpc_procinfo	pmap_procedures[];
-static struct rpc_clnt *	pmap_create(char *, struct sockaddr_in *, int, int);
+static struct rpc_clnt *	pmap_create(char *, struct sockaddr_in *,
+					    int, unsigned long);
 static void			pmap_getport_done(struct rpc_task *, void *);
 static struct rpc_program	pmap_program;
 
@@ -93,6 +94,7 @@ void rpc_getport(struct rpc_task *task)
 	struct rpc_clnt	*pmap_clnt;
 	struct rpc_task *child;
 	int status;
+	unsigned long create_flags;
 
 	dprintk("RPC: %4d rpc_getport(%s, %u, %u, %d)\n",
 			task->tk_pid, clnt->cl_server,
@@ -125,7 +127,13 @@ void rpc_getport(struct rpc_task *task)
 	map->pm_xprt = xprt_get(xprt);
 
 	rpc_peeraddr(clnt, (struct sockaddr *) &addr, sizeof(addr));
-	pmap_clnt = pmap_create(clnt->cl_server, &addr, map->pm_prot, 0);
+	create_flags = RPC_CLNT_CREATE_NONPRIVPORT;
+	if ( ! RPC_IS_SOFT(task))
+		create_flags |= RPC_CLNT_CREATE_HARDRTRY;
+	if ( ! RPC_TASK_UNINTERRUPTIBLE(task))
+		create_flags |= RPC_CLNT_CREATE_INTR;
+	pmap_clnt = pmap_create(clnt->cl_server, &addr, map->pm_prot,
+				create_flags);
 	status = PTR_ERR(pmap_clnt);
 	if (IS_ERR(pmap_clnt))
 		goto bailout;
@@ -178,7 +186,7 @@ int rpc_getport_external(struct sockaddr
 			NIPQUAD(sin->sin_addr.s_addr), prog, vers, prot);
 
 	sprintf(hostname, "%u.%u.%u.%u", NIPQUAD(sin->sin_addr.s_addr));
-	pmap_clnt = pmap_create(hostname, sin, prot, 0);
+	pmap_clnt = pmap_create(hostname, sin, prot, RPC_CLNT_CREATE_NONPRIVPORT);
 	if (IS_ERR(pmap_clnt))
 		return PTR_ERR(pmap_clnt);
 
@@ -257,7 +265,7 @@ int rpc_register(u32 prog, u32 vers, int
 	dprintk("RPC: registering (%u, %u, %d, %u) with portmapper.\n",
 			prog, vers, prot, port);
 
-	pmap_clnt = pmap_create("localhost", &sin, IPPROTO_UDP, 1);
+	pmap_clnt = pmap_create("localhost", &sin, IPPROTO_UDP, 0);
 	if (IS_ERR(pmap_clnt)) {
 		error = PTR_ERR(pmap_clnt);
 		dprintk("RPC: couldn't create pmap client. Error = %d\n", error);
@@ -277,7 +285,8 @@ int rpc_register(u32 prog, u32 vers, int
 	return error;
 }
 
-static struct rpc_clnt *pmap_create(char *hostname, struct sockaddr_in *srvaddr, int proto, int privileged)
+static struct rpc_clnt *pmap_create(char *hostname, struct sockaddr_in *srvaddr,
+				    int proto, unsigned long create_flags)
 {
 	struct rpc_create_args args = {
 		.protocol	= proto,
@@ -292,8 +301,7 @@ static struct rpc_clnt *pmap_create(char
 	};
 
 	srvaddr->sin_port = htons(RPC_PMAP_PORT);
-	if (!privileged)
-		args.flags |= RPC_CLNT_CREATE_NONPRIVPORT;
+	args.flags |= create_flags;
 	return rpc_create(&args);
 }
 

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 002 of 4] Make the initial RPC PING interruptible.
  2006-10-24  2:48 [PATCH 000 of 4] Introduction Possibly patches for RPC client changes NeilBrown
  2006-10-24  2:48 ` [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client NeilBrown
@ 2006-10-24  2:48 ` NeilBrown
  2006-10-24 23:19   ` Chuck Lever
  2006-10-24  2:49 ` [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly NeilBrown
  2006-10-24  2:49 ` [PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests NeilBrown
  3 siblings, 1 reply; 16+ messages in thread
From: NeilBrown @ 2006-10-24  2:48 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs


If an RPC client is created with CREATE_INTR and not CREATE_NOPING,
then the ping should be interruptible.  With tcp the PING can take 30
seconds to time out and not being able to interrupt that can be
frustrating.

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

### Diffstat output
 ./net/sunrpc/clnt.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff .prev/net/sunrpc/clnt.c ./net/sunrpc/clnt.c
--- .prev/net/sunrpc/clnt.c	2006-10-24 09:41:49.000000000 +1000
+++ ./net/sunrpc/clnt.c	2006-10-24 09:43:48.000000000 +1000
@@ -221,7 +221,15 @@ struct rpc_clnt *rpc_create(struct rpc_c
 		return clnt;
 
 	if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
-		int err = rpc_ping(clnt, RPC_TASK_SOFT|RPC_TASK_NOINTR);
+		int tskflags = RPC_TASK_SOFT;
+		int err;
+		if ( ! (args->flags & RPC_CLNT_CREATE_INTR))
+			tskflags |= RPC_TASK_NOINTR;
+		/* Note: we keep task_soft even if RPC_CLNT_CREATE_HARDRTRY
+		 * is set.  The whole point of the PING is to check that the
+		 * server is there *now*
+		 */
+		err = rpc_ping(clnt, tskflags);
 		if (err != 0) {
 			rpc_shutdown_client(clnt);
 			return ERR_PTR(err);

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.
  2006-10-24  2:48 [PATCH 000 of 4] Introduction Possibly patches for RPC client changes NeilBrown
  2006-10-24  2:48 ` [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client NeilBrown
  2006-10-24  2:48 ` [PATCH 002 of 4] Make the initial RPC PING interruptible NeilBrown
@ 2006-10-24  2:49 ` NeilBrown
  2006-10-24 23:37   ` Chuck Lever
  2006-11-15 17:14   ` Trond Myklebust
  2006-10-24  2:49 ` [PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests NeilBrown
  3 siblings, 2 replies; 16+ messages in thread
From: NeilBrown @ 2006-10-24  2:49 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs


Sometimes when an RPC client is created, an initial 'PING' is sent to
the RPC/NULL procedure to make sure the server is running.
This is always done as a 'soft' call so a major timeout will be fatal.

We change semantics so that if portmap reports that the service does
not exist, or if a ECONNREFUSED error is returned we abort early
rather than persist for the full timeout.

The core mechanism is a new flag RPC_TASK_ACCEPT_NO which tell the rpc
client to accept a 'no' answer rather than retrying.

This patch also causes lockd clients to *not* do the initial 'ping'
otherwise a lock request could fail due to a missing server, even on a
hard mount.

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

### Diffstat output
 ./fs/lockd/host.c              |    1 +
 ./include/linux/sunrpc/sched.h |    4 ++++
 ./net/sunrpc/clnt.c            |   14 +++++++++++---
 ./net/sunrpc/pmap_clnt.c       |    4 +++-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-10-24 10:09:41.000000000 +1000
+++ ./fs/lockd/host.c	2006-10-24 11:48:05.000000000 +1000
@@ -236,6 +236,7 @@ nlm_bind_host(struct nlm_host *host)
 			.version	= host->h_version,
 			.authflavor	= RPC_AUTH_UNIX,
 			.flags		= (RPC_CLNT_CREATE_HARDRTRY |
+					   RPC_CLNT_CREATE_NOPING |
 					   RPC_CLNT_CREATE_AUTOBIND),
 		};
 

diff .prev/include/linux/sunrpc/sched.h ./include/linux/sunrpc/sched.h
--- .prev/include/linux/sunrpc/sched.h	2006-10-24 11:35:21.000000000 +1000
+++ ./include/linux/sunrpc/sched.h	2006-10-24 11:46:25.000000000 +1000
@@ -133,6 +133,10 @@ struct rpc_call_ops {
 #define RPC_TASK_KILLED		0x0100		/* task was killed */
 #define RPC_TASK_SOFT		0x0200		/* Use soft timeouts */
 #define RPC_TASK_NOINTR		0x0400		/* uninterruptible task */
+#define RPC_TASK_ACCEPT_NO	0x0800		/* Accept a 'no' such as
+						 * -ECONNREFUSED or '0' from
+						 * portmap, to be reliable.
+						 */
 
 #define RPC_IS_ASYNC(t)		((t)->tk_flags & RPC_TASK_ASYNC)
 #define RPC_IS_SWAPPER(t)	((t)->tk_flags & RPC_TASK_SWAPPER)

diff .prev/net/sunrpc/clnt.c ./net/sunrpc/clnt.c
--- .prev/net/sunrpc/clnt.c	2006-10-24 09:43:48.000000000 +1000
+++ ./net/sunrpc/clnt.c	2006-10-24 11:45:54.000000000 +1000
@@ -221,7 +221,7 @@ struct rpc_clnt *rpc_create(struct rpc_c
 		return clnt;
 
 	if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
-		int tskflags = RPC_TASK_SOFT;
+		int tskflags = RPC_TASK_SOFT | RPC_TASK_ACCEPT_NO;
 		int err;
 		if ( ! (args->flags & RPC_CLNT_CREATE_INTR))
 			tskflags |= RPC_TASK_NOINTR;
@@ -868,6 +868,8 @@ call_bind_status(struct rpc_task *task)
 	case -EACCES:
 		dprintk("RPC: %4d remote rpcbind: RPC program/version unavailable\n",
 				task->tk_pid);
+		if (task->tk_flags & RPC_TASK_ACCEPT_NO)
+			break;
 		rpc_delay(task, 3*HZ);
 		goto retry_timeout;
 	case -ETIMEDOUT:
@@ -941,6 +943,8 @@ call_connect_status(struct rpc_task *tas
 	switch (status) {
 	case -ENOTCONN:
 	case -EAGAIN:
+		if (task->tk_flags & RPC_TASK_ACCEPT_NO)
+			break;
 		task->tk_action = call_bind;
 		if (!RPC_IS_SOFT(task))
 			return;
@@ -1044,8 +1048,12 @@ call_status(struct rpc_task *task)
 		break;
 	case -ECONNREFUSED:
 	case -ENOTCONN:
-		rpc_force_rebind(clnt);
-		task->tk_action = call_bind;
+		if (task->tk_flags & RPC_TASK_ACCEPT_NO)
+			rpc_exit(task, status);
+		else {
+			rpc_force_rebind(clnt);
+			task->tk_action = call_bind;
+		}
 		break;
 	case -EAGAIN:
 		task->tk_action = call_transmit;

diff .prev/net/sunrpc/pmap_clnt.c ./net/sunrpc/pmap_clnt.c
--- .prev/net/sunrpc/pmap_clnt.c	2006-10-24 09:33:08.000000000 +1000
+++ ./net/sunrpc/pmap_clnt.c	2006-10-24 11:51:32.000000000 +1000
@@ -139,7 +139,9 @@ void rpc_getport(struct rpc_task *task)
 		goto bailout;
 
 	status = -EIO;
-	child = rpc_run_task(pmap_clnt, RPC_TASK_ASYNC, &pmap_getport_ops, map);
+	child = rpc_run_task(pmap_clnt,
+			     RPC_TASK_ASYNC | (task->tk_flags & RPC_TASK_ACCEPT_NO),
+			     &pmap_getport_ops, map);
 	if (IS_ERR(child))
 		goto bailout;
 	rpc_release_task(child);

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests.
  2006-10-24  2:48 [PATCH 000 of 4] Introduction Possibly patches for RPC client changes NeilBrown
                   ` (2 preceding siblings ...)
  2006-10-24  2:49 ` [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly NeilBrown
@ 2006-10-24  2:49 ` NeilBrown
  3 siblings, 0 replies; 16+ messages in thread
From: NeilBrown @ 2006-10-24  2:49 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs


With this patch, lock requests with have the same soft/hard and
intr/nointr options as the mount that the lock request are for.
Without the patch all lock requests are hard/nointr

WARNING: this is only safe if the client responds to a failed lock
 request by queuing a hard, async unlock request.  I don't know if the
 current client does this.

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

### Diffstat output
 ./fs/lockd/clntproc.c |    8 ++++----
 ./fs/lockd/host.c     |    7 ++++++-
 ./fs/lockd/svc4proc.c |    2 +-
 ./fs/lockd/svclock.c  |    2 +-
 ./fs/lockd/svcproc.c  |    2 +-
 5 files changed, 13 insertions(+), 8 deletions(-)

diff .prev/fs/lockd/clntproc.c ./fs/lockd/clntproc.c
--- .prev/fs/lockd/clntproc.c	2006-10-24 12:25:25.000000000 +1000
+++ ./fs/lockd/clntproc.c	2006-10-24 12:20:39.000000000 +1000
@@ -193,7 +193,7 @@ nlmclnt_proc(struct inode *inode, int cm
 		sigfillset(&current->blocked);	/* Mask all signals */
 		recalc_sigpending();
 
-		call->a_flags = RPC_TASK_ASYNC;
+		call->a_flags |= RPC_TASK_ASYNC;
 	}
 	spin_unlock_irqrestore(&current->sighand->siglock, flags);
 
@@ -305,7 +305,7 @@ nlmclnt_call(struct nlm_rqst *req, u32 p
 		msg.rpc_proc = &clnt->cl_procinfo[proc];
 
 		/* Perform the RPC call. If an error occurs, try again */
-		if ((status = rpc_call_sync(clnt, &msg, 0)) < 0) {
+		if ((status = rpc_call_sync(clnt, &msg, req->a_flags)) < 0) {
 			dprintk("lockd: rpc_call returned error %d\n", -status);
 			switch (status) {
 			case -EPROTONOSUPPORT:
@@ -372,7 +372,7 @@ static int __nlm_async_call(struct nlm_r
 	msg->rpc_proc = &clnt->cl_procinfo[proc];
 
         /* bootstrap and kick off the async RPC call */
-        status = rpc_call_async(clnt, msg, RPC_TASK_ASYNC, tk_ops, req);
+        status = rpc_call_async(clnt, msg, req->a_flags, tk_ops, req);
 	if (status == 0)
 		return 0;
 out_err:
@@ -701,7 +701,7 @@ static int nlmclnt_cancel(struct nlm_hos
 	req = nlm_alloc_call(nlm_get_host(host));
 	if (!req)
 		return -ENOMEM;
-	req->a_flags = RPC_TASK_ASYNC;
+	req->a_flags |= RPC_TASK_ASYNC;
 
 	nlmclnt_setlockargs(req, fl);
 	req->a_args.block = block;

diff .prev/fs/lockd/host.c ./fs/lockd/host.c
--- .prev/fs/lockd/host.c	2006-10-24 12:25:25.000000000 +1000
+++ ./fs/lockd/host.c	2006-10-24 12:21:07.000000000 +1000
@@ -237,9 +237,14 @@ nlm_bind_host(struct nlm_host *host)
 			.authflavor	= RPC_AUTH_UNIX,
 			.flags		= (RPC_CLNT_CREATE_HARDRTRY |
 					   RPC_CLNT_CREATE_NOPING |
+					   RPC_CLNT_CREATE_INTR |
 					   RPC_CLNT_CREATE_AUTOBIND),
 		};
-
+		/* Note: The HARDRTRY and INTR flags are chosen so they
+		 * can be overrided by the flags argument to rpc_call_a?sync.
+		 * rpc_init_task forces 'soft' and 'nointr' from the client
+		 * onto the task.
+		 */
 		clnt = rpc_create(&args);
 		if (!IS_ERR(clnt))
 			host->h_rpcclnt = clnt;

diff .prev/fs/lockd/svc4proc.c ./fs/lockd/svc4proc.c
--- .prev/fs/lockd/svc4proc.c	2006-10-24 12:25:25.000000000 +1000
+++ ./fs/lockd/svc4proc.c	2006-10-24 12:20:39.000000000 +1000
@@ -276,7 +276,7 @@ static __be32 nlm4svc_callback(struct sv
 		return stat;
 	}
 
-	call->a_flags = RPC_TASK_ASYNC;
+	call->a_flags |= RPC_TASK_ASYNC;
 	if (nlm_async_reply(call, proc, &nlm4svc_callback_ops) < 0)
 		return rpc_system_err;
 	return rpc_success;

diff .prev/fs/lockd/svclock.c ./fs/lockd/svclock.c
--- .prev/fs/lockd/svclock.c	2006-10-24 12:25:25.000000000 +1000
+++ ./fs/lockd/svclock.c	2006-10-24 12:20:39.000000000 +1000
@@ -217,7 +217,7 @@ nlmsvc_create_block(struct svc_rqst *rqs
 
 	/* Set up RPC arguments for callback */
 	block->b_call = call;
-	call->a_flags   = RPC_TASK_ASYNC;
+	call->a_flags |= RPC_TASK_ASYNC;
 	call->a_block = block;
 
 	return block;

diff .prev/fs/lockd/svcproc.c ./fs/lockd/svcproc.c
--- .prev/fs/lockd/svcproc.c	2006-10-24 12:25:25.000000000 +1000
+++ ./fs/lockd/svcproc.c	2006-10-24 12:20:39.000000000 +1000
@@ -305,7 +305,7 @@ static __be32 nlmsvc_callback(struct svc
 		return stat;
 	}
 
-	call->a_flags = RPC_TASK_ASYNC;
+	call->a_flags |= RPC_TASK_ASYNC;
 	if (nlm_async_reply(call, proc, &nlmsvc_callback_ops) < 0)
 		return rpc_system_err;
 	return rpc_success;

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-10-24  2:48 ` [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client NeilBrown
@ 2006-10-24 23:05   ` Chuck Lever
  2006-11-15 17:11     ` Trond Myklebust
  2006-11-16  3:27     ` Neil Brown
  0 siblings, 2 replies; 16+ messages in thread
From: Chuck Lever @ 2006-10-24 23:05 UTC (permalink / raw)
  To: NeilBrown; +Cc: Olaf Kirch, Chuck Lever, nfs, Trond Myklebust

Looking at this has forced me to actually think about it  ;-)

Since portmapper is always a child task, I think it is safe and
reasonable to use a "hard,intr" RPC client all the time.  The parent
task will timeout automatically if it is a soft mount, right?  The RPC
client should terminate the child if the parent dies.

In any event, I think the NONPRIVPORT and NO_PING flags should be
managed entirely within pmap_create().  No need to clutter the callers
with that.

On 10/23/06, NeilBrown <neilb@suse.de> wrote:
>
> When an RPC request needs to make a subordinate RPC request to portmap
> to find the port number to communicate with, the 'soft' and 'intr'
> flags should be copied from the original request to the subordinate request
> to ensure consistent handling.
>
> Currently the pmap client defaults to soft,nointr, so while it is
> certain to timeout, it could still be 30 seconds without being
> interruptible.
>
> Note that copying the 'hard' flag down is not essential as if the pmap
> request aborts, the parent request - being hard - will retry it
> indefinitely.  However copying it is more obviously right.
>
> Signed-off-by: Neil Brown <neilb@suse.de>
>
> ### Diffstat output
>  ./net/sunrpc/pmap_clnt.c |   22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff .prev/net/sunrpc/pmap_clnt.c ./net/sunrpc/pmap_clnt.c
> --- .prev/net/sunrpc/pmap_clnt.c        2006-10-24 09:23:13.000000000 +1000
> +++ ./net/sunrpc/pmap_clnt.c    2006-10-24 09:33:08.000000000 +1000
> @@ -34,7 +34,8 @@ struct portmap_args {
>  };
>
>  static struct rpc_procinfo     pmap_procedures[];
> -static struct rpc_clnt *       pmap_create(char *, struct sockaddr_in *, int, int);
> +static struct rpc_clnt *       pmap_create(char *, struct sockaddr_in *,
> +                                           int, unsigned long);
>  static void                    pmap_getport_done(struct rpc_task *, void *);
>  static struct rpc_program      pmap_program;
>
> @@ -93,6 +94,7 @@ void rpc_getport(struct rpc_task *task)
>         struct rpc_clnt *pmap_clnt;
>         struct rpc_task *child;
>         int status;
> +       unsigned long create_flags;
>
>         dprintk("RPC: %4d rpc_getport(%s, %u, %u, %d)\n",
>                         task->tk_pid, clnt->cl_server,
> @@ -125,7 +127,13 @@ void rpc_getport(struct rpc_task *task)
>         map->pm_xprt = xprt_get(xprt);
>
>         rpc_peeraddr(clnt, (struct sockaddr *) &addr, sizeof(addr));
> -       pmap_clnt = pmap_create(clnt->cl_server, &addr, map->pm_prot, 0);
> +       create_flags = RPC_CLNT_CREATE_NONPRIVPORT;
> +       if ( ! RPC_IS_SOFT(task))
> +               create_flags |= RPC_CLNT_CREATE_HARDRTRY;
> +       if ( ! RPC_TASK_UNINTERRUPTIBLE(task))
> +               create_flags |= RPC_CLNT_CREATE_INTR;
> +       pmap_clnt = pmap_create(clnt->cl_server, &addr, map->pm_prot,
> +                               create_flags);
>         status = PTR_ERR(pmap_clnt);
>         if (IS_ERR(pmap_clnt))
>                 goto bailout;
> @@ -178,7 +186,7 @@ int rpc_getport_external(struct sockaddr
>                         NIPQUAD(sin->sin_addr.s_addr), prog, vers, prot);
>
>         sprintf(hostname, "%u.%u.%u.%u", NIPQUAD(sin->sin_addr.s_addr));
> -       pmap_clnt = pmap_create(hostname, sin, prot, 0);
> +       pmap_clnt = pmap_create(hostname, sin, prot, RPC_CLNT_CREATE_NONPRIVPORT);
>         if (IS_ERR(pmap_clnt))
>                 return PTR_ERR(pmap_clnt);
>
> @@ -257,7 +265,7 @@ int rpc_register(u32 prog, u32 vers, int
>         dprintk("RPC: registering (%u, %u, %d, %u) with portmapper.\n",
>                         prog, vers, prot, port);
>
> -       pmap_clnt = pmap_create("localhost", &sin, IPPROTO_UDP, 1);
> +       pmap_clnt = pmap_create("localhost", &sin, IPPROTO_UDP, 0);
>         if (IS_ERR(pmap_clnt)) {
>                 error = PTR_ERR(pmap_clnt);
>                 dprintk("RPC: couldn't create pmap client. Error = %d\n", error);
> @@ -277,7 +285,8 @@ int rpc_register(u32 prog, u32 vers, int
>         return error;
>  }
>
> -static struct rpc_clnt *pmap_create(char *hostname, struct sockaddr_in *srvaddr, int proto, int privileged)
> +static struct rpc_clnt *pmap_create(char *hostname, struct sockaddr_in *srvaddr,
> +                                   int proto, unsigned long create_flags)
>  {
>         struct rpc_create_args args = {
>                 .protocol       = proto,
> @@ -292,8 +301,7 @@ static struct rpc_clnt *pmap_create(char
>         };
>
>         srvaddr->sin_port = htons(RPC_PMAP_PORT);
> -       if (!privileged)
> -               args.flags |= RPC_CLNT_CREATE_NONPRIVPORT;
> +       args.flags |= create_flags;
>         return rpc_create(&args);
>  }
>
>
> -------------------------------------------------------------------------
> 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

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 002 of 4] Make the initial RPC PING interruptible.
  2006-10-24  2:48 ` [PATCH 002 of 4] Make the initial RPC PING interruptible NeilBrown
@ 2006-10-24 23:19   ` Chuck Lever
  2006-11-16  3:22     ` Neil Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2006-10-24 23:19 UTC (permalink / raw)
  To: NeilBrown; +Cc: Olaf Kirch, Chuck Lever, nfs, Trond Myklebust

What's the harm in always using SOFT|INTR for doing pings?

I would think this would be a bit cleaner if you just eliminated the
"flags" parameter of rpc_ping() and calculated the correct task flags
inside rpc_ping().

On 10/23/06, NeilBrown <neilb@suse.de> wrote:
>
> If an RPC client is created with CREATE_INTR and not CREATE_NOPING,
> then the ping should be interruptible.  With tcp the PING can take 30
> seconds to time out and not being able to interrupt that can be
> frustrating.
>
> Signed-off-by: Neil Brown <neilb@suse.de>
>
> ### Diffstat output
>  ./net/sunrpc/clnt.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff .prev/net/sunrpc/clnt.c ./net/sunrpc/clnt.c
> --- .prev/net/sunrpc/clnt.c     2006-10-24 09:41:49.000000000 +1000
> +++ ./net/sunrpc/clnt.c 2006-10-24 09:43:48.000000000 +1000
> @@ -221,7 +221,15 @@ struct rpc_clnt *rpc_create(struct rpc_c
>                 return clnt;
>
>         if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
> -               int err = rpc_ping(clnt, RPC_TASK_SOFT|RPC_TASK_NOINTR);
> +               int tskflags = RPC_TASK_SOFT;
> +               int err;
> +               if ( ! (args->flags & RPC_CLNT_CREATE_INTR))
> +                       tskflags |= RPC_TASK_NOINTR;
> +               /* Note: we keep task_soft even if RPC_CLNT_CREATE_HARDRTRY
> +                * is set.  The whole point of the PING is to check that the
> +                * server is there *now*
> +                */
> +               err = rpc_ping(clnt, tskflags);
>                 if (err != 0) {
>                         rpc_shutdown_client(clnt);
>                         return ERR_PTR(err);
>
> -------------------------------------------------------------------------
> 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

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.
  2006-10-24  2:49 ` [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly NeilBrown
@ 2006-10-24 23:37   ` Chuck Lever
  2006-11-15 17:14   ` Trond Myklebust
  1 sibling, 0 replies; 16+ messages in thread
From: Chuck Lever @ 2006-10-24 23:37 UTC (permalink / raw)
  To: NeilBrown; +Cc: Olaf Kirch, Chuck Lever, nfs, Trond Myklebust

On 10/23/06, NeilBrown <neilb@suse.de> wrote:
>
> Sometimes when an RPC client is created, an initial 'PING' is sent to
> the RPC/NULL procedure to make sure the server is running.
> This is always done as a 'soft' call so a major timeout will be fatal.
>
> We change semantics so that if portmap reports that the service does
> not exist, or if a ECONNREFUSED error is returned we abort early
> rather than persist for the full timeout.
>
> The core mechanism is a new flag RPC_TASK_ACCEPT_NO which tell the rpc
> client to accept a 'no' answer rather than retrying.

This doesn't seem like a per task thing.... It maybe should be a
client option, like AUTOBIND is.  RPC_CREATE_BIND_ONE_SHOT, perhaps?

Is the local statd case the only one where an immediate timeout is desirable?

> This patch also causes lockd clients to *not* do the initial 'ping'
> otherwise a lock request could fail due to a missing server, even on a
> hard mount.

It's a small change, but maybe it's worth splitting this into a
separate patch.  This seems a worthy change, but the other stuff I
still don't quite grok.


> ### Diffstat output
>  ./fs/lockd/host.c              |    1 +
>  ./include/linux/sunrpc/sched.h |    4 ++++
>  ./net/sunrpc/clnt.c            |   14 +++++++++++---
>  ./net/sunrpc/pmap_clnt.c       |    4 +++-
>  4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff .prev/fs/lockd/host.c ./fs/lockd/host.c
> --- .prev/fs/lockd/host.c       2006-10-24 10:09:41.000000000 +1000
> +++ ./fs/lockd/host.c   2006-10-24 11:48:05.000000000 +1000
> @@ -236,6 +236,7 @@ nlm_bind_host(struct nlm_host *host)
>                         .version        = host->h_version,
>                         .authflavor     = RPC_AUTH_UNIX,
>                         .flags          = (RPC_CLNT_CREATE_HARDRTRY |
> +                                          RPC_CLNT_CREATE_NOPING |
>                                            RPC_CLNT_CREATE_AUTOBIND),
>                 };
>
>
> diff .prev/include/linux/sunrpc/sched.h ./include/linux/sunrpc/sched.h
> --- .prev/include/linux/sunrpc/sched.h  2006-10-24 11:35:21.000000000 +1000
> +++ ./include/linux/sunrpc/sched.h      2006-10-24 11:46:25.000000000 +1000
> @@ -133,6 +133,10 @@ struct rpc_call_ops {
>  #define RPC_TASK_KILLED                0x0100          /* task was killed */
>  #define RPC_TASK_SOFT          0x0200          /* Use soft timeouts */
>  #define RPC_TASK_NOINTR                0x0400          /* uninterruptible task */
> +#define RPC_TASK_ACCEPT_NO     0x0800          /* Accept a 'no' such as
> +                                                * -ECONNREFUSED or '0' from
> +                                                * portmap, to be reliable.
> +                                                */
>
>  #define RPC_IS_ASYNC(t)                ((t)->tk_flags & RPC_TASK_ASYNC)
>  #define RPC_IS_SWAPPER(t)      ((t)->tk_flags & RPC_TASK_SWAPPER)
>
> diff .prev/net/sunrpc/clnt.c ./net/sunrpc/clnt.c
> --- .prev/net/sunrpc/clnt.c     2006-10-24 09:43:48.000000000 +1000
> +++ ./net/sunrpc/clnt.c 2006-10-24 11:45:54.000000000 +1000
> @@ -221,7 +221,7 @@ struct rpc_clnt *rpc_create(struct rpc_c
>                 return clnt;
>
>         if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
> -               int tskflags = RPC_TASK_SOFT;
> +               int tskflags = RPC_TASK_SOFT | RPC_TASK_ACCEPT_NO;
>                 int err;
>                 if ( ! (args->flags & RPC_CLNT_CREATE_INTR))
>                         tskflags |= RPC_TASK_NOINTR;
> @@ -868,6 +868,8 @@ call_bind_status(struct rpc_task *task)
>         case -EACCES:
>                 dprintk("RPC: %4d remote rpcbind: RPC program/version unavailable\n",
>                                 task->tk_pid);
> +               if (task->tk_flags & RPC_TASK_ACCEPT_NO)
> +                       break;
>                 rpc_delay(task, 3*HZ);
>                 goto retry_timeout;
>         case -ETIMEDOUT:
> @@ -941,6 +943,8 @@ call_connect_status(struct rpc_task *tas
>         switch (status) {
>         case -ENOTCONN:
>         case -EAGAIN:
> +               if (task->tk_flags & RPC_TASK_ACCEPT_NO)
> +                       break;
>                 task->tk_action = call_bind;
>                 if (!RPC_IS_SOFT(task))
>                         return;
> @@ -1044,8 +1048,12 @@ call_status(struct rpc_task *task)
>                 break;
>         case -ECONNREFUSED:
>         case -ENOTCONN:
> -               rpc_force_rebind(clnt);
> -               task->tk_action = call_bind;
> +               if (task->tk_flags & RPC_TASK_ACCEPT_NO)
> +                       rpc_exit(task, status);
> +               else {
> +                       rpc_force_rebind(clnt);
> +                       task->tk_action = call_bind;
> +               }
>                 break;
>         case -EAGAIN:
>                 task->tk_action = call_transmit;
>
> diff .prev/net/sunrpc/pmap_clnt.c ./net/sunrpc/pmap_clnt.c
> --- .prev/net/sunrpc/pmap_clnt.c        2006-10-24 09:33:08.000000000 +1000
> +++ ./net/sunrpc/pmap_clnt.c    2006-10-24 11:51:32.000000000 +1000
> @@ -139,7 +139,9 @@ void rpc_getport(struct rpc_task *task)
>                 goto bailout;
>
>         status = -EIO;
> -       child = rpc_run_task(pmap_clnt, RPC_TASK_ASYNC, &pmap_getport_ops, map);
> +       child = rpc_run_task(pmap_clnt,
> +                            RPC_TASK_ASYNC | (task->tk_flags & RPC_TASK_ACCEPT_NO),
> +                            &pmap_getport_ops, map);
>         if (IS_ERR(child))
>                 goto bailout;
>         rpc_release_task(child);
>
> -------------------------------------------------------------------------
> 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

-------------------------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-10-24 23:05   ` Chuck Lever
@ 2006-11-15 17:11     ` Trond Myklebust
  2006-11-15 19:38       ` Chuck Lever
  2006-11-16  3:27     ` Neil Brown
  1 sibling, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2006-11-15 17:11 UTC (permalink / raw)
  To: Chuck Lever; +Cc: NeilBrown, Olaf Kirch, Chuck Lever, nfs

On Tue, 2006-10-24 at 16:05 -0700, Chuck Lever wrote:
> Looking at this has forced me to actually think about it  ;-)
> 
> Since portmapper is always a child task, I think it is safe and
> reasonable to use a "hard,intr" RPC client all the time.  The parent
> task will timeout automatically if it is a soft mount, right?  The RPC
> client should terminate the child if the parent dies.

...or to set it to always be soft. The main problem with that might be
nfsroot.

Hmm... Looking at that code, though there is something that bothers me.
Why are we calling pmap_wake_portmap_waiters every time the call to
xprt_test_and_set_binding(xprt)!=0? That looks like a nasty bug.

Trond


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.
  2006-10-24  2:49 ` [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly NeilBrown
  2006-10-24 23:37   ` Chuck Lever
@ 2006-11-15 17:14   ` Trond Myklebust
  2006-11-16  3:20     ` Neil Brown
  1 sibling, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2006-11-15 17:14 UTC (permalink / raw)
  To: NeilBrown; +Cc: Olaf Kirch, Chuck Lever, nfs

On Tue, 2006-10-24 at 12:49 +1000, NeilBrown wrote:
> Sometimes when an RPC client is created, an initial 'PING' is sent to
> the RPC/NULL procedure to make sure the server is running.
> This is always done as a 'soft' call so a major timeout will be fatal.
> 
> We change semantics so that if portmap reports that the service does
> not exist, or if a ECONNREFUSED error is returned we abort early
> rather than persist for the full timeout.

What if the server is in the process of booting? If the RPC layer
doesn't handle that case, then we will have to add a loop to sleep and
wait in the NLM client.

Cheers
  Trond


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-11-15 17:11     ` Trond Myklebust
@ 2006-11-15 19:38       ` Chuck Lever
  2006-11-15 20:31         ` Trond Myklebust
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2006-11-15 19:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: NeilBrown, Olaf Kirch, Chuck Lever, nfs

On 11/15/06, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> On Tue, 2006-10-24 at 16:05 -0700, Chuck Lever wrote:
> > Looking at this has forced me to actually think about it  ;-)
> >
> > Since portmapper is always a child task, I think it is safe and
> > reasonable to use a "hard,intr" RPC client all the time.  The parent
> > task will timeout automatically if it is a soft mount, right?  The RPC
> > client should terminate the child if the parent dies.
>
> ...or to set it to always be soft.

I suppose that would be OK, since call_bind_status will retry
timed-out rpcbind requests for hard mounts.

> Hmm... Looking at that code, though there is something that bothers me.
> Why are we calling pmap_wake_portmap_waiters every time the call to
> xprt_test_and_set_binding(xprt)!=0? That looks like a nasty bug.

Well the idea is to make sure the task isn't left on the binding queue
if pmap_getport can't actually queue an rpcbind request.   I don't
think it should be waking up all waiters, though -- that part is
probably wrong.

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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-11-15 19:38       ` Chuck Lever
@ 2006-11-15 20:31         ` Trond Myklebust
  2006-11-15 21:14           ` Chuck Lever
  0 siblings, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2006-11-15 20:31 UTC (permalink / raw)
  To: Chuck Lever; +Cc: NeilBrown, Olaf Kirch, Chuck Lever, nfs

On Wed, 2006-11-15 at 14:38 -0500, Chuck Lever wrote:
> > Hmm... Looking at that code, though there is something that bothers me.
> > Why are we calling pmap_wake_portmap_waiters every time the call to
> > xprt_test_and_set_binding(xprt)!=0? That looks like a nasty bug.
> 
> Well the idea is to make sure the task isn't left on the binding queue
> if pmap_getport can't actually queue an rpcbind request.   I don't
> think it should be waking up all waiters, though -- that part is
> probably wrong.

It shouldn't be waking anybody up if it just failed because a portmapper
request has already been created. In that case, it should just sleep and
wait for the request to finish.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-11-15 20:31         ` Trond Myklebust
@ 2006-11-15 21:14           ` Chuck Lever
  0 siblings, 0 replies; 16+ messages in thread
From: Chuck Lever @ 2006-11-15 21:14 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: NeilBrown, Olaf Kirch, Chuck Lever, nfs

On 11/15/06, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> On Wed, 2006-11-15 at 14:38 -0500, Chuck Lever wrote:
> > > Hmm... Looking at that code, though there is something that bothers me.
> > > Why are we calling pmap_wake_portmap_waiters every time the call to
> > > xprt_test_and_set_binding(xprt)!=0? That looks like a nasty bug.
> >
> > Well the idea is to make sure the task isn't left on the binding queue
> > if pmap_getport can't actually queue an rpcbind request.   I don't
> > think it should be waking up all waiters, though -- that part is
> > probably wrong.
>
> It shouldn't be waking anybody up if it just failed because a portmapper
> request has already been created. In that case, it should just sleep and
> wait for the request to finish.

OK, agreed.  That logic should just set task->tk_status to -EACESS and return.

Waking up all waiters after bailout_nofree: is still excessive, though.

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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.
  2006-11-15 17:14   ` Trond Myklebust
@ 2006-11-16  3:20     ` Neil Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Neil Brown @ 2006-11-16  3:20 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Olaf Kirch, Chuck Lever, nfs

On Wednesday November 15, trond.myklebust@fys.uio.no wrote:
> On Tue, 2006-10-24 at 12:49 +1000, NeilBrown wrote:
> > Sometimes when an RPC client is created, an initial 'PING' is sent to
> > the RPC/NULL procedure to make sure the server is running.
> > This is always done as a 'soft' call so a major timeout will be fatal.
> > 
> > We change semantics so that if portmap reports that the service does
> > not exist, or if a ECONNREFUSED error is returned we abort early
> > rather than persist for the full timeout.
> 
> What if the server is in the process of booting? If the RPC layer
> doesn't handle that case, then we will have to add a loop to sleep and
> wait in the NLM client.

What is the purpose of the PING?

It seems to me that it is intended to allow a one-time soft-fail.
e.g. it would be used when first mounting a filesystem.  If the server
isn't responding, then fail the mount, even if it is a hard mount.

Once the PING succeeds we assume the server is genuine and all future
requests follow whatever soft/hard/intr rules are given for the mount.

If this is the case, then a PING should fail as soon as there seems to
be a genuine justification for the failure, such as an rpcbind error.

NeilBrown

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 002 of 4] Make the initial RPC PING interruptible.
  2006-10-24 23:19   ` Chuck Lever
@ 2006-11-16  3:22     ` Neil Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Neil Brown @ 2006-11-16  3:22 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Olaf Kirch, Chuck Lever, nfs, Trond Myklebust

On Tuesday October 24, chucklever@gmail.com wrote:
> What's the harm in always using SOFT|INTR for doing pings?
> 
> I would think this would be a bit cleaner if you just eliminated the
> "flags" parameter of rpc_ping() and calculated the correct task flags
> inside rpc_ping().

Yes, that sounds right.

Thanks,

NeilBrown

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client
  2006-10-24 23:05   ` Chuck Lever
  2006-11-15 17:11     ` Trond Myklebust
@ 2006-11-16  3:27     ` Neil Brown
  1 sibling, 0 replies; 16+ messages in thread
From: Neil Brown @ 2006-11-16  3:27 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Olaf Kirch, Chuck Lever, nfs, Trond Myklebust


On Tuesday October 24, chucklever@gmail.com wrote:
> Looking at this has forced me to actually think about it  ;-)

Good :-)

> 
> Since portmapper is always a child task, I think it is safe and
> reasonable to use a "hard,intr" RPC client all the time.  The parent
> task will timeout automatically if it is a soft mount, right?  The RPC
> client should terminate the child if the parent dies.

Maybe.  Can one task timeout while a child task is active?  If it can
then 'hard' would make sense.
I guess 'intr' makes sense to and the parent would just retry, but I
would need to pour over the code again to be sure.

> 
> In any event, I think the NONPRIVPORT and NO_PING flags should be
> managed entirely within pmap_create().  No need to clutter the callers
> with that.

pmap needs a privport when registering and not when doing a lookup, so
I don't think we can get rid of all the flags.

NeilBrown

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2006-11-16  3:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-24  2:48 [PATCH 000 of 4] Introduction Possibly patches for RPC client changes NeilBrown
2006-10-24  2:48 ` [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client NeilBrown
2006-10-24 23:05   ` Chuck Lever
2006-11-15 17:11     ` Trond Myklebust
2006-11-15 19:38       ` Chuck Lever
2006-11-15 20:31         ` Trond Myklebust
2006-11-15 21:14           ` Chuck Lever
2006-11-16  3:27     ` Neil Brown
2006-10-24  2:48 ` [PATCH 002 of 4] Make the initial RPC PING interruptible NeilBrown
2006-10-24 23:19   ` Chuck Lever
2006-11-16  3:22     ` Neil Brown
2006-10-24  2:49 ` [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly NeilBrown
2006-10-24 23:37   ` Chuck Lever
2006-11-15 17:14   ` Trond Myklebust
2006-11-16  3:20     ` Neil Brown
2006-10-24  2:49 ` [PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests NeilBrown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.