From ac4e89087f2346fab8a1fa39995d6e9c5c4ba2ca Mon Sep 17 00:00:00 2001 From: mpi Date: Mon, 18 Dec 2017 10:07:55 +0000 Subject: [PATCH] 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. --- sys/kern/uipc_socket.c | 19 +++---------------- sys/miscfs/fifofs/fifo_vnops.c | 14 +++----------- sys/sys/socketvar.h | 5 ++++- 3 files changed, 10 insertions(+), 28 deletions(-) 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); } -- 2.20.1