From: mpi <mpi@openbsd.org>
Date: Mon, 18 Dec 2017 10:07:55 +0000 (+0000)
Subject: Revert grabbing the socket lock in kqueue(2) filters.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ac4e89087f2346fab8a1fa39995d6e9c5c4ba2ca;p=openbsd

Revert grabbing the socket lock in kqueue(2) filters.

This change exposed or created a situation where a CPU started to be
irresponsive while holding the KERNEL_LOCK().  These led to lockups and
even with MP_LOCKDEBUG it was not clear what happened to this CPU.

These situations have been experience by dhill@ with dcrwallet and jcs@
with syncthing.  Both applications are written in Go and do kevent(2)
& networking across multiple threads.
---

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 611cdb88165..3e4423be70f 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.210 2017/12/10 11:31:54 mpi Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.211 2017/12/18 10:07:55 mpi Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -1923,10 +1923,8 @@ int
 filt_soread(struct knote *kn, long hint)
 {
 	struct socket *so = kn->kn_fp->f_data;
-	int s, rv;
+	int rv;
 
-	if (!(hint & NOTE_SUBMIT))
-		s = solock(so);
 	kn->kn_data = so->so_rcv.sb_cc;
 #ifdef SOCKET_SPLICE
 	if (isspliced(so)) {
@@ -1944,8 +1942,6 @@ filt_soread(struct knote *kn, long hint)
 	} else {
 		rv = (kn->kn_data >= so->so_rcv.sb_lowat);
 	}
-	if (!(hint & NOTE_SUBMIT))
-		sounlock(s);
 
 	return rv;
 }
@@ -1966,10 +1962,8 @@ int
 filt_sowrite(struct knote *kn, long hint)
 {
 	struct socket *so = kn->kn_fp->f_data;
-	int s, rv;
+	int rv;
 
-	if (!(hint & NOTE_SUBMIT))
-		s = solock(so);
 	kn->kn_data = sbspace(so, &so->so_snd);
 	if (so->so_state & SS_CANTSENDMORE) {
 		kn->kn_flags |= EV_EOF;
@@ -1985,8 +1979,6 @@ filt_sowrite(struct knote *kn, long hint)
 	} else {
 		rv = (kn->kn_data >= so->so_snd.sb_lowat);
 	}
-	if (!(hint & NOTE_SUBMIT))
-		sounlock(s);
 
 	return (rv);
 }
@@ -1995,13 +1987,8 @@ int
 filt_solisten(struct knote *kn, long hint)
 {
 	struct socket *so = kn->kn_fp->f_data;
-	int s;
 
-	if (!(hint & NOTE_SUBMIT))
-		s = solock(so);
 	kn->kn_data = so->so_qlen;
-	if (!(hint & NOTE_SUBMIT))
-		sounlock(s);
 
 	return (kn->kn_data != 0);
 }
diff --git a/sys/miscfs/fifofs/fifo_vnops.c b/sys/miscfs/fifofs/fifo_vnops.c
index c2eb03a96c3..06c544b4b53 100644
--- a/sys/miscfs/fifofs/fifo_vnops.c
+++ b/sys/miscfs/fifofs/fifo_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: fifo_vnops.c,v 1.60 2017/12/10 11:31:54 mpi Exp $	*/
+/*	$OpenBSD: fifo_vnops.c,v 1.61 2017/12/18 10:07:55 mpi Exp $	*/
 /*	$NetBSD: fifo_vnops.c,v 1.18 1996/03/16 23:52:42 christos Exp $	*/
 
 /*
@@ -548,10 +548,8 @@ int
 filt_fiforead(struct knote *kn, long hint)
 {
 	struct socket *so = (struct socket *)kn->kn_hook;
-	int s, rv;
+	int rv;
 
-	if (!(hint & NOTE_SUBMIT))
-		s = solock(so);
 	kn->kn_data = so->so_rcv.sb_cc;
 	if (so->so_state & SS_CANTRCVMORE) {
 		kn->kn_flags |= EV_EOF;
@@ -560,8 +558,6 @@ filt_fiforead(struct knote *kn, long hint)
 		kn->kn_flags &= ~EV_EOF;
 		rv = (kn->kn_data > 0);
 	}
-	if (!(hint & NOTE_SUBMIT))
-		sounlock(s);
 
 	return (rv);
 }
@@ -580,10 +576,8 @@ int
 filt_fifowrite(struct knote *kn, long hint)
 {
 	struct socket *so = (struct socket *)kn->kn_hook;
-	int s, rv;
+	int rv;
 
-	if (!(hint & NOTE_SUBMIT))
-		s = solock(so);
 	kn->kn_data = sbspace(so, &so->so_snd);
 	if (so->so_state & SS_CANTSENDMORE) {
 		kn->kn_flags |= EV_EOF;
@@ -592,8 +586,6 @@ filt_fifowrite(struct knote *kn, long hint)
 		kn->kn_flags &= ~EV_EOF;
 		rv = (kn->kn_data >= so->so_snd.sb_lowat);
 	}
-	if (!(hint & NOTE_SUBMIT))
-		sounlock(s);
 
 	return (rv);
 }
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index e6dd63562b7..a54fcdcc231 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: socketvar.h,v 1.79 2017/11/23 13:45:46 mpi Exp $	*/
+/*	$OpenBSD: socketvar.h,v 1.80 2017/12/18 10:07:55 mpi Exp $	*/
 /*	$NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $	*/
 
 /*-
@@ -186,7 +186,10 @@ static inline long
 sbspace(struct socket *so, struct sockbuf *sb)
 {
 	KASSERT(sb == &so->so_rcv || sb == &so->so_snd);
+#if 0
+	/* XXXSMP kqueue_scan() calling filt_sowrite() cannot sleep. */
 	soassertlocked(so);
+#endif
 	return lmin(sb->sb_hiwat - sb->sb_cc, sb->sb_mbmax - sb->sb_mbcnt);
 }