[PATCH 000 of 4] Introduction Possibly patches for RPC client changes.

[PATCH 000 of 4] Introduction Possibly patches for RPC client changes.

[NeilBrown]

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

[PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[NeilBrown]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Chuck Lever]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Trond Myklebust]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Chuck Lever]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Trond Myklebust]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Chuck Lever]

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

Re: [PATCH 001 of 4] Copy intr and soft flags to portmap-bind client

[Neil Brown]

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

[PATCH 002 of 4] Make the initial RPC PING interruptible.

[NeilBrown]

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

Re: [PATCH 002 of 4] Make the initial RPC PING interruptible.

[Chuck Lever]

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

Re: [PATCH 002 of 4] Make the initial RPC PING interruptible.

[Neil Brown]

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

[PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.

[NeilBrown]

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

Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.

[Chuck Lever]

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

Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.

[Trond Myklebust]

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

Re: [PATCH 003 of 4] Make RPC 'ping' requests fail more quickly.

[Neil Brown]

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

[PATCH 004 of 4] Inherit soft/intr flags from NFS mount to lockd requests.

[NeilBrown]

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