From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: rewrite m_getuio() and bring it into this century
To: tech@openbsd.org
Date: Thu, 26 Jun 2025 17:35:25 +0200

On Tue, Jun 24, 2025 at 01:59:35PM +0200, Claudio Jeker wrote:
> 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

tested and OK bluhm@

> 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;
>  }