Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
rewrite m_getuio() and bring it into this century
To:
tech@openbsd.org
Date:
Tue, 24 Jun 2025 13:59:35 +0200

Download raw body.

Thread
The m_getuio() function has a few problems.
First of all it does not align the data well so we end up with extra
m_prepend() calls later on. Then it does not handle cluster allocation
failure in a good way and falls back to massive mbuf chains (which will
result in an m_defrag() call in most drivers anyway).

This is joint work with dlg@ (he did the m_align() bits).
I just added the M_WAIT bit to m_clget() which simplifies the function
even more.

The old M_NOWAIT and chain the hell out of mbufs mode is from a long time
ago when the number of mbuf clusters was a compile time option and very
limited. This mode makes no sense anymore.
-- 
:wq Claudio

diff --git sys/kern/uipc_socket.c sys/kern/uipc_socket.c
index 06110f1480a..6ad840d9203 100644
--- sys/kern/uipc_socket.c
+++ sys/kern/uipc_socket.c
@@ -724,47 +724,34 @@ m_getuio(struct mbuf **mp, int atomic, long space, struct uio *uio)
 {
 	struct mbuf *m, *top = NULL;
 	struct mbuf **nextp = &top;
-	u_long len, mlen;
-	size_t resid = uio->uio_resid;
+	u_long len, mlen, alen;
+	int align = atomic ? roundup(max_hdr, sizeof(long)) : 0;
 	int error;
 
 	do {
-		if (top == NULL) {
-			MGETHDR(m, M_WAIT, MT_DATA);
+		/* How much data we want to put in this mbuf? */
+		len = ulmin(uio->uio_resid, space);
+		/* How much space are we allocating for that data? */
+		alen = align + len;
+		if (top == NULL && alen <= MHLEN) {
+			m = m_gethdr(M_WAIT, MT_DATA);
 			mlen = MHLEN;
 		} else {
-			MGET(m, M_WAIT, MT_DATA);
-			mlen = MLEN;
+			m = m_clget(NULL, M_WAIT, ulmin(alen, MAXMCLBYTES));
+			mlen = m->m_ext.ext_size;
+			if (top != NULL)
+				m->m_flags &= ~M_PKTHDR;
 		}
+
 		/* chain mbuf together */
 		*nextp = m;
 		nextp = &m->m_next;
 
-		resid = ulmin(resid, space);
-		if (resid >= MINCLSIZE) {
-			MCLGETL(m, M_NOWAIT, ulmin(resid, MAXMCLBYTES));
-			if ((m->m_flags & M_EXT) == 0)
-				MCLGETL(m, M_NOWAIT, MCLBYTES);
-			if ((m->m_flags & M_EXT) == 0)
-				goto nopages;
-			mlen = m->m_ext.ext_size;
-			len = ulmin(mlen, resid);
-			/*
-			 * For datagram protocols, leave room
-			 * for protocol headers in first mbuf.
-			 */
-			if (atomic && m == top && len < mlen - max_hdr)
-				m->m_data += max_hdr;
-		} else {
-nopages:
-			len = ulmin(mlen, resid);
-			/*
-			 * For datagram protocols, leave room
-			 * for protocol headers in first mbuf.
-			 */
-			if (atomic && m == top && len < mlen - max_hdr)
-				m_align(m, len);
-		}
+		/* put the data at the end of the buffer */
+		if (len < mlen)
+			m_align(m, len);
+		else
+			len = mlen;
 
 		error = uiomove(mtod(m, caddr_t), len, uio);
 		if (error) {
@@ -773,14 +760,15 @@ nopages:
 		}
 
 		/* adjust counters */
-		resid = uio->uio_resid;
 		space -= len;
 		m->m_len = len;
 		top->m_pkthdr.len += len;
+		align = 0;
 
 		/* Is there more space and more data? */
-	} while (space > 0 && resid > 0);
+	} while (space > 0 && uio->uio_resid > 0);
 
+	KASSERT(top != NULL);
 	*mp = top;
 	return 0;
 }