From 9f40d4ac917f4741ac5a363a0e31bf8149ffc6ce Mon Sep 17 00:00:00 2001
From: mvs <mvs@openbsd.org>
Date: Tue, 4 Jul 2023 22:28:24 +0000
Subject: [PATCH] Introduce SBL_WAIT and SBL_NOINTR sbwait() flags.

This refactoring is another step to make standalone socket buffers
locking. sblock() uses M_WAITOK and M_NOWAIT flags passed as the third
argument together with the SB_NOINTR flag on the `sb_flags' to control
sleep behaviour. To perform uninterruptible acquisition, SB_NOINTR flag
should be set before sblock() call. `sb_flags' modification requires to
hold solock() around sblock()/sbunlock() that makes standalone call
impossible.

Also `sb_flags' modifications outside sblock()/sbunlock() makes
uninterruptible acquisition code huge enough. This time only sorflush()
does this (and forgets to restore SB_NOINTR flag, so shutdown(SHUT_RDWR)
call permanently modifies socket locking behaviour) and this looks not
the big problem. But with the standalone socket buffer locking it will
be many such places, so this huge construction is unwanted.

Introduce new SBL_NOINTR flag passed as third sblock() argument. The
sblock() acquisition will be uninterruptible when existing SB_NOINTR
flag is set on `sb_flags' or SBL_NOINTR was passed.

The M_WAITOK and M_NOWAIT flags belongs to malloc(9). It has no M_NOINTR
flag and there is no reason to introduce it. So for consistency reasons
introduce new SBL_WAIT and use it together with SBL_NOINTR instead of
M_WAITOK and M_NOINTR respectively.

ok bluhm
---
 sys/kern/uipc_socket.c  | 15 +++++++--------
 sys/kern/uipc_socket2.c | 10 ++++++----
 sys/sys/socketvar.h     | 13 ++++++++++---
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 978bfff87f8..da37b31f797 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.304 2023/06/30 11:52:11 mvs Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.305 2023/07/04 22:28:24 mvs Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -524,7 +524,7 @@ sodisconnect(struct socket *so)
 
 int m_getuio(struct mbuf **, int, long, struct uio *);
 
-#define	SBLOCKWAIT(f)	(((f) & MSG_DONTWAIT) ? M_NOWAIT : M_WAITOK)
+#define	SBLOCKWAIT(f)	(((f) & MSG_DONTWAIT) ? 0 : SBL_WAIT)
 /*
  * Send on a socket.
  * If send must go all at once and message is larger than
@@ -1219,9 +1219,8 @@ sorflush(struct socket *so)
 	const struct protosw *pr = so->so_proto;
 	int error;
 
-	sb->sb_flags |= SB_NOINTR;
-	error = sblock(so, sb, M_WAITOK);
-	/* with SB_NOINTR and M_WAITOK sblock() must not fail */
+	error = sblock(so, sb, SBL_WAIT | SBL_NOINTR);
+	/* with SBL_WAIT and SLB_NOINTR sblock() must not fail */
 	KASSERT(error == 0);
 	socantrcvmore(so);
 	m = sb->sb_mb;
@@ -1290,7 +1289,7 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
 	/* If no fd is given, unsplice by removing existing link. */
 	if (fd < 0) {
 		/* Lock receive buffer. */
-		if ((error = sblock(so, &so->so_rcv, M_WAITOK)) != 0) {
+		if ((error = sblock(so, &so->so_rcv, SBL_WAIT)) != 0) {
 			return (error);
 		}
 		if (so->so_sp->ssp_socket)
@@ -1323,10 +1322,10 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
 	}
 
 	/* Lock both receive and send buffer. */
-	if ((error = sblock(so, &so->so_rcv, M_WAITOK)) != 0) {
+	if ((error = sblock(so, &so->so_rcv, SBL_WAIT)) != 0) {
 		goto frele;
 	}
-	if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) {
+	if ((error = sblock(so, &sosp->so_snd, SBL_WAIT)) != 0) {
 		sbunlock(so, &so->so_rcv);
 		goto frele;
 	}
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index 9965baaf91b..e6cc57fdf9c 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket2.c,v 1.136 2023/02/10 14:34:17 visa Exp $	*/
+/*	$OpenBSD: uipc_socket2.c,v 1.137 2023/07/04 22:28:24 mvs Exp $	*/
 /*	$NetBSD: uipc_socket2.c,v 1.11 1996/02/04 02:17:55 christos Exp $	*/
 
 /*
@@ -494,9 +494,9 @@ sbwait(struct socket *so, struct sockbuf *sb)
 }
 
 int
-sblock(struct socket *so, struct sockbuf *sb, int wait)
+sblock(struct socket *so, struct sockbuf *sb, int flags)
 {
-	int error, prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
+	int error, prio = PSOCK;
 
 	soassertlocked(so);
 
@@ -504,8 +504,10 @@ sblock(struct socket *so, struct sockbuf *sb, int wait)
 		sb->sb_flags |= SB_LOCK;
 		return (0);
 	}
-	if (wait & M_NOWAIT)
+	if ((flags & SBL_WAIT) == 0)
 		return (EWOULDBLOCK);
+	if (!(flags & SBL_NOINTR || sb->sb_flags & SB_NOINTR))
+		prio |= PCATCH;
 
 	while (sb->sb_flags & SB_LOCK) {
 		sb->sb_flags |= SB_WANT;
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index 834a2b582eb..0290d643780 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: socketvar.h,v 1.119 2023/01/27 18:46:34 mvs Exp $	*/
+/*	$OpenBSD: socketvar.h,v 1.120 2023/07/04 22:28:24 mvs Exp $	*/
 /*	$NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $	*/
 
 /*-
@@ -272,10 +272,17 @@ sbfree(struct socket *so, struct sockbuf *sb, struct mbuf *m)
 		sb->sb_mbcnt -= m->m_ext.ext_size;
 }
 
+/*
+ * Flags to sblock()
+ */
+#define SBL_WAIT	0x01	/* Wait if lock not immediately available. */
+#define SBL_NOINTR	0x02	/* Enforce non-interruptible sleep. */
+
 /*
  * Set lock on sockbuf sb; sleep if lock is already held.
- * Unless SB_NOINTR is set on sockbuf, sleep is interruptible.
- * Returns error without lock if sleep is interrupted.
+ * Unless SB_NOINTR is set on sockbuf or SBL_NOINTR passed,
+ * sleep is interruptible. Returns error without lock if
+ * sleep is interrupted.
  */
 int sblock(struct socket *, struct sockbuf *, int);
 
-- 
2.20.1