From: Matthew Dillon <dillon@apollo.backplane.com>
Date: Wed, 18 Aug 2010 18:50:56 +0000 (-0700)
Subject: kernel - Fix excessive mbuf use in nfs_realign()
X-Git-Tag: v2.9.0~490
X-Git-Url: https://gitweb.dragonflybsd.org/~tuxillo/dragonfly.git/commitdiff_plain/3bf6fec386b09ee8846e7093b18975148d9da2cc

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 1b2a53f0b4..c5bb02b702 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -1316,6 +1316,80 @@ nospace0:
 	return (NULL);
 }
 
+/*
+ * 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 = &top;
+	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).
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index c39820ab93..f796d9aa64 100644
--- 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 7399d003d5..c241c3cead 100644
--- 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;
 	}