svn commit: r193436 - head/sys/rpc

[Rick Macklem]

Author: rmacklem
Date: Thu Jun  4 14:13:06 2009
New Revision: 193436
URL: http://svn.freebsd.org/changeset/base/193436

Log:
  Fix two races in the server side krpc w.r.t upcalls:
    Add a flag so that soupcall_clear() is only called once to cancel
    an upcall.
    Move the test for xprt_registered in the upcall down to after the
    mtx_lock() of the pool mutex, to catch the case where it is
    unregistered while the upcall is waiting for the mutex.
  Also, move the mtx_destroy() of the pool mutex to after SVC_RELEASE(),
  so that it isn't destroyed before the upcalls are disabled.
  
  Reviewed by:	dfr, jhb
  Tested by:	pho
  Approved by:	kib (mentor)

Modified:
  head/sys/rpc/svc.c
  head/sys/rpc/svc.h
  head/sys/rpc/svc_vc.c

Modified: head/sys/rpc/svc.c
==============================================================================
--- head/sys/rpc/svc.c	Thu Jun  4 12:27:57 2009	(r193435)
+++ head/sys/rpc/svc.c	Thu Jun  4 14:13:06 2009	(r193436)
@@ -175,12 +175,12 @@ svcpool_destroy(SVCPOOL *pool)
 		mtx_lock(&pool->sp_lock);
 	}
 
-	mtx_destroy(&pool->sp_lock);
-
 	TAILQ_FOREACH_SAFE(xprt, &cleanup, xp_link, nxprt) {
 		SVC_RELEASE(xprt);
 	}
 
+	mtx_destroy(&pool->sp_lock);
+
 	if (pool->sp_rcache)
 		replay_freecache(pool->sp_rcache);
 
@@ -353,15 +353,16 @@ xprt_active(SVCXPRT *xprt)
 {
 	SVCPOOL *pool = xprt->xp_pool;
 
+	mtx_lock(&pool->sp_lock);
+
 	if (!xprt->xp_registered) {
 		/*
 		 * Race with xprt_unregister - we lose.
 		 */
+		mtx_unlock(&pool->sp_lock);
 		return;
 	}
 
-	mtx_lock(&pool->sp_lock);
-
 	if (!xprt->xp_active) {
 		TAILQ_INSERT_TAIL(&pool->sp_active, xprt, xp_alink);
 		xprt->xp_active = TRUE;

Modified: head/sys/rpc/svc.h
==============================================================================
--- head/sys/rpc/svc.h	Thu Jun  4 12:27:57 2009	(r193435)
+++ head/sys/rpc/svc.h	Thu Jun  4 14:13:06 2009	(r193436)
@@ -166,6 +166,7 @@ typedef struct __rpc_svcxprt {
 	int		xp_idletimeout; /* idle time before closing */
 	time_t		xp_lastactive;	/* time of last RPC */
 	u_int64_t	xp_sockref;	/* set by nfsv4 to identify socket */
+	int		xp_upcallset;	/* socket upcall is set up */
 #else
 	int		xp_fd;
 	u_short		xp_port;	 /* associated port number */

Modified: head/sys/rpc/svc_vc.c
==============================================================================
--- head/sys/rpc/svc_vc.c	Thu Jun  4 12:27:57 2009	(r193435)
+++ head/sys/rpc/svc_vc.c	Thu Jun  4 14:13:06 2009	(r193436)
@@ -160,6 +160,7 @@ svc_vc_create(SVCPOOL *pool, struct sock
 	solisten(so, SOMAXCONN, curthread);
 
 	SOCKBUF_LOCK(&so->so_rcv);
+	xprt->xp_upcallset = 1;
 	soupcall_set(so, SO_RCV, svc_vc_soupcall, xprt);
 	SOCKBUF_UNLOCK(&so->so_rcv);
 
@@ -234,6 +235,7 @@ svc_vc_create_conn(SVCPOOL *pool, struct
 	xprt_register(xprt);
 
 	SOCKBUF_LOCK(&so->so_rcv);
+	xprt->xp_upcallset = 1;
 	soupcall_set(so, SO_RCV, svc_vc_soupcall, xprt);
 	SOCKBUF_UNLOCK(&so->so_rcv);
 
@@ -352,7 +354,10 @@ svc_vc_rendezvous_recv(SVCXPRT *xprt, st
 
 	if (error) {
 		SOCKBUF_LOCK(&xprt->xp_socket->so_rcv);
-		soupcall_clear(xprt->xp_socket, SO_RCV);
+		if (xprt->xp_upcallset) {
+			xprt->xp_upcallset = 0;
+			soupcall_clear(xprt->xp_socket, SO_RCV);
+		}
 		SOCKBUF_UNLOCK(&xprt->xp_socket->so_rcv);
 		xprt_inactive(xprt);
 		sx_xunlock(&xprt->xp_lock);
@@ -397,7 +402,10 @@ static void
 svc_vc_destroy_common(SVCXPRT *xprt)
 {
 	SOCKBUF_LOCK(&xprt->xp_socket->so_rcv);
-	soupcall_clear(xprt->xp_socket, SO_RCV);
+	if (xprt->xp_upcallset) {
+		xprt->xp_upcallset = 0;
+		soupcall_clear(xprt->xp_socket, SO_RCV);
+	}
 	SOCKBUF_UNLOCK(&xprt->xp_socket->so_rcv);
 
 	sx_destroy(&xprt->xp_lock);
@@ -632,7 +640,10 @@ svc_vc_recv(SVCXPRT *xprt, struct rpc_ms
 
 		if (error) {
 			SOCKBUF_LOCK(&xprt->xp_socket->so_rcv);
-			soupcall_clear(xprt->xp_socket, SO_RCV);
+			if (xprt->xp_upcallset) {
+				xprt->xp_upcallset = 0;
+				soupcall_clear(xprt->xp_socket, SO_RCV);
+			}
 			SOCKBUF_UNLOCK(&xprt->xp_socket->so_rcv);
 			xprt_inactive(xprt);
 			cd->strm_stat = XPRT_DIED;