Merge additional socket buffer locking from rwatson_netperf:

- Lock down low hanging fruit use of sb_flags with socket buffer
  lock.

- Lock down low hanging fruit use of so_state with socket lock.

- Lock down low hanging fruit use of so_options.

- Lock down low-hanging fruit use of sb_lowwat and sb_hiwat with
  socket buffer lock.

- Annotate situations in which we unlock the socket lock and then
  grab the receive socket buffer lock, which are currently actually
  the same lock.  Depending on how we want to play our cards, we
  may want to coallesce these lock uses to reduce overhead.

- Convert a if()->panic() into a KASSERT relating to so_state in
  soaccept().

- Remove a number of splnet()/splx() references.

More complex merging of socket and socket buffer locking to
follow.

5 files changed, 108 insertions, 16 deletions

diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c
index a580b43..986edf7 100644
--- a/sys/kern/sys_socket.c
+++ b/sys/kern/sys_socket.c
@@ -131,21 +131,41 @@ soo_ioctl(fp, cmd, data, active_cred, td)
 	switch (cmd) {
 
 	case FIONBIO:
+		SOCK_LOCK(so);
 		if (*(int *)data)
 			so->so_state |= SS_NBIO;
 		else
 			so->so_state &= ~SS_NBIO;
+		SOCK_UNLOCK(so);
 		return (0);
 
 	case FIOASYNC:
+		/*
+		 * XXXRW: This code separately acquires SOCK_LOCK(so)
+		 * and SOCKBUF_LOCK(&so->so_rcv) even though they are
+		 * the same mutex to avoid introducing the assumption
+		 * that they are the same.
+		 */
 		if (*(int *)data) {
+			SOCK_LOCK(so);
 			so->so_state |= SS_ASYNC;
+			SOCK_UNLOCK(so);
+			SOCKBUF_LOCK(&so->so_rcv);
 			so->so_rcv.sb_flags |= SB_ASYNC;
+			SOCKBUF_UNLOCK(&so->so_rcv);
+			SOCKBUF_LOCK(&so->so_snd);
 			so->so_snd.sb_flags |= SB_ASYNC;
+			SOCKBUF_UNLOCK(&so->so_snd);
 		} else {
+			SOCK_LOCK(so);
 			so->so_state &= ~SS_ASYNC;
+			SOCK_UNLOCK(so);
+			SOCKBUF_LOCK(&so->so_rcv);
 			so->so_rcv.sb_flags &= ~SB_ASYNC;
+			SOCKBUF_UNLOCK(&so->so_rcv);
+			SOCKBUF_LOCK(&so->so_snd);
 			so->so_snd.sb_flags &= ~SB_ASYNC;
+			SOCKBUF_UNLOCK(&so->so_snd);
 		}
 		return (0);
 
diff --git a/sys/kern/uipc_sockbuf.c b/sys/kern/uipc_sockbuf.c
index 9b02fec..c385253 100644
--- a/sys/kern/uipc_sockbuf.c
+++ b/sys/kern/uipc_sockbuf.c
@@ -105,8 +105,10 @@ soisconnecting(so)
 	register struct socket *so;
 {
 
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING);
 	so->so_state |= SS_ISCONNECTING;
+	SOCK_UNLOCK(so);
 }
 
 void
@@ -115,8 +117,10 @@ soisconnected(so)
 {
 	struct socket *head;
 
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING);
 	so->so_state |= SS_ISCONNECTED;
+	SOCK_UNLOCK(so);
 	ACCEPT_LOCK();
 	head = so->so_head;
 	if (head != NULL && (so->so_qstate & SQ_INCOMP)) {
@@ -132,11 +136,13 @@ soisconnected(so)
 			wakeup_one(&head->so_timeo);
 		} else {
 			ACCEPT_UNLOCK();
+			SOCK_LOCK(so);
 			so->so_upcall =
 			    head->so_accf->so_accept_filter->accf_callback;
 			so->so_upcallarg = head->so_accf->so_accept_filter_arg;
 			so->so_rcv.sb_flags |= SB_UPCALL;
 			so->so_options &= ~SO_ACCEPTFILTER;
+			SOCK_UNLOCK(so);
 			so->so_upcall(so, so->so_upcallarg, M_TRYWAIT);
 		}
 		return;
@@ -152,8 +158,15 @@ soisdisconnecting(so)
 	register struct socket *so;
 {
 
+	/*
+	 * XXXRW: This code separately acquires SOCK_LOCK(so) and
+	 * SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
+	 * avoid introducing the assumption  that they are the same.
+	 */
+	SOCK_LOCK(so);
 	so->so_state &= ~SS_ISCONNECTING;
 	so->so_state |= SS_ISDISCONNECTING;
+	SOCK_UNLOCK(so);
 	SOCKBUF_LOCK(&so->so_rcv);
 	so->so_rcv.sb_state |= SBS_CANTRCVMORE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
@@ -170,8 +183,15 @@ soisdisconnected(so)
 	register struct socket *so;
 {
 
+	/*
+	 * XXXRW: This code separately acquires SOCK_LOCK(so) and
+	 * SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
+	 * avoid introducing the assumption  that they are the same.
+	 */
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
 	so->so_state |= SS_ISDISCONNECTED;
+	SOCK_UNLOCK(so);
 	SOCKBUF_LOCK(&so->so_rcv);
 	so->so_rcv.sb_state |= SBS_CANTRCVMORE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
@@ -400,12 +420,16 @@ soreserve(so, sndcc, rcvcc)
 		goto bad;
 	if (sbreserve(&so->so_rcv, rcvcc, so, td) == 0)
 		goto bad2;
+	SOCKBUF_LOCK(&so->so_rcv);
 	if (so->so_rcv.sb_lowat == 0)
 		so->so_rcv.sb_lowat = 1;
+	SOCKBUF_UNLOCK(&so->so_rcv);
+	SOCKBUF_LOCK(&so->so_snd);
 	if (so->so_snd.sb_lowat == 0)
 		so->so_snd.sb_lowat = MCLBYTES;
 	if (so->so_snd.sb_lowat > so->so_snd.sb_hiwat)
 		so->so_snd.sb_lowat = so->so_snd.sb_hiwat;
+	SOCKBUF_UNLOCK(&so->so_snd);
 	return (0);
 bad2:
 	sbrelease(&so->so_snd, so);
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index fb9043c..6606138 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -277,8 +277,11 @@ solisten(so, backlog, td)
 		return (error);
 	}
 	ACCEPT_LOCK();
-	if (TAILQ_EMPTY(&so->so_comp))
+	if (TAILQ_EMPTY(&so->so_comp)) {
+		SOCK_LOCK(so);
 		so->so_options |= SO_ACCEPTCONN;
+		SOCK_UNLOCK(so);
+	}
 	if (backlog < 0 || backlog > somaxconn)
 		backlog = somaxconn;
 	so->so_qlimit = backlog;
@@ -448,14 +451,13 @@ soaccept(so, nam)
 	struct socket *so;
 	struct sockaddr **nam;
 {
-	int s = splnet();
 	int error;
 
-	if ((so->so_state & SS_NOFDREF) == 0)
-		panic("soaccept: !NOFDREF");
+	SOCK_LOCK(so);
+	KASSERT((so->so_state & SS_NOFDREF) != 0, ("soaccept: !NOFDREF"));
 	so->so_state &= ~SS_NOFDREF;
+	SOCK_UNLOCK(so);
 	error = (*so->so_proto->pr_usrreqs->pru_accept)(so, nam);
-	splx(s);
 	return (error);
 }
 
@@ -754,9 +756,11 @@ restart:
 				break;
 			}
 		    } while (space > 0 && atomic);
-		    if (dontroute)
+		    if (dontroute) {
+			    SOCK_LOCK(so);
 			    so->so_options |= SO_DONTROUTE;
-		    s = splnet();				/* XXX */
+			    SOCK_UNLOCK(so);
+		    }
 		    /*
 		     * XXX all the SBS_CANTSENDMORE checks previously
 		     * done could be out of date.  We could have recieved
@@ -1396,11 +1400,13 @@ sosetopt(so, sopt)
 			if (error)
 				goto bad;
 
+			SOCK_LOCK(so);
 			so->so_linger = l.l_linger;
 			if (l.l_onoff)
 				so->so_options |= SO_LINGER;
 			else
 				so->so_options &= ~SO_LINGER;
+			SOCK_UNLOCK(so);
 			break;
 
 		case SO_DEBUG:
@@ -1418,10 +1424,12 @@ sosetopt(so, sopt)
 					    sizeof optval);
 			if (error)
 				goto bad;
+			SOCK_LOCK(so);
 			if (optval)
 				so->so_options |= sopt->sopt_name;
 			else
 				so->so_options &= ~sopt->sopt_name;
+			SOCK_UNLOCK(so);
 			break;
 
 		case SO_SNDBUF:
@@ -1842,12 +1850,16 @@ sopoll(struct socket *so, int events, struct ucred *active_cred,
 		    (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM |
 		     POLLRDBAND)) {
 			selrecord(td, &so->so_rcv.sb_sel);
+			SOCKBUF_LOCK(&so->so_rcv);
 			so->so_rcv.sb_flags |= SB_SEL;
+			SOCKBUF_UNLOCK(&so->so_rcv);
 		}
 
 		if (events & (POLLOUT | POLLWRNORM)) {
 			selrecord(td, &so->so_snd.sb_sel);
+			SOCKBUF_LOCK(&so->so_snd);
 			so->so_snd.sb_flags |= SB_SEL;
+			SOCKBUF_UNLOCK(&so->so_snd);
 		}
 	}
 
@@ -1860,7 +1872,6 @@ soo_kqfilter(struct file *fp, struct knote *kn)
 {
 	struct socket *so = kn->kn_fp->f_data;
 	struct sockbuf *sb;
-	int s;
 
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
@@ -1878,10 +1889,10 @@ soo_kqfilter(struct file *fp, struct knote *kn)
 		return (1);
 	}
 
-	s = splnet();
+	SOCKBUF_LOCK(sb);
 	SLIST_INSERT_HEAD(&sb->sb_sel.si_note, kn, kn_selnext);
 	sb->sb_flags |= SB_KNOTE;
-	splx(s);
+	SOCKBUF_UNLOCK(sb);
 	return (0);
 }
 
@@ -1889,12 +1900,12 @@ static void
 filt_sordetach(struct knote *kn)
 {
 	struct socket *so = kn->kn_fp->f_data;
-	int s = splnet();
 
+	SOCKBUF_LOCK(&so->so_rcv);
 	SLIST_REMOVE(&so->so_rcv.sb_sel.si_note, kn, knote, kn_selnext);
 	if (SLIST_EMPTY(&so->so_rcv.sb_sel.si_note))
 		so->so_rcv.sb_flags &= ~SB_KNOTE;
-	splx(s);
+	SOCKBUF_UNLOCK(&so->so_rcv);
 }
 
 /*ARGSUSED*/
@@ -1922,12 +1933,12 @@ static void
 filt_sowdetach(struct knote *kn)
 {
 	struct socket *so = kn->kn_fp->f_data;
-	int s = splnet();
 
+	SOCKBUF_LOCK(&so->so_snd);
 	SLIST_REMOVE(&so->so_snd.sb_sel.si_note, kn, knote, kn_selnext);
 	if (SLIST_EMPTY(&so->so_snd.sb_sel.si_note))
 		so->so_snd.sb_flags &= ~SB_KNOTE;
-	splx(s);
+	SOCKBUF_UNLOCK(&so->so_snd);
 }
 
 /*ARGSUSED*/
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index 9b02fec..c385253 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -105,8 +105,10 @@ soisconnecting(so)
 	register struct socket *so;
 {
 
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTED|SS_ISDISCONNECTING);
 	so->so_state |= SS_ISCONNECTING;
+	SOCK_UNLOCK(so);
 }
 
 void
@@ -115,8 +117,10 @@ soisconnected(so)
 {
 	struct socket *head;
 
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING);
 	so->so_state |= SS_ISCONNECTED;
+	SOCK_UNLOCK(so);
 	ACCEPT_LOCK();
 	head = so->so_head;
 	if (head != NULL && (so->so_qstate & SQ_INCOMP)) {
@@ -132,11 +136,13 @@ soisconnected(so)
 			wakeup_one(&head->so_timeo);
 		} else {
 			ACCEPT_UNLOCK();
+			SOCK_LOCK(so);
 			so->so_upcall =
 			    head->so_accf->so_accept_filter->accf_callback;
 			so->so_upcallarg = head->so_accf->so_accept_filter_arg;
 			so->so_rcv.sb_flags |= SB_UPCALL;
 			so->so_options &= ~SO_ACCEPTFILTER;
+			SOCK_UNLOCK(so);
 			so->so_upcall(so, so->so_upcallarg, M_TRYWAIT);
 		}
 		return;
@@ -152,8 +158,15 @@ soisdisconnecting(so)
 	register struct socket *so;
 {
 
+	/*
+	 * XXXRW: This code separately acquires SOCK_LOCK(so) and
+	 * SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
+	 * avoid introducing the assumption  that they are the same.
+	 */
+	SOCK_LOCK(so);
 	so->so_state &= ~SS_ISCONNECTING;
 	so->so_state |= SS_ISDISCONNECTING;
+	SOCK_UNLOCK(so);
 	SOCKBUF_LOCK(&so->so_rcv);
 	so->so_rcv.sb_state |= SBS_CANTRCVMORE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
@@ -170,8 +183,15 @@ soisdisconnected(so)
 	register struct socket *so;
 {
 
+	/*
+	 * XXXRW: This code separately acquires SOCK_LOCK(so) and
+	 * SOCKBUF_LOCK(&so->so_rcv) even though they are the same mutex to
+	 * avoid introducing the assumption  that they are the same.
+	 */
+	SOCK_LOCK(so);
 	so->so_state &= ~(SS_ISCONNECTING|SS_ISCONNECTED|SS_ISDISCONNECTING);
 	so->so_state |= SS_ISDISCONNECTED;
+	SOCK_UNLOCK(so);
 	SOCKBUF_LOCK(&so->so_rcv);
 	so->so_rcv.sb_state |= SBS_CANTRCVMORE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
@@ -400,12 +420,16 @@ soreserve(so, sndcc, rcvcc)
 		goto bad;
 	if (sbreserve(&so->so_rcv, rcvcc, so, td) == 0)
 		goto bad2;
+	SOCKBUF_LOCK(&so->so_rcv);
 	if (so->so_rcv.sb_lowat == 0)
 		so->so_rcv.sb_lowat = 1;
+	SOCKBUF_UNLOCK(&so->so_rcv);
+	SOCKBUF_LOCK(&so->so_snd);
 	if (so->so_snd.sb_lowat == 0)
 		so->so_snd.sb_lowat = MCLBYTES;
 	if (so->so_snd.sb_lowat > so->so_snd.sb_hiwat)
 		so->so_snd.sb_lowat = so->so_snd.sb_hiwat;
+	SOCKBUF_UNLOCK(&so->so_snd);
 	return (0);
 bad2:
 	sbrelease(&so->so_snd, so);
diff --git a/sys/kern/vfs_aio.c b/sys/kern/vfs_aio.c
index 3978203..a56a54f 100644
--- a/sys/kern/vfs_aio.c
+++ b/sys/kern/vfs_aio.c
@@ -566,8 +566,12 @@ aio_proc_rundown(void *arg, struct proc *p)
 			so = fp->f_data;
 			TAILQ_REMOVE(&so->so_aiojobq, aiocbe, list);
 			if (TAILQ_EMPTY(&so->so_aiojobq)) {
+				SOCKBUF_LOCK(&so->so_snd);
 				so->so_snd.sb_flags &= ~SB_AIO;
+				SOCKBUF_UNLOCK(&so->so_snd);
+				SOCKBUF_LOCK(&so->so_rcv);
 				so->so_rcv.sb_flags &= ~SB_AIO;
+				SOCKBUF_UNLOCK(&so->so_rcv);
 			}
 		}
 		TAILQ_REMOVE(&ki->kaio_sockqueue, aiocbe, plist);
@@ -1231,10 +1235,14 @@ aio_swake_cb(struct socket *so, struct sockbuf *sb)
 
 	if (sb == &so->so_snd) {
 		opcode = LIO_WRITE;
+		SOCKBUF_LOCK(&so->so_snd);
 		so->so_snd.sb_flags &= ~SB_AIO;
+		SOCKBUF_UNLOCK(&so->so_snd);
 	} else {
 		opcode = LIO_READ;
+		SOCKBUF_LOCK(&so->so_rcv);
 		so->so_rcv.sb_flags &= ~SB_AIO;
+		SOCKBUF_UNLOCK(&so->so_rcv);
 	}
 
 	for (cb = TAILQ_FIRST(&so->so_aiojobq); cb; cb = cbn) {
@@ -1443,10 +1451,15 @@ no_kqueue:
 		    LIO_WRITE) && (!sowriteable(so)))) {
 			TAILQ_INSERT_TAIL(&so->so_aiojobq, aiocbe, list);
 			TAILQ_INSERT_TAIL(&ki->kaio_sockqueue, aiocbe, plist);
-			if (opcode == LIO_READ)
+			if (opcode == LIO_READ) {
+				SOCKBUF_LOCK(&so->so_rcv);
 				so->so_rcv.sb_flags |= SB_AIO;
-			else
+				SOCKBUF_UNLOCK(&so->so_rcv);
+			} else {
+				SOCKBUF_LOCK(&so->so_snd);
 				so->so_snd.sb_flags |= SB_AIO;
+				SOCKBUF_UNLOCK(&so->so_snd);
+			}
 			aiocbe->jobstate = JOBST_JOBQGLOBAL; /* XXX */
 			ki->kaio_queue_count++;
 			num_queue_count++;