From b11de491e7642ff96b344b5e0fd0a31b38d30d87 Mon Sep 17 00:00:00 2001 From: mvs Date: Wed, 10 Apr 2024 12:04:41 +0000 Subject: [PATCH] Remove `head' socket re-locking in sonewconn(). uipc_attach() releases solock() because it should be taken after `unp_gc_lock' rwlock(9) which protects the `unp_link' list. For this reason, the listening `head' socket should be unlocked too while sonewconn() calls uipc_attach(). This could be reworked because now `so_rcv' sockbuf relies on `sb_mtx' mutex(9). The last one `unp_link' foreach loop within unp_gc() discards sockets previously marked as UNP_GCDEAD. These sockets are not accessed from the userland. The only exception is the sosend() threads of connected sending peers, but they only sbappend*() mbuf(9) to `so_rcv'. So it's enough to unlink mbuf(9) chain with `sb_mtx' held and discard lockless. Please note, the existing SS_NEWCONN_WAIT logic was never used because the listening unix(4) socket protected from concurrent unp_detach() by vnode(9) lock, however `head' re-locked all times. ok bluhm --- sys/kern/uipc_socket.c | 12 ++---------- sys/kern/uipc_socket2.c | 38 +++++--------------------------------- sys/kern/uipc_usrreq.c | 27 ++++++++++++++++----------- sys/sys/socketvar.h | 7 ++----- 4 files changed, 25 insertions(+), 59 deletions(-) diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index 8c33b9defd6..e5f4c1c9c19 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uipc_socket.c,v 1.327 2024/04/02 14:23:15 claudio Exp $ */ +/* $OpenBSD: uipc_socket.c,v 1.328 2024/04/10 12:04:41 mvs Exp $ */ /* $NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $ */ /* @@ -65,6 +65,7 @@ void sotask(void *); void soreaper(void *); void soput(void *); int somove(struct socket *, int); +void sorflush(struct socket *); void filt_sordetach(struct knote *kn); int filt_soread(struct knote *kn, long hint); @@ -414,15 +415,6 @@ drop: if (so->so_options & SO_ACCEPTCONN) { int persocket = solock_persocket(so); - if (persocket) { - /* Wait concurrent sonewconn() threads. */ - while (so->so_newconn > 0) { - so->so_state |= SS_NEWCONN_WAIT; - sosleep_nsec(so, &so->so_newconn, PSOCK, - "newcon", INFSLP); - } - } - while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) { if (persocket) solock(so2); diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c index 4fb48191b8d..9c52cf7b42a 100644 --- a/sys/kern/uipc_socket2.c +++ b/sys/kern/uipc_socket2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uipc_socket2.c,v 1.147 2024/03/31 13:50:00 mvs Exp $ */ +/* $OpenBSD: uipc_socket2.c,v 1.148 2024/04/10 12:04:41 mvs Exp $ */ /* $NetBSD: uipc_socket2.c,v 1.11 1996/02/04 02:17:55 christos Exp $ */ /* @@ -179,7 +179,7 @@ sonewconn(struct socket *head, int connstatus, int wait) { struct socket *so; int persocket = solock_persocket(head); - int error; + int soqueue = connstatus ? 1 : 0; /* * XXXSMP as long as `so' and `head' share the same lock, we @@ -232,41 +232,13 @@ sonewconn(struct socket *head, int connstatus, int wait) sigio_copy(&so->so_sigio, &head->so_sigio); - soqinsque(head, so, 0); - - /* - * We need to unlock `head' because PCB layer could release - * solock() to enforce desired lock order. - */ - if (persocket) { - head->so_newconn++; - sounlock(head); - } - - error = pru_attach(so, 0, wait); - - if (persocket) { - sounlock(so); - solock(head); - solock(so); - - if ((head->so_newconn--) == 0) { - if ((head->so_state & SS_NEWCONN_WAIT) != 0) { - head->so_state &= ~SS_NEWCONN_WAIT; - wakeup(&head->so_newconn); - } - } - } - - if (error) { - soqremque(so, 0); + soqinsque(head, so, soqueue); + if (pru_attach(so, 0, wait) != 0) { + soqremque(so, soqueue); goto fail; } - if (connstatus) { so->so_state |= connstatus; - soqremque(so, 0); - soqinsque(head, so, 1); sorwakeup(head); wakeup(&head->so_timeo); } diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 17db0cd4d79..f506397f767 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uipc_usrreq.c,v 1.203 2024/03/26 09:46:47 mvs Exp $ */ +/* $OpenBSD: uipc_usrreq.c,v 1.204 2024/04/10 12:04:41 mvs Exp $ */ /* $NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $ */ /* @@ -293,14 +293,10 @@ uipc_attach(struct socket *so, int proto, int wait) so->so_pcb = unp; getnanotime(&unp->unp_ctime); - /* - * Enforce `unp_gc_lock' -> `solock()' lock order. - */ - sounlock(so); rw_enter_write(&unp_gc_lock); LIST_INSERT_HEAD(&unp_head, unp, unp_link); rw_exit_write(&unp_gc_lock); - solock(so); + return (0); } @@ -753,7 +749,6 @@ unp_detach(struct unpcb *unp) unp->unp_vnode = NULL; /* - * Enforce `unp_gc_lock' -> `solock()' lock order. * Enforce `i_lock' -> `solock()' lock order. */ sounlock(so); @@ -1443,16 +1438,26 @@ unp_gc(void *arg __unused) if (nunref) { LIST_FOREACH(unp, &unp_head, unp_link) { if (unp->unp_gcflags & UNP_GCDEAD) { + struct sockbuf *sb = &unp->unp_socket->so_rcv; + struct mbuf *m; + /* * This socket could still be connected * and if so it's `so_rcv' is still * accessible by concurrent PRU_SEND * thread. */ - so = unp->unp_socket; - solock(so); - sorflush(so); - sounlock(so); + + mtx_enter(&sb->sb_mtx); + m = sb->sb_mb; + memset(&sb->sb_startzero, 0, + (caddr_t)&sb->sb_endzero - + (caddr_t)&sb->sb_startzero); + sb->sb_timeo_nsecs = INFSLP; + mtx_leave(&sb->sb_mtx); + + unp_scan(m, unp_discard); + m_purge(m); } } } diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h index 0169c6273d6..cca3780c0bb 100644 --- a/sys/sys/socketvar.h +++ b/sys/sys/socketvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: socketvar.h,v 1.127 2024/03/27 22:47:53 mvs Exp $ */ +/* $OpenBSD: socketvar.h,v 1.128 2024/04/10 12:04:41 mvs Exp $ */ /* $NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $ */ /*- @@ -86,7 +86,6 @@ struct socket { short so_q0len; /* partials on so_q0 */ short so_qlen; /* number of connections on so_q */ short so_qlimit; /* max number queued connections */ - u_long so_newconn; /* # of pending sonewconn() threads */ short so_timeo; /* connection timeout */ u_long so_oobmark; /* chars to oob mark */ u_int so_error; /* error affecting connection */ @@ -169,8 +168,7 @@ struct socket { #define SS_CONNECTOUT 0x1000 /* connect, not accept, at this end */ #define SS_ISSENDING 0x2000 /* hint for lower layer */ #define SS_DNS 0x4000 /* created using SOCK_DNS socket(2) */ -#define SS_NEWCONN_WAIT 0x8000 /* waiting sonewconn() relock */ -#define SS_YP 0x10000 /* created using ypconnect(2) */ +#define SS_YP 0x8000 /* created using ypconnect(2) */ #ifdef _KERNEL @@ -400,7 +398,6 @@ int sosend(struct socket *, struct mbuf *, struct uio *, struct mbuf *, struct mbuf *, int); int sosetopt(struct socket *, int, int, struct mbuf *); int soshutdown(struct socket *, int); -void sorflush(struct socket *); void sowakeup(struct socket *, struct sockbuf *); void sorwakeup(struct socket *); void sowwakeup(struct socket *); -- 2.20.1