[src/trunk]: src/sys/kern Modify m_defrag, so that it never free...

[maxv <maxv%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/74df38228e77
branches:  trunk
changeset: 318521:74df38228e77
user:      maxv <maxv%NetBSD.org@localhost>
date:      Sat Apr 28 08:16:15 2018 +0000
description:
Modify m_defrag, so that it never frees the first mbuf of the chain. While
here use the given 'flags' argument, and not M_DONTWAIT.

We have a problem with several drivers: they poll an mbuf chain from their
queues and call m_defrag on them, but m_defrag could update the mbuf
pointer, so the mbuf in the queue is no longer valid. It is not easy to
fix each driver, because doing pop+push will reorder the queue, and we
don't really want that to happen.

This problem was independently spotted by me, Kengo, Masanobu, and other
people too it seems (perhaps PR/53218).

Now m_defrag leaves the first mbuf in place, and compresses the chain
only starting from the second mbuf in the chain.

It is important not to compress the first mbuf with hacks, because the
storage of this first mbuf may be shared with other mbufs.

diffstat:

 sys/kern/uipc_mbuf.c |  32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diffs (81 lines):

diff -r 560ef32153d3 -r 74df38228e77 sys/kern/uipc_mbuf.c
--- a/sys/kern/uipc_mbuf.c      Sat Apr 28 05:12:54 2018 +0000
+++ b/sys/kern/uipc_mbuf.c      Sat Apr 28 08:16:15 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_mbuf.c,v 1.210 2018/04/27 19:06:48 maxv Exp $     */
+/*     $NetBSD: uipc_mbuf.c,v 1.211 2018/04/28 08:16:15 maxv Exp $     */
 
 /*
  * Copyright (c) 1999, 2001 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.210 2018/04/27 19:06:48 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.211 2018/04/28 08:16:15 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_mbuftrace.h"
@@ -1475,27 +1475,32 @@
 }
 
 /*
- * Copy the mbuf chain to a new mbuf chain that is as short as possible.
- * Return the new mbuf chain on success, NULL on failure.  On success,
- * free the old mbuf chain.
+ * Compress the mbuf chain. Return the new mbuf chain on success, NULL on
+ * failure. The first mbuf is preserved, and on success the pointer returned
+ * is the same as the one passed.
  */
 struct mbuf *
 m_defrag(struct mbuf *mold, int flags)
 {
        struct mbuf *m0, *mn, *n;
-       size_t sz = mold->m_pkthdr.len;
+       int sz;
 
        KASSERT((mold->m_flags & M_PKTHDR) != 0);
 
-       m0 = m_gethdr(flags, MT_DATA);
+       if (mold->m_next == NULL)
+               return mold;
+
+       m0 = m_get(flags, MT_DATA);
        if (m0 == NULL)
                return NULL;
-       M_COPY_PKTHDR(m0, mold);
        mn = m0;
 
+       sz = mold->m_pkthdr.len - mold->m_len;
+       KASSERT(sz >= 0);
+
        do {
-               if (sz > MHLEN) {
-                       MCLGET(mn, M_DONTWAIT);
+               if (sz > MLEN) {
+                       MCLGET(mn, flags);
                        if ((mn->m_flags & M_EXT) == 0) {
                                m_freem(m0);
                                return NULL;
@@ -1511,7 +1516,7 @@
 
                if (sz > 0) {
                        /* need more mbufs */
-                       n = m_get(M_NOWAIT, MT_DATA);
+                       n = m_get(flags, MT_DATA);
                        if (n == NULL) {
                                m_freem(m0);
                                return NULL;
@@ -1522,9 +1527,10 @@
                }
        } while (sz > 0);
 
-       m_freem(mold);
+       m_freem(mold->m_next);
+       mold->m_next = m0;
 
-       return m0;
+       return mold;
 }
 
 void