From 3d2a3ff25e9504edd8d8762138b2cc7f990c66df Mon Sep 17 00:00:00 2001 From: Bosko Milekic Date: Thu, 10 Feb 2005 22:23:02 +0000 Subject: [PATCH] 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 --- sys/kern/kern_mbuf.c | 6 ++--- sys/kern/uipc_mbuf.c | 58 ++++++++++++++++++++++++++++---------------- sys/sys/mbuf.h | 24 +++++++++++++++--- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c index 0b81b321e71..382e1495230 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 2f2846282b8..90beee54dbb 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 b7d8ce1dc9a..d8e84a60c47 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 { \