[RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Tom Tucker]

This patchset is a series of independent, incremental patches against
the previously posted svc transport switch patchset. I believe each
of these patches can be taken independently. They are presented as a 
series soley because they reflect what I believe to be the collective 
consensus of the svc transport switch review.

The main, non-trivial changes are as follows:

- Transport class data is no longer copied to each transport instance.

- A svc_xprt_find service has been added to allow a service to query
  for transport instances based on class name, address family and port.

-- 
Signed-off-by: Tom Tucker <tom@opengridcomputing.com>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC, PATCH 1/7] svc: Don't copy xprt_class data into svc_xprt instance

[Tom Tucker]

Previously, data from the transport class was copied into the transport
structure. Make the xpt_xpo a ptr to the ops structure from the transport
class, and access the max_payload value through the existing xpt_class 
pointer. 

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 include/linux/sunrpc/svc_xprt.h |    3 +--
 net/sunrpc/svc.c                |    4 ++--
 net/sunrpc/svc_xprt.c           |   19 +++++++++----------
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 53c8891..89bd5ff 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -34,8 +34,7 @@ struct svc_xprt_class {
 
 struct svc_xprt {
 	struct svc_xprt_class	*xpt_class;
-	struct svc_xprt_ops	xpt_ops;
-	u32			xpt_max_payload;
+	struct svc_xprt_ops	*xpt_ops;
 	struct kref		xpt_ref;
 	struct list_head	xpt_list;
 	struct list_head	xpt_ready;
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 440ea59..e1d4c03 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -812,7 +812,7 @@ svc_process(struct svc_rqst *rqstp)
 	rqstp->rq_splice_ok = 1;
 
 	/* Setup reply header */
-	rqstp->rq_xprt->xpt_ops.xpo_prep_reply_hdr(rqstp);
+	rqstp->rq_xprt->xpt_ops->xpo_prep_reply_hdr(rqstp);
 
 	rqstp->rq_xid = svc_getu32(argv);
 	svc_putu32(resv, rqstp->rq_xid);
@@ -1029,7 +1029,7 @@ err_bad:
  */
 u32 svc_max_payload(const struct svc_rqst *rqstp)
 {
-	int max = rqstp->rq_xprt->xpt_max_payload;
+	int max = rqstp->rq_xprt->xpt_class->xcl_max_payload;
 
 	if (rqstp->rq_server->sv_max_payload < max)
 		max = rqstp->rq_server->sv_max_payload;
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index e55904f..16ffc73 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -127,7 +127,7 @@ static inline void svc_xprt_free(struct 
 		container_of(kref, struct svc_xprt, xpt_ref);
 	struct module *owner = xprt->xpt_class->xcl_owner;
 	BUG_ON(atomic_read(&kref->refcount));
-	xprt->xpt_ops.xpo_free(xprt);
+	xprt->xpt_ops->xpo_free(xprt);
 	if (test_bit(XPT_CACHE_AUTH, &xprt->xpt_flags)
 	    && xprt->xpt_auth_cache != NULL)
 		svcauth_unix_info_release(xprt->xpt_auth_cache);
@@ -148,8 +148,7 @@ void svc_xprt_init(struct svc_xprt_class
 		   struct svc_serv *serv)
 {
 	xpt->xpt_class = xcl;
-	xpt->xpt_ops = *xcl->xcl_ops;
-	xpt->xpt_max_payload = xcl->xcl_max_payload;
+	xpt->xpt_ops = xcl->xcl_ops;
 	kref_init(&xpt->xpt_ref);
 	xpt->xpt_server = serv;
 	INIT_LIST_HEAD(&xpt->xpt_list);
@@ -281,7 +280,7 @@ svc_xprt_enqueue(struct svc_xprt *xprt)
 		goto process;
 
 	/* Check if we have space to reply to a request */
-	if (!xprt->xpt_ops.xpo_has_wspace(xprt)) {
+	if (!xprt->xpt_ops->xpo_has_wspace(xprt)) {
 		/* Don't enqueue while not enough space for reply */
 		dprintk("svc: no write space, transport %p  not enqueued\n", xprt);
 		xprt->xpt_pool = NULL;
@@ -382,7 +381,7 @@ svc_xprt_release(struct svc_rqst *rqstp)
 {
 	struct svc_xprt	*xprt = rqstp->rq_xprt;
 
-	rqstp->rq_xprt->xpt_ops.xpo_release(rqstp);
+	rqstp->rq_xprt->xpt_ops->xpo_release(rqstp);
 
 	svc_free_res_pages(rqstp);
 	rqstp->rq_res.page_len = 0;
@@ -604,7 +603,7 @@ svc_recv(struct svc_rqst *rqstp, long ti
 		svc_delete_xprt(xprt);
 	} else if (test_bit(XPT_LISTENER, &xprt->xpt_flags)) {
 		struct svc_xprt *newxpt;
-		newxpt = xprt->xpt_ops.xpo_accept(xprt);
+		newxpt = xprt->xpt_ops->xpo_accept(xprt);
 		if (newxpt) {
 			svc_xprt_received(newxpt);
 			/*
@@ -636,7 +635,7 @@ svc_recv(struct svc_rqst *rqstp, long ti
 			svc_xprt_received(xprt);
 			len = svc_deferred_recv(rqstp);
 		} else
-			len = xprt->xpt_ops.xpo_recvfrom(rqstp);
+			len = xprt->xpt_ops->xpo_recvfrom(rqstp);
 		svc_copy_addr(rqstp, xprt);
 		dprintk("svc: got len=%d\n", len);
 	}
@@ -684,7 +683,7 @@ svc_send(struct svc_rqst *rqstp)
 	}
 
 	/* release the receive skb before sending the reply */
-	rqstp->rq_xprt->xpt_ops.xpo_release(rqstp);
+	rqstp->rq_xprt->xpt_ops->xpo_release(rqstp);
 
 	/* calculate over-all length */
 	xb = & rqstp->rq_res;
@@ -697,7 +696,7 @@ svc_send(struct svc_rqst *rqstp)
 	if (test_bit(XPT_DEAD, &xprt->xpt_flags))
 		len = -ENOTCONN;
 	else
-		len = xprt->xpt_ops.xpo_sendto(rqstp);
+		len = xprt->xpt_ops->xpo_sendto(rqstp);
 	mutex_unlock(&xprt->xpt_mutex);
 	svc_xprt_release(rqstp);
 
@@ -772,7 +771,7 @@ svc_delete_xprt(struct svc_xprt *xprt)
 
 	serv = xprt->xpt_server;
 
-	xprt->xpt_ops.xpo_detach(xprt);
+	xprt->xpt_ops->xpo_detach(xprt);
 
 	spin_lock_bh(&serv->sv_lock);
 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC,PATCH 2/7] svc: Rename xpo_release to xpo_release_rqst

[Tom Tucker]

The xpo_release function removes resources that the transport has 
associated with a particular request. To avoid confusion with the 
xpo_detach and xpo_free functions, rename this function xpo_release_rqst.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 include/linux/sunrpc/svc_xprt.h |    2 +-
 net/sunrpc/svc_xprt.c           |    4 ++--
 net/sunrpc/svcsock.c            |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 89bd5ff..a1a7d15 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -19,7 +19,7 @@ struct svc_xprt_ops {
 	int		(*xpo_recvfrom)(struct svc_rqst *);
 	void		(*xpo_prep_reply_hdr)(struct svc_rqst *);
 	int		(*xpo_sendto)(struct svc_rqst *);
-	void		(*xpo_release)(struct svc_rqst *);
+	void		(*xpo_release_rqst)(struct svc_rqst *);
 	void		(*xpo_detach)(struct svc_xprt *);
 	void		(*xpo_free)(struct svc_xprt *);
 };
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 16ffc73..807795f 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -381,7 +381,7 @@ svc_xprt_release(struct svc_rqst *rqstp)
 {
 	struct svc_xprt	*xprt = rqstp->rq_xprt;
 
-	rqstp->rq_xprt->xpt_ops->xpo_release(rqstp);
+	rqstp->rq_xprt->xpt_ops->xpo_release_rqst(rqstp);
 
 	svc_free_res_pages(rqstp);
 	rqstp->rq_res.page_len = 0;
@@ -683,7 +683,7 @@ svc_send(struct svc_rqst *rqstp)
 	}
 
 	/* release the receive skb before sending the reply */
-	rqstp->rq_xprt->xpt_ops->xpo_release(rqstp);
+	rqstp->rq_xprt->xpt_ops->xpo_release_rqst(rqstp);
 
 	/* calculate over-all length */
 	xb = & rqstp->rq_res;
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index e170866..f967c23 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -665,7 +665,7 @@ static struct svc_xprt_ops svc_udp_ops =
 	.xpo_create = svc_udp_create,
 	.xpo_recvfrom = svc_udp_recvfrom,
 	.xpo_sendto = svc_udp_sendto,
-	.xpo_release = svc_release_skb,
+	.xpo_release_rqst = svc_release_skb,
 	.xpo_detach = svc_sock_detach,
 	.xpo_free = svc_sock_free,
 	.xpo_prep_reply_hdr = svc_udp_prep_reply_hdr,
@@ -1091,7 +1091,7 @@ static struct svc_xprt_ops svc_tcp_ops =
 	.xpo_create = svc_tcp_create,
 	.xpo_recvfrom = svc_tcp_recvfrom,
 	.xpo_sendto = svc_tcp_sendto,
-	.xpo_release = svc_release_skb,
+	.xpo_release_rqst = svc_release_skb,
 	.xpo_detach = svc_sock_detach,
 	.xpo_free = svc_sock_free,
 	.xpo_prep_reply_hdr = svc_tcp_prep_reply_hdr,

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC, PATCH 3/7] svc: Move setting of XPT_LISTENER bit to svc_tcp_init

[Tom Tucker]

Move the setting of the XPT_LISTENER bit to svc_tcp_init where 
the remaining TCP transport initializiation is done.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 net/sunrpc/svcsock.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index f967c23..07a6f42 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1128,6 +1128,7 @@ svc_tcp_init(struct svc_sock *svsk, stru
 	set_bit(XPT_CACHE_AUTH, &svsk->sk_xprt.xpt_flags);
 	if (sk->sk_state == TCP_LISTEN) {
 		dprintk("setting up TCP socket for listening\n");
+		set_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags);
 		sk->sk_data_ready = svc_tcp_listen_data_ready;
 		set_bit(XPT_CONN, &svsk->sk_xprt.xpt_flags);
 	} else {
@@ -1253,8 +1254,6 @@ int svc_addsock(struct svc_serv *serv,
 			svc_xprt_received(&svsk->sk_xprt);
 			err = 0;
 		}
-		if (so->sk->sk_protocol == IPPROTO_TCP)
-			set_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags);
 		clear_bit(XPT_TEMP, &svsk->sk_xprt.xpt_flags);
 		spin_lock_bh(&serv->sv_lock);
 		list_add(&svsk->sk_xprt.xpt_list, &serv->sv_permsocks);
@@ -1311,8 +1310,6 @@ svc_create_socket(struct svc_serv *serv,
 	}
 
 	if ((svsk = svc_setup_socket(serv, sock, &error, flags)) != NULL) {
-		if (protocol == IPPROTO_TCP)
-			set_bit(XPT_LISTENER, &svsk->sk_xprt.xpt_flags);
 		svc_xprt_received(&svsk->sk_xprt);
 		return (struct svc_xprt *)svsk;
 	}

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC, PATCH 4/7] svc: Add a sockaddr length argument to the xpo_create function.

[Tom Tucker]

The xpo_create function doesn't currently accept a sockaddr length.
Add this as a parameter to be consistent with other kernel interfaces
taking a sockaddr.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 include/linux/sunrpc/svc_xprt.h |    2 +-
 net/sunrpc/svc_xprt.c           |    4 +++-
 net/sunrpc/svcsock.c            |   10 ++++------
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index a1a7d15..2f45327 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -12,7 +12,7 @@ #include <linux/module.h>
 
 struct svc_xprt_ops {
 	struct svc_xprt	*(*xpo_create)(struct svc_serv *,
-				       struct sockaddr *,
+				       struct sockaddr *, int,
 				       int);
 	struct svc_xprt	*(*xpo_accept)(struct svc_xprt *);
 	int		(*xpo_has_wspace)(struct svc_xprt *);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 807795f..76036af 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -178,7 +178,9 @@ int svc_create_xprt(struct svc_serv *ser
 				struct svc_xprt *newxprt;
 				ret = 0;
 				newxprt = xcl->xcl_ops->xpo_create
-					(serv, (struct sockaddr *)&sin, flags);
+					(serv,
+					 (struct sockaddr *)&sin, sizeof(sin),
+					 flags);
 				if (IS_ERR(newxprt)) {
 					module_put(xcl->xcl_owner);
 					ret = PTR_ERR(newxprt);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 07a6f42..ec834a7 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -655,10 +655,9 @@ svc_udp_accept(struct svc_xprt *xprt)
 }
 
 static struct svc_xprt *
-svc_udp_create(struct svc_serv *serv, struct sockaddr *sa, int flags)
+svc_udp_create(struct svc_serv *serv, struct sockaddr *sa, int alen, int flags)
 {
-	return svc_create_socket(serv, IPPROTO_UDP, sa,
-				 sizeof(struct sockaddr_in), flags);
+	return svc_create_socket(serv, IPPROTO_UDP, sa, alen, flags);
 }
 
 static struct svc_xprt_ops svc_udp_ops = {
@@ -1081,10 +1080,9 @@ svc_tcp_has_wspace(struct svc_xprt *xprt
 }
 
 static struct svc_xprt *
-svc_tcp_create(struct svc_serv *serv, struct sockaddr *sa, int flags)
+svc_tcp_create(struct svc_serv *serv, struct sockaddr *sa, int alen, int flags)
 {
-	return svc_create_socket(serv, IPPROTO_TCP, sa,
-				 sizeof(struct sockaddr_in), flags);
+	return svc_create_socket(serv, IPPROTO_TCP, sa, alen, flags);
 }
 
 static struct svc_xprt_ops svc_tcp_ops = {

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC, PATCH 5/7] svc: Remove extraneous debug svc_send printk

[Tom Tucker]

The svc_send function has a debug check and kern info printk 
for a null xprt pointers in the rqstp structure. Remove the 
printk.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 net/sunrpc/svc_xprt.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 76036af..323977a 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -678,11 +678,8 @@ svc_send(struct svc_rqst *rqstp)
 	int		len;
 	struct xdr_buf	*xb;
 
-	if ((xprt = rqstp->rq_xprt) == NULL) {
-		printk(KERN_WARNING "NULL transport pointer in %s:%d\n",
-				__FILE__, __LINE__);
+	if ((xprt = rqstp->rq_xprt) == NULL)
 		return -EFAULT;
-	}
 
 	/* release the receive skb before sending the reply */
 	rqstp->rq_xprt->xpt_ops->xpo_release_rqst(rqstp);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC, PATCH 6/7] svc: Add svc API that queries for a transport instance

[Tom Tucker]

Add a new svc function that allows a service to query whether a 
transport instance has already been created. This is used in lockd 
to determine whether or not a transport needs to be created when
a lockd instance is brought up. 

Specifying 0 for the address family or port is effectively a wild-card,
and will result in matching the first transport in the service's list
that has a matching class name.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 fs/lockd/svc.c                  |   16 ++--------------
 include/linux/sunrpc/svc_xprt.h |    2 ++
 net/sunrpc/svc_xprt.c           |   30 ++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index a8e79a9..470af01 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -219,18 +219,6 @@ lockd(struct svc_rqst *rqstp)
 	module_put_and_exit(0);
 }
 
-static int find_xprt(struct svc_serv *serv, char *proto)
-{
-	struct svc_xprt *xprt;
-	int found = 0;
-	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list)
-		if (strcmp(xprt->xpt_class->xcl_name, proto) == 0) {
-			found = 1;
-			break;
-		}
-	return found;
-}
-
 /*
  * Make any sockets that are needed but not present.
  * If nlm_udpport or nlm_tcpport were set as module
@@ -242,11 +230,11 @@ static int make_socks(struct svc_serv *s
 	int err = 0;
 
 	if (proto == IPPROTO_UDP || nlm_udpport)
-		if (!find_xprt(serv, "udp"))
+		if (!svc_find_xprt(serv, "udp", 0, 0))
 			err = svc_create_xprt(serv, "udp", nlm_udpport,
 					      SVC_SOCK_DEFAULTS);
 	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport))
-		if (!find_xprt(serv, "tcp"))
+		if (!svc_find_xprt(serv, "tcp", 0, 0))
 			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
 					      SVC_SOCK_DEFAULTS);
 
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 2f45327..5dba9a5 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -82,4 +82,6 @@ static inline void svc_xprt_get(struct s
 void	svc_delete_xprt(struct svc_xprt *xprt);
 void	svc_close_xprt(struct svc_xprt *xprt);
 int	svc_print_xprts(char *buf, int maxlen);
+struct svc_xprt *
+svc_find_xprt(struct svc_serv *serv, char *xprt_class, int af, int port);
 #endif /* SUNRPC_SVC_XPRT_H */
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 323977a..4841c4b 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -950,3 +950,33 @@ static struct svc_deferred_req *svc_defe
 	spin_unlock(&xprt->xpt_lock);
 	return dr;
 }
+
+/*
+ * Return the transport instance pointer for the endpoint accepting
+ * connections/peer traffic from the specified transport class,
+ * address family and port.
+ *
+ * Specifying 0 for the address family or port is a wildcard and will
+ * match any transport with the firt transport with the same class
+ * name active for the service.
+ */
+struct svc_xprt *svc_find_xprt(struct svc_serv *serv, char *xcl_name,
+			       int af, int port)
+{
+	struct svc_xprt *xprt = NULL;
+	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
+		struct sockaddr_in *sin;
+		if (strcmp(xprt->xpt_class->xcl_name, xcl_name))
+			continue;
+		sin = (struct sockaddr_in *)&xprt->xpt_local;
+		if (af && sin->sin_family != af)
+			continue;
+
+		if (port && (sin->sin_port != port))
+			continue;
+
+		break;
+	}
+	return xprt;
+}
+EXPORT_SYMBOL_GPL(svc_find_xprt);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

[RFC,PATCH 7/7] svc: Place type on same line for new API

[Tom Tucker]

The convention is place the type name on the same line as the function
name. The inline directive was also removed to allow the compiler to
elect whether or not to inline.

Signed-off-by: Tom Tucker <tom@opengridcomputing.com>
---

 net/sunrpc/svc_xprt.c |   52 +++++++++++++++++--------------------------------
 1 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 4841c4b..080ecf5 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -121,7 +121,7 @@ int svc_print_xprts(char *buf, int maxle
 	return len;
 }
 
-static inline void svc_xprt_free(struct kref *kref)
+static void svc_xprt_free(struct kref *kref)
 {
 	struct svc_xprt *xprt =
 		container_of(kref, struct svc_xprt, xpt_ref);
@@ -209,8 +209,7 @@ EXPORT_SYMBOL_GPL(svc_create_xprt);
  * use as many different threads as we need, and the rest don't pollute
  * the cache.
  */
-static inline void
-svc_thread_enqueue(struct svc_pool *pool, struct svc_rqst *rqstp)
+static void svc_thread_enqueue(struct svc_pool *pool, struct svc_rqst *rqstp)
 {
 	list_add(&rqstp->rq_list, &pool->sp_threads);
 }
@@ -218,8 +217,7 @@ svc_thread_enqueue(struct svc_pool *pool
 /*
  * Dequeue an nfsd thread.  Must have pool->sp_lock held.
  */
-static inline void
-svc_thread_dequeue(struct svc_pool *pool, struct svc_rqst *rqstp)
+static void svc_thread_dequeue(struct svc_pool *pool, struct svc_rqst *rqstp)
 {
 	list_del(&rqstp->rq_list);
 }
@@ -229,8 +227,7 @@ svc_thread_dequeue(struct svc_pool *pool
  * processes, wake 'em up.
  *
  */
-void
-svc_xprt_enqueue(struct svc_xprt *xprt)
+void svc_xprt_enqueue(struct svc_xprt *xprt)
 {
 	struct svc_serv	*serv = xprt->xpt_server;
 	struct svc_pool *pool;
@@ -322,8 +319,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_enqueue);
 /*
  * Dequeue the first transport.  Must be called with the pool->sp_lock held.
  */
-static inline struct svc_xprt *
-svc_xprt_dequeue(struct svc_pool *pool)
+static struct svc_xprt *svc_xprt_dequeue(struct svc_pool *pool)
 {
 	struct svc_xprt	*xprt;
 
@@ -346,8 +342,7 @@ svc_xprt_dequeue(struct svc_pool *pool)
  * Note: XPT_DATA only gets cleared when a read-attempt finds
  * no (or insufficient) data.
  */
-void
-svc_xprt_received(struct svc_xprt *xprt)
+void svc_xprt_received(struct svc_xprt *xprt)
 {
 	xprt->xpt_pool = NULL;
 	clear_bit(XPT_BUSY, &xprt->xpt_flags);
@@ -378,8 +373,7 @@ void svc_reserve(struct svc_rqst *rqstp,
 	}
 }
 
-static void
-svc_xprt_release(struct svc_rqst *rqstp)
+static void svc_xprt_release(struct svc_rqst *rqstp)
 {
 	struct svc_xprt	*xprt = rqstp->rq_xprt;
 
@@ -411,8 +405,7 @@ svc_xprt_release(struct svc_rqst *rqstp)
  * This really only makes sense for services like lockd
  * which have exactly one thread anyway.
  */
-void
-svc_wake_up(struct svc_serv *serv)
+void svc_wake_up(struct svc_serv *serv)
 {
 	struct svc_rqst	*rqstp;
 	unsigned int i;
@@ -437,8 +430,7 @@ svc_wake_up(struct svc_serv *serv)
 	}
 }
 
-static void
-svc_check_conn_limits(struct svc_serv *serv)
+static void svc_check_conn_limits(struct svc_serv *serv)
 {
 	char	buf[RPC_MAX_ADDRBUFLEN];
 
@@ -485,7 +477,7 @@ svc_check_conn_limits(struct svc_serv *s
 	}
 }
 
-static inline void svc_copy_addr(struct svc_rqst *rqstp, struct svc_xprt *xprt)
+static void svc_copy_addr(struct svc_rqst *rqstp, struct svc_xprt *xprt)
 {
 	struct sockaddr *sin;
 
@@ -513,8 +505,7 @@ static inline void svc_copy_addr(struct 
  * organised not to touch any cachelines in the shared svc_serv
  * structure, only cachelines in the local svc_pool.
  */
-int
-svc_recv(struct svc_rqst *rqstp, long timeout)
+int svc_recv(struct svc_rqst *rqstp, long timeout)
 {
 	struct svc_xprt		*xprt = NULL;
 	struct svc_serv		*serv = rqstp->rq_server;
@@ -661,8 +652,7 @@ svc_recv(struct svc_rqst *rqstp, long ti
 /*
  * Drop request
  */
-void
-svc_drop(struct svc_rqst *rqstp)
+void svc_drop(struct svc_rqst *rqstp)
 {
 	dprintk("svc: xprt %p dropped request\n", rqstp->rq_xprt);
 	svc_xprt_release(rqstp);
@@ -671,8 +661,7 @@ svc_drop(struct svc_rqst *rqstp)
 /*
  * Return reply to client.
  */
-int
-svc_send(struct svc_rqst *rqstp)
+int svc_send(struct svc_rqst *rqstp)
 {
 	struct svc_xprt	*xprt;
 	int		len;
@@ -708,8 +697,7 @@ svc_send(struct svc_rqst *rqstp)
  * Timer function to close old temporary transports, using
  * a mark-and-sweep algorithm.
  */
-static void
-svc_age_temp_xprts(unsigned long closure)
+static void svc_age_temp_xprts(unsigned long closure)
 {
 	struct svc_serv *serv = (struct svc_serv *)closure;
 	struct svc_xprt *xprt;
@@ -761,8 +749,7 @@ svc_age_temp_xprts(unsigned long closure
 /*
  * Remove a dead transport
  */
-void
-svc_delete_xprt(struct svc_xprt *xprt)
+void svc_delete_xprt(struct svc_xprt *xprt)
 {
 	struct svc_serv	*serv;
 
@@ -871,8 +858,7 @@ static void svc_revisit(struct cache_def
  * This code can only handle requests that consist of an xprt-header
  * and rpc-header.
  */
-static struct cache_deferred_req *
-svc_defer(struct cache_req *req)
+static struct cache_deferred_req *svc_defer(struct cache_req *req)
 {
 	struct svc_rqst *rqstp = container_of(req, struct svc_rqst, rq_chandle);
 	struct svc_deferred_req *dr;
@@ -971,12 +957,10 @@ struct svc_xprt *svc_find_xprt(struct sv
 		sin = (struct sockaddr_in *)&xprt->xpt_local;
 		if (af && sin->sin_family != af)
 			continue;
-
 		if (port && (sin->sin_port != port))
 			continue;
-
-		break;
+		return xprt;
 	}
-	return xprt;
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(svc_find_xprt);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 7/7] svc: Place type on same line for new API

[Aaron Wiebe]

New to list here, so I'll apologize in advance if this has already
been discussed...

On 10/9/07, Tom Tucker <tom@opengridcomputing.com> wrote:
>
> The convention is place the type name on the same line as the function
> name. The inline directive was also removed to allow the compiler to
> elect whether or not to inline.

The two versions of gcc that I checked do not attempt to decide
whether or not to inline without -O3.  Has the kernel tree started
using -O3?

-Aaron

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Tom Tucker]

On Tue, 2007-10-09 at 10:35 -0500, Tom Tucker wrote:
> This patchset is a series of independent, incremental patches against
> the previously posted svc transport switch patchset. I believe each
> of these patches can be taken independently. They are presented as a 
> series soley because they reflect what I believe to be the collective 
> consensus of the svc transport switch review.

I realize the above may imply I'm done. I still have to post the rdma
transport driver and a deferral processing optimization.

Tom

> 
> The main, non-trivial changes are as follows:
> 
> - Transport class data is no longer copied to each transport instance.
> 
> - A svc_xprt_find service has been added to allow a service to query
>   for transport instances based on class name, address family and port.
> 


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Greg Banks]

G'day,

Compendium reply.

On Tue, Oct 09, 2007 at 10:35:39AM -0500, Tom Tucker wrote:
> This patchset is a series of independent, incremental patches against
> the previously posted svc transport switch patchset. I believe each
> of these patches can be taken independently. They are presented as a 
> series soley because they reflect what I believe to be the collective 
> consensus of the svc transport switch review.
> 
> The main, non-trivial changes are as follows:
> 
> - Transport class data is no longer copied to each transport instance.
> 
> - A svc_xprt_find service has been added to allow a service to query
>   for transport instances based on class name, address family and port.
> 

> Subject: [RFC,PATCH 1/7] svc: Don't copy xprt_class data into svc_xprt instance

ok

> Subject: [RFC,PATCH 2/7] svc: Rename xpo_release to xpo_release_rqst

ok

> Subject: [RFC,PATCH 3/7] svc: Move setting of XPT_LISTENER bit to svc_tcp_init

ok

> Subject: [RFC,PATCH 4/7] svc: Add a sockaddr length argument to the xpo_create function.

ok

> Subject: [RFC,PATCH 5/7] svc: Remove extraneous debug svc_send printk

ok

> Subject: [RFC,PATCH 6/7] svc: Add svc API that queries for a transport instance

> @@ -242,11 +230,11 @@ static int make_socks(struct svc_serv *s
>         int err = 0;
>                                                                                                                                           
>         if (proto == IPPROTO_UDP || nlm_udpport)
> -               if (!find_xprt(serv, "udp"))
> +               if (!svc_find_xprt(serv, "udp", 0, 0))
>                         err = svc_create_xprt(serv, "udp", nlm_udpport,
>                                               SVC_SOCK_DEFAULTS);
>         if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport))
> -               if (!find_xprt(serv, "tcp"))
> +               if (!svc_find_xprt(serv, "tcp", 0, 0))
>                         err = svc_create_xprt(serv, "tcp", nlm_tcpport,
>                                               SVC_SOCK_DEFAULTS);

You could use AF_UNSPEC as the "wildcard" value for address family.

> +struct svc_xprt *svc_find_xprt(struct svc_serv *serv, char *xcl_name,
> +                              int af, int port)
> +{
> +       struct svc_xprt *xprt = NULL;
> +       list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {

Why have address family and port?  Do you need them?

Also, if this is a generic, exported API, it should be taking
serv->sv_lock to protect traversal of the sv_permsocks list.

> + * Specifying 0 for the address family or port is a wildcard and will
> + * match any transport with the firt transport with the same class
> + * name active for the service.
> [...]
> +               struct sockaddr_in *sin;
> +               if (strcmp(xprt->xpt_class->xcl_name, xcl_name))
> +                       continue;
> +               sin = (struct sockaddr_in *)&xprt->xpt_local;
> +               if (af && sin->sin_family != af)
> +                       continue;
> +
> +               if (port && (sin->sin_port != port))
> +                       continue;

So what are the intended semantics of svc_find_xprt(,,0,2049) ?
When called like that, it will reach into xprt->xpt_local assuming
it's a sockaddr_in and match the passed port number against whatever
it finds at bytes 2-3.

Perhaps this function could be pruned back to only do what it needs
to do right now, which is match on transport name?


> Subject: [RFC,PATCH 7/7] svc: Place type on same line for new API

ok

Greg.
-- 
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: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Tom Tucker]

On Wed, 2007-10-10 at 12:24 +1000, Greg Banks wrote:
> G'day,
> 
> Compendium reply.
> 
> On Tue, Oct 09, 2007 at 10:35:39AM -0500, Tom Tucker wrote:
> > This patchset is a series of independent, incremental patches against
> > the previously posted svc transport switch patchset. I believe each
> > of these patches can be taken independently. They are presented as a 
> > series soley because they reflect what I believe to be the collective 
> > consensus of the svc transport switch review.
> > 
> > The main, non-trivial changes are as follows:
> > 
> > - Transport class data is no longer copied to each transport instance.
> > 
> > - A svc_xprt_find service has been added to allow a service to query
> >   for transport instances based on class name, address family and port.
> > 
> 
> > Subject: [RFC,PATCH 1/7] svc: Don't copy xprt_class data into svc_xprt instance
> 
> ok
> 
> > Subject: [RFC,PATCH 2/7] svc: Rename xpo_release to xpo_release_rqst
> 
> ok
> 
> > Subject: [RFC,PATCH 3/7] svc: Move setting of XPT_LISTENER bit to svc_tcp_init
> 
> ok
> 
> > Subject: [RFC,PATCH 4/7] svc: Add a sockaddr length argument to the xpo_create function.
> 
> ok
> 
> > Subject: [RFC,PATCH 5/7] svc: Remove extraneous debug svc_send printk
> 
> ok
> 
> > Subject: [RFC,PATCH 6/7] svc: Add svc API that queries for a transport instance
> 
> > @@ -242,11 +230,11 @@ static int make_socks(struct svc_serv *s
> >         int err = 0;
> >                                                                                                                                           
> >         if (proto == IPPROTO_UDP || nlm_udpport)
> > -               if (!find_xprt(serv, "udp"))
> > +               if (!svc_find_xprt(serv, "udp", 0, 0))
> >                         err = svc_create_xprt(serv, "udp", nlm_udpport,
> >                                               SVC_SOCK_DEFAULTS);
> >         if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport))
> > -               if (!find_xprt(serv, "tcp"))
> > +               if (!svc_find_xprt(serv, "tcp", 0, 0))
> >                         err = svc_create_xprt(serv, "tcp", nlm_tcpport,
> >                                               SVC_SOCK_DEFAULTS);
> 
> You could use AF_UNSPEC as the "wildcard" value for address family.

Yes, this is clearly preferred. Good suggestion..

> 
> > +struct svc_xprt *svc_find_xprt(struct svc_serv *serv, char *xcl_name,
> > +                              int af, int port)
> > +{
> > +       struct svc_xprt *xprt = NULL;
> > +       list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> 
> Why have address family and port?  Do you need them?

Well, I think not based on the current user 'lockd'. But the additional
logic is trivial and it allows services to discriminate between AF_INET,
AF_INET6, port this, port that, etc.... when searching for existing
transports.

> 
> Also, if this is a generic, exported API, it should be taking
> serv->sv_lock to protect traversal of the sv_permsocks list.

Good point. Thanks,
> 
> > + * Specifying 0 for the address family or port is a wildcard and will
> > + * match any transport with the firt transport with the same class
> > + * name active for the service.
> > [...]
> > +               struct sockaddr_in *sin;
> > +               if (strcmp(xprt->xpt_class->xcl_name, xcl_name))
> > +                       continue;
> > +               sin = (struct sockaddr_in *)&xprt->xpt_local;
> > +               if (af && sin->sin_family != af)
> > +                       continue;
> > +
> > +               if (port && (sin->sin_port != port))
> > +                       continue;
> 
> So what are the intended semantics of svc_find_xprt(,,0,2049) ?
> When called like that, it will reach into xprt->xpt_local assuming
> it's a sockaddr_in and match the passed port number against whatever
> it finds at bytes 2-3.
> 
> Perhaps this function could be pruned back to only do what it needs
> to do right now, which is match on transport name?
> 
> 
> > Subject: [RFC,PATCH 7/7] svc: Place type on same line for new API
> 
> ok
> 
> Greg.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Greg Banks]

On Tue, Oct 09, 2007 at 09:39:46PM -0500, Tom Tucker wrote:
> On Wed, 2007-10-10 at 12:24 +1000, Greg Banks wrote:
> > 
> > Why have address family and port?  Do you need them?
> 
> Well, I think not based on the current user 'lockd'. But the additional
> logic is trivial and it allows services to discriminate between AF_INET,
> AF_INET6, port this, port that, etc.... when searching for existing
> transports.

Is it still trivial when implemented correctly?

Greg.
-- 
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: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Tom Tucker]

On Wed, 2007-10-10 at 12:24 +1000, Greg Banks wrote:
> G'day,
> 
> Compendium reply.
> 
> On Tue, Oct 09, 2007 at 10:35:39AM -0500, Tom Tucker wrote:

> So what are the intended semantics of svc_find_xprt(,,0,2049) ?
> When called like that, it will reach into xprt->xpt_local assuming
> it's a sockaddr_in and match the passed port number against whatever
> it finds at bytes 2-3.

Sorry, I missed this question. My intent was that the filters were
hierarchical: class, af, port.

AF_UNSPEC, notwithstanding, a class of <null> never matches anything.

> 
> Perhaps this function could be pruned back to only do what it needs
> to do right now, which is match on transport name?
> 

Yes, it could.

> 
> > Subject: [RFC,PATCH 7/7] svc: Place type on same line for new API
> 
> ok
> 
> Greg.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [RFC,PATCH 0/7] svc: Incremental svc patchset cleanup

[Greg Banks]

On Tue, Oct 09, 2007 at 10:32:05PM -0500, Tom Tucker wrote:
> On Wed, 2007-10-10 at 12:24 +1000, Greg Banks wrote:
> > G'day,
> > 
> > Compendium reply.
> > 
> > On Tue, Oct 09, 2007 at 10:35:39AM -0500, Tom Tucker wrote:
> 
> > So what are the intended semantics of svc_find_xprt(,,0,2049) ?
> > When called like that, it will reach into xprt->xpt_local assuming
> > it's a sockaddr_in and match the passed port number against whatever
> > it finds at bytes 2-3.
> 
> Sorry, I missed this question. My intent was that the filters were
> hierarchical: class, af, port.

Ah.  Perhaps the comment should say that, and the code enforce it.

Greg.
-- 
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: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs