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

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 8c33b9d..e5f4c1c 100644 (file)
--- 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 4fb4819..9c52cf7 100644 (file)
--- 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 17db0cd..f506397 100644 (file)
--- 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 0169c62..cca3780 100644 (file)
--- 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 *);