Rework socket upcalls to close some races with setup/teardown of upcalls.

- Each socket upcall is now invoked with the appropriate socket buffer
  locked.  It is not permissible to call soisconnected() with this lock
  held; however, so socket upcalls now return an integer value.  The two
  possible values are SU_OK and SU_ISCONNECTED.  If an upcall returns
  SU_ISCONNECTED, then the soisconnected() will be invoked on the
  socket after the socket buffer lock is dropped.
- A new API is provided for setting and clearing socket upcalls.  The
  API consists of soupcall_set() and soupcall_clear().
- To simplify locking, each socket buffer now has a separate upcall.
- When a socket upcall returns SU_ISCONNECTED, the upcall is cleared from
  the receive socket buffer automatically.  Note that a SO_SND upcall
  should never return SU_ISCONNECTED.
- All this means that accept filters should now return SU_ISCONNECTED
  instead of calling soisconnected() directly.  They also no longer need
  to explicitly clear the upcall on the new socket.
- The HTTP accept filter still uses soupcall_set() to manage its internal
  state machine, but other accept filters no longer have any explicit
  knowlege of socket upcall internals aside from their return value.
- The various RPC client upcalls currently drop the socket buffer lock
  while invoking soreceive() as a temporary band-aid.  The plan for
  the future is to add a new flag to allow soreceive() to be called with
  the socket buffer locked.
- The AIO callback for socket I/O is now also invoked with the socket
  buffer locked.  Previously sowakeup() would drop the socket buffer
  lock only to call aio_swake() which immediately re-acquired the socket
  buffer lock for the duration of the function call.

Discussed with:	rwatson, rmacklem

1 files changed, 19 insertions, 16 deletions

diff --git a/sys/nfsclient/nfs_socket.c b/sys/nfsclient/nfs_socket.c
index 64951fa..1ae31a5 100644
--- a/sys/nfsclient/nfs_socket.c
+++ b/sys/nfsclient/nfs_socket.c
@@ -122,8 +122,8 @@ static int	nfs_realign(struct mbuf **pm, int hsiz);
 static int	nfs_reply(struct nfsreq *);
 static void	nfs_softterm(struct nfsreq *rep);
 static int	nfs_reconnect(struct nfsreq *rep);
-static void nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag);
-static void nfs_clnt_udp_soupcall(struct socket *so, void *arg, int waitflag);
+static int	nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag);
+static int	nfs_clnt_udp_soupcall(struct socket *so, void *arg, int waitflag);
 
 extern struct mtx nfs_reqq_mtx;
 
@@ -457,12 +457,10 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		goto bad;
 	SOCKBUF_LOCK(&so->so_rcv);
 	so->so_rcv.sb_flags |= SB_NOINTR;
- 	so->so_upcallarg = (caddr_t)nmp;
  	if (so->so_type == SOCK_STREAM)
- 		so->so_upcall = nfs_clnt_tcp_soupcall;
- 	else	
- 		so->so_upcall = nfs_clnt_udp_soupcall;
- 	so->so_rcv.sb_flags |= SB_UPCALL;
+		soupcall_set(so, SO_RCV, nfs_clnt_tcp_soupcall, nmp);
+ 	else
+		soupcall_set(so, SO_RCV, nfs_clnt_udp_soupcall, nmp);
 	SOCKBUF_UNLOCK(&so->so_rcv);
 	SOCKBUF_LOCK(&so->so_snd);
 	so->so_snd.sb_flags |= SB_NOINTR;
@@ -600,9 +598,7 @@ nfs_disconnect(struct nfsmount *nmp)
 		nmp->nm_so = NULL;
 		mtx_unlock(&nmp->nm_mtx);
 		SOCKBUF_LOCK(&so->so_rcv);
- 		so->so_upcallarg = NULL;
- 		so->so_upcall = NULL;
- 		so->so_rcv.sb_flags &= ~SB_UPCALL;
+		soupcall_clear(so, SO_RCV);
 		SOCKBUF_UNLOCK(&so->so_rcv);
 		soshutdown(so, SHUT_WR);
 		soclose(so);
@@ -811,7 +807,7 @@ tryagain:
  * XXX TO DO
  * Make nfs_realign() non-blocking. Also make nfsm_dissect() nonblocking.
  */
-static void
+static int
 nfs_clnt_match_xid(struct socket *so, 
 		   struct nfsmount *nmp, 
 		   struct mbuf *mrep)
@@ -928,11 +924,10 @@ nfstcp_readable(struct socket *so, int bytes)
 {
 	int retval;
 	
-	SOCKBUF_LOCK(&so->so_rcv);
+	SOCKBUF_LOCK_ASSERT(&so->so_rcv);
 	retval = (so->so_rcv.sb_cc >= (bytes) ||
 		  (so->so_rcv.sb_state & SBS_CANTRCVMORE) ||
 		  so->so_error);
-	SOCKBUF_UNLOCK(&so->so_rcv);
 	return (retval);
 }
 
@@ -969,7 +964,7 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 	mtx_lock(&nmp->nm_mtx);
 	if (nmp->nm_nfstcpstate.flags & NFS_TCP_FORCE_RECONNECT) {
 		mtx_unlock(&nmp->nm_mtx);		
-		return;
+		return (SU_OK);
 	} else			
 		mtx_unlock(&nmp->nm_mtx);
 	auio.uio_td = curthread;
@@ -983,8 +978,9 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 			mtx_unlock(&nmp->nm_mtx);
 			if (!nfstcp_marker_readable(so)) {
 				/* Marker is not readable */
-				return;
+				return (SU_OK);
 			}
+			SOCKBUF_UNLOCK(&so->so_rcv);
 			auio.uio_resid = sizeof(u_int32_t);
 			auio.uio_iov = NULL;
 			auio.uio_iovcnt = 0;
@@ -992,6 +988,7 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 			rcvflg = (MSG_DONTWAIT | MSG_SOCALLBCK);
 			error =  soreceive(so, (struct sockaddr **)0, &auio,
 			    &mp, (struct mbuf **)0, &rcvflg);
+			SOCKBUF_LOCK(&so->so_rcv);
 			/*
 			 * We've already tested that the socket is readable. 2 cases 
 			 * here, we either read 0 bytes (client closed connection), 
@@ -1052,8 +1049,9 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 			mtx_unlock(&nmp->nm_mtx);
 			if (!nfstcp_readable(so, nmp->nm_nfstcpstate.rpcresid)) {
 				/* All data not readable */
-				return;
+				return (SU_OK);
 			}
+			SOCKBUF_UNLOCK(&so->so_rcv);
 			auio.uio_resid = nmp->nm_nfstcpstate.rpcresid;
 			auio.uio_iov = NULL;
 			auio.uio_iovcnt = 0;
@@ -1061,6 +1059,7 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 			rcvflg = (MSG_DONTWAIT | MSG_SOCALLBCK);
 			error =  soreceive(so, (struct sockaddr **)0, &auio,
 			    &mp, (struct mbuf **)0, &rcvflg);
+			SOCKBUF_LOCK(&so->so_rcv);
 			if (error || auio.uio_resid > 0) {
 				if (error && error != ECONNRESET) {
 					log(LOG_ERR, 
@@ -1083,6 +1082,7 @@ nfs_clnt_tcp_soupcall(struct socket *so, void *arg, int waitflag)
 
 mark_reconnect:
 	nfs_mark_for_reconnect(nmp);
+	return (SU_OK);
 }
 
 static void
@@ -1094,6 +1094,7 @@ nfs_clnt_udp_soupcall(struct socket *so, void *arg, int waitflag)
 	struct mbuf *control = NULL;
 	int error, rcvflag;
 
+	SOCKBUF_UNLOCK(&so->so_rcv);
 	auio.uio_resid = 1000000;
 	auio.uio_td = curthread;
 	rcvflag = MSG_DONTWAIT;
@@ -1106,6 +1107,8 @@ nfs_clnt_udp_soupcall(struct socket *so, void *arg, int waitflag)
 		if (mp)
 			nfs_clnt_match_xid(so, nmp, mp);
 	} while (mp && !error);
+	SOCKBUF_LOCK(&so->so_rcv);
+	return (SU_OK);
 }
 
 /*