Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 9 Oct 2017 20:51:59 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r324447 - in head/sys: kern sys
Message-ID:  <201710092051.v99KpxPC077380@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: glebius
Date: Mon Oct  9 20:51:58 2017
New Revision: 324447
URL: https://svnweb.freebsd.org/changeset/base/324447

Log:
  In mb_dupcl() don't copy full m_ext, to avoid cache miss.  Respectively,
  in mb_free_ext() always use fields from the original refcount holding
  mbuf (see. r296242) mbuf.  Cuts another cache miss from mb_free_ext().
  
  However, treat EXT_EXTREF mbufs differently, since they are different -
  they don't have a refcount holding mbuf.
  
  Provide longer comments in m_ext declaration to explain this change
  and change from r296242.
  
  In collaboration with:	gallatin
  Differential Revision:	https://reviews.freebsd.org/D12615

Modified:
  head/sys/kern/kern_mbuf.c
  head/sys/kern/uipc_mbuf.c
  head/sys/sys/mbuf.h

Modified: head/sys/kern/kern_mbuf.c
==============================================================================
--- head/sys/kern/kern_mbuf.c	Mon Oct  9 20:35:31 2017	(r324446)
+++ head/sys/kern/kern_mbuf.c	Mon Oct  9 20:51:58 2017	(r324447)
@@ -675,20 +675,20 @@ mb_free_ext(struct mbuf *m)
 			uma_zfree(zone_mbuf, mref);
 			break;
 		case EXT_SFBUF:
-			sf_ext_free(m->m_ext.ext_arg1, m->m_ext.ext_arg2);
+			sf_ext_free(mref->m_ext.ext_arg1, mref->m_ext.ext_arg2);
 			uma_zfree(zone_mbuf, mref);
 			break;
 		case EXT_SFBUF_NOCACHE:
-			sf_ext_free_nocache(m->m_ext.ext_arg1,
-			    m->m_ext.ext_arg2);
+			sf_ext_free_nocache(mref->m_ext.ext_arg1,
+			    mref->m_ext.ext_arg2);
 			uma_zfree(zone_mbuf, mref);
 			break;
 		case EXT_NET_DRV:
 		case EXT_MOD_TYPE:
 		case EXT_DISPOSABLE:
-			KASSERT(m->m_ext.ext_free != NULL,
+			KASSERT(mref->m_ext.ext_free != NULL,
 				("%s: ext_free not set", __func__));
-			m->m_ext.ext_free(m);
+			mref->m_ext.ext_free(mref);
 			uma_zfree(zone_mbuf, mref);
 			break;
 		case EXT_EXTREF:

Modified: head/sys/kern/uipc_mbuf.c
==============================================================================
--- head/sys/kern/uipc_mbuf.c	Mon Oct  9 20:35:31 2017	(r324446)
+++ head/sys/kern/uipc_mbuf.c	Mon Oct  9 20:51:58 2017	(r324447)
@@ -188,7 +188,17 @@ mb_dupcl(struct mbuf *n, struct mbuf *m)
 	KASSERT(m->m_flags & M_EXT, ("%s: M_EXT not set on %p", __func__, m));
 	KASSERT(!(n->m_flags & M_EXT), ("%s: M_EXT set on %p", __func__, n));
 
-	n->m_ext = m->m_ext;
+	/*
+	 * Cache access optimization.  For most kinds of external
+	 * storage we don't need full copy of m_ext, since the
+	 * holder of the 'ext_count' is responsible to carry the
+	 * free routine and its arguments.  Exclusion is EXT_EXTREF,
+	 * where 'ext_cnt' doesn't point into mbuf at all.
+	 */
+	if (m->m_ext.ext_type == EXT_EXTREF)
+		bcopy(&m->m_ext, &n->m_ext, sizeof(struct m_ext));
+	else
+		bcopy(&m->m_ext, &n->m_ext, m_ext_copylen);
 	n->m_flags |= M_EXT;
 	n->m_flags |= m->m_flags & M_RDONLY;
 

Modified: head/sys/sys/mbuf.h
==============================================================================
--- head/sys/sys/mbuf.h	Mon Oct  9 20:35:31 2017	(r324446)
+++ head/sys/sys/mbuf.h	Mon Oct  9 20:51:58 2017	(r324447)
@@ -200,13 +200,29 @@ struct pkthdr {
 typedef	void m_ext_free_t(struct mbuf *);
 struct m_ext {
 	union {
-		volatile u_int	 ext_count;	/* value of ref count info */
-		volatile u_int	*ext_cnt;	/* pointer to ref count info */
+		/*
+		 * If EXT_FLAG_EMBREF is set, then we use refcount in the
+		 * mbuf, the 'ext_count' member.  Otherwise, we have a
+		 * shadow copy and we use pointer 'ext_cnt'.  The original
+		 * mbuf is responsible to carry the pointer to free routine
+		 * and its arguments.  They aren't copied into shadows in
+		 * mb_dupcl() to avoid dereferencing next cachelines.
+		 */
+		volatile u_int	 ext_count;
+		volatile u_int	*ext_cnt;
 	};
 	char		*ext_buf;	/* start of buffer */
 	uint32_t	 ext_size;	/* size of buffer, for ext_free */
 	uint32_t	 ext_type:8,	/* type of external storage */
 			 ext_flags:24;	/* external storage mbuf flags */
+	/*
+	 * Fields below store the free context for the external storage.
+	 * They are valid only in the refcount carrying mbuf, the one with
+	 * EXT_FLAG_EMBREF flag, with exclusion for EXT_EXTREF type, where
+	 * the free context is copied into all mbufs that use same external
+	 * storage.
+	 */
+#define	m_ext_copylen	offsetof(struct m_ext, ext_free)
 	m_ext_free_t	*ext_free;	/* free routine if not the usual */
 	void		*ext_arg1;	/* optional argument pointer */
 	void		*ext_arg2;	/* optional argument pointer */

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201710092051.v99KpxPC077380>