[RFC,PATCH 6/14] knfsd: add svc_sock_is_connection

[RFC,PATCH 6/14] knfsd: add svc_sock_is_connection

[Greg Banks]

Add a svc_sock_is_connection() predicate to test whether a struct
svc_sock is a connected socket (currently this means TCP).  Also, fix
ip_map_cached_put() to detect whether the svc_sock is a connection
using the new predicate, instead of reaching into the svc_sock's
socket, because later the NFS/RDMA transport will not have a socket.

Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
Signed-off-by: Peter Leckie <pleckie@melbourne.sgi.com>
---

 include/linux/sunrpc/svcsock.h |    5 +++++
 net/sunrpc/svcauth_unix.c      |    2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Index: linux/include/linux/sunrpc/svcsock.h
===================================================================
--- linux.orig/include/linux/sunrpc/svcsock.h	2007-05-17 01:28:41.131377156 +1000
+++ linux/include/linux/sunrpc/svcsock.h	2007-05-17 01:58:40.829117616 +1000
@@ -95,6 +95,11 @@ struct svc_sock {
 	int			sk_remotelen;	/* length of address */
 };
 
+static inline int svc_sock_is_connection(struct svc_sock *svsk)
+{
+	return (test_bit(SK_TEMP, &svsk->sk_flags));
+}
+
 /*
  * Function prototypes.
  */
Index: linux/net/sunrpc/svcauth_unix.c
===================================================================
--- linux.orig/net/sunrpc/svcauth_unix.c	2007-04-26 13:08:32.000000000 +1000
+++ linux/net/sunrpc/svcauth_unix.c	2007-05-17 01:58:40.865112934 +1000
@@ -411,7 +411,7 @@ ip_map_cached_put(struct svc_rqst *rqstp
 	struct svc_sock *svsk = rqstp->rq_sock;
 
 	spin_lock_bh(&svsk->sk_defer_lock);
-	if (svsk->sk_sock->type == SOCK_STREAM &&
+	if (svc_sock_is_connection(svsk) &&
 	    svsk->sk_info_authunix == NULL) {
 		/* newly cached, keep the reference */
 		svsk->sk_info_authunix = ipm;
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere.  Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 6/14] knfsd: add svc_sock_is_connection

[Neil Brown]

On Thursday May 17, gnb@sgi.com wrote:
>  
> +static inline int svc_sock_is_connection(struct svc_sock *svsk)
> +{
> +	return (test_bit(SK_TEMP, &svsk->sk_flags));
> +}

You seem to do a lot of this:  Pulling a simple bit operation out into
an inline function.
I must confess that I am not a big fan of this.  It makes the code
harder to read.  I keep running in to that sort of thing in the block
layer and buffer code, and it just slows me down: I go hunting to find
out what a function really does, and it turns out to be a simple
bit-op.

In this case, it adds some documentation (it isn't obvious that
testing SK_TEMP is the same as testing if the connection has a stable
remote endpoint), but a comment could fix that.

So I would rather simple change:
> -	if (svsk->sk_sock->type == SOCK_STREAM &&
to
> +	if (test_bit(SK_TEMP, &scsk->sk_flags) && /* is a tcp connection */

or similar.

Ditto the set/clear/test of  SK_CONN, SK_DATA, SK_CLOSE, unless you
have a very good reason.

The svc_sock_get, svc_sock_put I can live with as it is a very widely
used abstraction.  But not the bit ops.

NeilBrown

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs