Optimize the way reference counting is performed with Mbufs. We

do not need to perform an extra memory fetch in the Packet (Mbuf+Cluster)
constructor to initialize the reference counter anymore.  The reference
counts are located in a separate memory region (in the slab header,
because this zone is UMA_ZONE_REFCNT), so the memory fetch resulted very
often in a cache miss.  Additionally, and perhaps more significantly,
optimize the free mbuf+cluster (packet) case, which is very common, to
no longer require an atomic operation on free (to verify the reference
counter) if the reference on the cluster has never been increased (also
very common).  Reduces an atomic on mbuf free on average.

Original patch submitted by: Gerrit Nagelhout <gnagelhout@sandvine.com>

3 files changed, 60 insertions, 28 deletions

diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c
index 0b81b32..382e149 100644
--- a/sys/kern/kern_mbuf.c
+++ b/sys/kern/kern_mbuf.c
@@ -262,9 +262,7 @@ mb_ctor_clust(void *mem, int size, void *arg, int how)
 	m->m_ext.ext_args = NULL;
 	m->m_ext.ext_size = MCLBYTES;
 	m->m_ext.ext_type = EXT_CLUSTER;
-	m->m_ext.ref_cnt = (u_int *)uma_find_refcnt(zone_clust,
-	    m->m_ext.ext_buf);
-	*(m->m_ext.ref_cnt) = 1;
+	m->m_ext.ref_cnt = NULL;	/* Lazy counter assign. */
 	mbstat.m_mclusts += 1;	/* XXX */
 	return (0);
 }
@@ -337,7 +335,7 @@ mb_ctor_pack(void *mem, int size, void *arg, int how)
 	m->m_ext.ext_args = NULL;
 	m->m_ext.ext_size = MCLBYTES;
 	m->m_ext.ext_type = EXT_PACKET;
-	*(m->m_ext.ref_cnt) = 1;
+	m->m_ext.ref_cnt = NULL;	/* Lazy counter assign. */
 
 	if (flags & M_PKTHDR) {
 		m->m_pkthdr.rcvif = NULL;
diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 2f28462..90beee5 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -222,37 +222,49 @@ void
 mb_free_ext(struct mbuf *m)
 {
 	u_int cnt;
+	int dofree;
+
+	/* Account for lazy ref count assign. */
+	if (m->m_ext.ref_cnt == NULL)
+		dofree = 1;
+	else
+		dofree = 0;
 
 	/*
 	 * This is tricky.  We need to make sure to decrement the
 	 * refcount in a safe way but to also clean up if we're the
 	 * last reference.  This method seems to do it without race.
 	 */
-	do {
+	while (dofree == 0) {
 		cnt = *(m->m_ext.ref_cnt);
 		if (atomic_cmpset_int(m->m_ext.ref_cnt, cnt, cnt - 1)) {
-			if (cnt == 1) {
-				/*
-				 * Do the free, should be safe.
-				 */
-				if (m->m_ext.ext_type == EXT_PACKET) {
-					uma_zfree(zone_pack, m);
-					return;
-				} else if (m->m_ext.ext_type == EXT_CLUSTER) {
-					uma_zfree(zone_clust, m->m_ext.ext_buf);
-					m->m_ext.ext_buf = NULL;
-				} else {
-					(*(m->m_ext.ext_free))(m->m_ext.ext_buf,
-					    m->m_ext.ext_args);
-					if (m->m_ext.ext_type != EXT_EXTREF)
-						free(m->m_ext.ref_cnt, M_MBUF);
-					m->m_ext.ext_buf = NULL;
-				}
-			}
-			/* Decrement (and potentially free) done, safely. */
+			if (cnt == 1)
+				dofree = 1;
 			break;
 		}
-	} while (1);
+	}
+
+	if (dofree) {
+		/*
+		 * Do the free, should be safe.
+		 */
+		if (m->m_ext.ext_type == EXT_PACKET) {
+			uma_zfree(zone_pack, m);
+			return;
+		} else if (m->m_ext.ext_type == EXT_CLUSTER) {
+			uma_zfree(zone_clust, m->m_ext.ext_buf);
+			m->m_ext.ext_buf = NULL;
+		} else {
+			(*(m->m_ext.ext_free))(m->m_ext.ext_buf,
+			    m->m_ext.ext_args);
+			if (m->m_ext.ext_type != EXT_EXTREF) {
+				if (m->m_ext.ref_cnt != NULL)
+					free(m->m_ext.ref_cnt, M_MBUF);
+				m->m_ext.ref_cnt = NULL;
+			}
+			m->m_ext.ext_buf = NULL;
+		}
+	}
 	uma_zfree(zone_mbuf, m);
 }
 
@@ -405,6 +417,7 @@ m_copym(struct mbuf *m, int off0, int len, int wait)
 			n->m_ext = m->m_ext;
 			n->m_flags |= M_EXT;
 			MEXT_ADD_REF(m);
+			n->m_ext.ref_cnt = m->m_ext.ref_cnt;
 		} else
 			bcopy(mtod(m, caddr_t)+off, mtod(n, caddr_t),
 			    (u_int)n->m_len);
@@ -452,6 +465,7 @@ m_copypacket(struct mbuf *m, int how)
 		n->m_ext = m->m_ext;
 		n->m_flags |= M_EXT;
 		MEXT_ADD_REF(m);
+		n->m_ext.ref_cnt = m->m_ext.ref_cnt;
 	} else {
 		n->m_data = n->m_pktdat + (m->m_data - m->m_pktdat );
 		bcopy(mtod(m, char *), mtod(n, char *), n->m_len);
@@ -472,6 +486,7 @@ m_copypacket(struct mbuf *m, int how)
 			n->m_ext = m->m_ext;
 			n->m_flags |= M_EXT;
 			MEXT_ADD_REF(m);
+			n->m_ext.ref_cnt = m->m_ext.ref_cnt;
 		} else {
 			bcopy(mtod(m, char *), mtod(n, char *), n->m_len);
 		}
@@ -807,6 +822,7 @@ extpacket:
 		n->m_flags |= M_EXT;
 		n->m_ext = m->m_ext;
 		MEXT_ADD_REF(m);
+		n->m_ext.ref_cnt = m->m_ext.ref_cnt;
 		n->m_data = m->m_data + len;
 	} else {
 		bcopy(mtod(m, caddr_t) + len, mtod(n, caddr_t), remain);
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index b7d8ce1..d8e84a6 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -295,16 +295,34 @@ struct mbstat {
  * MEXT_REM_REF(m): remove reference to m_ext object.
  *
  * MEXT_ADD_REF(m): add reference to m_ext object already
- *     referred to by (m).
+ *     referred to by (m).  XXX Note that it is VERY important that you
+ *     always set the second mbuf's m_ext.ref_cnt to point to the first
+ *     one's (i.e., n->m_ext.ref_cnt = m->m_ext.ref_cnt) AFTER you run
+ *     MEXT_ADD_REF(m).  This is because m might have a lazy initialized
+ *     ref_cnt (NULL) before this is run and it will only be looked up
+ *     from here.  We should make MEXT_ADD_REF() always take two mbufs
+ *     as arguments so that it can take care of this itself.
  */
-#define	MEXT_IS_REF(m)	(*((m)->m_ext.ref_cnt) > 1)
+#define	MEXT_IS_REF(m)	(((m)->m_ext.ref_cnt != NULL)			\
+    && (*((m)->m_ext.ref_cnt) > 1))
 
 #define	MEXT_REM_REF(m) do {						\
+	KASSERT((m)->m_ext.ref_cnt != NULL, ("m_ext refcnt lazy NULL")); \
 	KASSERT(*((m)->m_ext.ref_cnt) > 0, ("m_ext refcnt < 0"));	\
 	atomic_subtract_int((m)->m_ext.ref_cnt, 1);			\
 } while(0)
 
-#define	MEXT_ADD_REF(m)	atomic_add_int((m)->m_ext.ref_cnt, 1)
+#define	MEXT_ADD_REF(m)	do {						\
+	if ((m)->m_ext.ref_cnt == NULL) {				\
+		KASSERT((m)->m_ext.ext_type == EXT_CLUSTER ||		\
+		    (m)->m_ext.ext_type == EXT_PACKET,			\
+		    ("Unexpected mbuf type has lazy refcnt"));		\
+		(m)->m_ext.ref_cnt = (u_int *)uma_find_refcnt(		\
+		    zone_clust, (m)->m_ext.ext_buf);			\
+		*((m)->m_ext.ref_cnt) = 2;				\
+	} else								\
+		atomic_add_int((m)->m_ext.ref_cnt, 1);			\
+} while (0)
 
 #ifdef WITNESS
 #define MBUF_CHECKSLEEP(how) do {					\