Download raw body.
m_getuio - prepend mbuf allocations
On 10/31/24 19:10, David Hill wrote:
> I am attempting to cleanup 'prepend mbuf allocations' and have included
> a diff below.
>
> These sendto's are the most frequest now that wireguard fixes are in.
> I assume these are mostly unbound doing DNS.
>
> m_prepend: len 48
> -+- mbuf 0xfffffd8013662000, dat, off 0, len 33, pktlen 33, size 144
> \\- total chain 1, len 33, size 144
> Starting stack trace...
> m_print_debug(fffffd8013662000,ffffffff819eb167,30) at m_print_debug+0x31d
> m_prepend(fffffd8013662000,30,2) at m_prepend+0xbe
> udp6_output(fffffd824dab66d0,fffffd8013662000,0,0) at udp6_output+0x167
> sosend(ffff8000003b2040,0,ffff80003172d0c0,0,0,80) at sosend+0x3e2
> sendit(ffff80003162e540,6,ffff80003172d1b8,0,ffff80003172d250) at
> sendit+0x395
> sys_sendto(ffff80003162e540,ffff80003172d2e0,ffff80003172d250) at
> sys_sendto+0x6c
> syscall(ffff80003172d2e0) at syscall+0x620
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7cc803157ff0, count: 249
> End of stack trace.
>
> m_prepend: len 28
> -+- mbuf 0xfffffd800a2f5a00, dat, off 0, len 23, pktlen 23, size 144
> \- total chain 1, len 23, size 144
> Starting stack trace...
> m_print_debug(fffffd800a2f5a00,ffffffff819e7e0d,1c) at m_print_debug+0x31d
> m_prepend(fffffd800a2f5a00,1c,2) at m_prepend+0xbe
> udp_output(fffffd820aef7938,fffffd800a2f5a00,fffffd801483b600,0) at
> udp_output+0x2f2
> sosend(ffff80000089e548,fffffd801483b600,ffff800031614ff0,0,0,80) at
> sosend+0x3e2
> sendit(ffff80003161c2a8,3,ffff8000316150e8,0,ffff800031615180) at
> sendit+0x395
> sys_sendto(ffff80003161c2a8,ffff800031615210,ffff800031615180) at
> sys_sendto+0x6c
> syscall(ffff800031615210) at syscall+0x620
> Xsyscall() at Xsyscall+0x128
> end of kernel
> end trace frame: 0x7560c50f07b0, count: 249
> End of stack trace.
>
> The following diff does not produce any of the debugs above. Please
> tell me if this is correct or have caused another issue.
>
> 1.) on first iteration, only write at most (MHLEN-max_hdr) bytes, saving
> room for max_hdr.
>
> 2.) since MINCLSIZE > (MHLEN-max_hdr), delete the code to move m_data
> forward since it m will never equal top here.
>
> 3.) fix off by one in comparison.
>
> Index: kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> diff -u -p -r1.344 uipc_socket.c
> --- kern/uipc_socket.c 31 Oct 2024 12:51:55 -0000 1.344
> +++ kern/uipc_socket.c 31 Oct 2024 17:41:27 -0000
> @@ -795,6 +795,7 @@ m_getuio(struct mbuf **mp, int atomic, l
> if (top == NULL) {
> MGETHDR(m, M_WAIT, MT_DATA);
> mlen = MHLEN;
> + resid = ulmin(resid, MHLEN - max_hdr);
> } else {
> MGET(m, M_WAIT, MT_DATA);
> mlen = MLEN;
> @@ -812,12 +813,6 @@ m_getuio(struct mbuf **mp, int atomic, l
> 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);
> @@ -825,7 +820,7 @@ nopages:
> * For datagram protocols, leave room
> * for protocol headers in first mbuf.
> */
> - if (atomic && m == top && len < mlen - max_hdr)
> + if (atomic && m == top && len <= mlen - max_hdr)
> m_align(m, len);
> }
>
I missed checking for atomic. Which makes the diff larger and the code
smaller:
1.) if atomic, put at most (MHLEN-max_hdr) bytes into the header,
leaving room for max_hdr; This removes the need for the atomic checks
in each conditional.
2.) By reversing the logic check for (m->m_flags & M_EXT), we can remove
the else {} block.
Index: uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
diff -u -p -r1.344 uipc_socket.c
--- uipc_socket.c 31 Oct 2024 12:51:55 -0000 1.344
+++ uipc_socket.c 1 Nov 2024 12:13:04 -0000
@@ -795,6 +795,14 @@ m_getuio(struct mbuf **mp, int atomic, l
if (top == NULL) {
MGETHDR(m, M_WAIT, MT_DATA);
mlen = MHLEN;
+ /*
+ * For datagram protocols, leave room
+ * for protocol headers in first mbuf.
+ */
+ if (atomic) {
+ resid = ulmin(resid, MHLEN - max_hdr);
+ m_align(m, resid);
+ }
} else {
MGET(m, M_WAIT, MT_DATA);
mlen = MLEN;
@@ -808,26 +816,10 @@ m_getuio(struct mbuf **mp, int atomic, l
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);
+ if ((m->m_flags & M_EXT) != 0)
+ mlen = m->m_ext.ext_size;
}
+ len = ulmin(mlen, resid);
error = uiomove(mtod(m, caddr_t), len, uio);
if (error) {
m_getuio - prepend mbuf allocations