kernel - Fix excessive mbuf use in nfs_realign()

* nfs_realign() was calling m_copyback() which itself uses the deprecated
  m_getclr(), and m_getclr() only allocates non-cluster mbufs.

  This caused nfs_realign() to use an excessive number of mbufs and can
  exhaust the mbuf pool on systems with small amounts of memory.

Reported-by: Antonio Huete Jimenez <tuxillo@quantumachine.net>

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 1b2a53f..c5bb02b 100644 (file)
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -1317,6 +1317,80 @@ nospace0:
 }
 
 /*
+ * Copy the non-packet mbuf data chain into a new set of mbufs, including
+ * copying any mbuf clusters.  This is typically used to realign a data
+ * chain by nfs_realign().
+ *
+ * The original chain is left intact.  how should be MB_WAIT or MB_DONTWAIT
+ * and NULL can be returned if MB_DONTWAIT is passed.
+ *
+ * Be careful to use cluster mbufs, a large mbuf chain converted to non
+ * cluster mbufs can exhaust our supply of mbufs.
+ */
+struct mbuf *
+m_dup_data(struct mbuf *m, int how)
+{
+       struct mbuf **p, *n, *top = NULL;
+       int mlen, moff, chunk, gsize, nsize;
+
+       /*
+        * Degenerate case
+        */
+       if (m == NULL)
+               return (NULL);
+
+       /*
+        * Optimize the mbuf allocation but do not get too carried away.
+        */
+       if (m->m_next || m->m_len > MLEN)
+               gsize = MCLBYTES;
+       else
+               gsize = MLEN;
+
+       /* Chain control */
+       p = ⊤
+       n = NULL;
+       nsize = 0;
+
+       /*
+        * Scan the mbuf chain until nothing is left, the new mbuf chain
+        * will be allocated on the fly as needed.
+        */
+       while (m) {
+               mlen = m->m_len;
+               moff = 0;
+
+               while (mlen) {
+                       KKASSERT(m->m_type == MT_DATA);
+                       if (n == NULL) {
+                               n = m_getl(gsize, how, MT_DATA, 0, &nsize);
+                               n->m_len = 0;
+                               if (n == NULL)
+                                       goto nospace;
+                               *p = n;
+                               p = &n->m_next;
+                       }
+                       chunk = imin(mlen, nsize);
+                       bcopy(m->m_data + moff, n->m_data + n->m_len, chunk);
+                       mlen -= chunk;
+                       moff += chunk;
+                       n->m_len += chunk;
+                       nsize -= chunk;
+                       if (nsize == 0)
+                               n = NULL;
+               }
+               m = m->m_next;
+       }
+       *p = NULL;
+       return(top);
+nospace:
+       *p = NULL;
+       m_freem(top);
+       atomic_add_long_nonlocked(&mbstat[mycpu->gd_cpuid].m_mcfail, 1);
+       return (NULL);
+}
+
+/*
  * Concatenate mbuf chain n to m.
  * Both chains must be of the same type (e.g. MT_DATA).
  * Any m_pkthdr is not updated.

diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index c39820a..f796d9a 100644 (file)
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -471,6 +471,7 @@ struct      mbuf    *m_defrag_nofree(struct mbuf *, int);
 struct mbuf    *m_devget(char *, int, int, struct ifnet *,
                  void (*copy)(volatile const void *, volatile void *, size_t));
 struct mbuf    *m_dup(struct mbuf *, int);
+struct mbuf    *m_dup_data(struct mbuf *, int);
 int             m_dup_pkthdr(struct mbuf *, const struct mbuf *, int);
 struct mbuf    *m_free(struct mbuf *);
 void            m_freem(struct mbuf *);

diff --git a/sys/vfs/nfs/nfs_socket.c b/sys/vfs/nfs/nfs_socket.c
index 7399d00..c241c3c 100644 (file)
--- a/sys/vfs/nfs/nfs_socket.c
+++ b/sys/vfs/nfs/nfs_socket.c
@@ -2178,30 +2178,23 @@ nfs_realign(struct mbuf **pm, int hsiz)
 {
        struct mbuf *m;
        struct mbuf *n = NULL;
-       int off = 0;
 
+       /*
+        * Check for misalignemnt
+        */
        ++nfs_realign_test;
-
        while ((m = *pm) != NULL) {
-               if ((m->m_len & 0x3) || (mtod(m, intptr_t) & 0x3)) {
-                       n = m_getl(m->m_len, MB_WAIT, MT_DATA, 0, NULL);
-                       n->m_len = 0;
+               if ((m->m_len & 0x3) || (mtod(m, intptr_t) & 0x3))
                        break;
-               }
                pm = &m->m_next;
        }
 
        /*
-        * If n is non-NULL, loop on m copying data, then replace the
-        * portion of the chain that had to be realigned.
+        * If misalignment found make a completely new copy.
         */
-       if (n != NULL) {
+       if (m) {
                ++nfs_realign_count;
-               while (m) {
-                       m_copyback(n, off, m->m_len, mtod(m, caddr_t));
-                       off += m->m_len;
-                       m = m->m_next;
-               }
+               n = m_dup_data(m, MB_WAIT);
                m_freem(*pm);
                *pm = n;
        }