Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: mbuf counters splnet
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
tech@openbsd.org
Date:
Sun, 22 Jun 2025 01:32:06 +0300

Download raw body.

Thread
On Sun, Jun 22, 2025 at 12:01:21AM +0200, Alexander Bluhm wrote:
> Hi,
> 
> As dlg@ has pointed out, MP safe conters still need spl protection
> when called from interrupt context.  Some increments of mbuf counters
> have that, others do not.
> 
> I would like to introduce mbstat_inc() inline function and enum
> counter types for mbufs, like we have elsewhere.  With that splnet()
> is taken everywhere.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.478 kern_sysctl.c
> --- sys/kern/kern_sysctl.c	12 Jun 2025 20:37:58 -0000	1.478
> +++ sys/kern/kern_sysctl.c	20 Jun 2025 20:56:56 -0000
> @@ -611,25 +611,25 @@ kern_sysctl(int *name, u_int namelen, vo
>  		return (sysctl_rdstruct(oldp, oldlenp, newp, &bt, sizeof bt));
>  	}
>  	case KERN_MBSTAT: {
> -		uint64_t counters[MBSTAT_COUNT];
> +		uint64_t counters[mbs_ncounters];
>  		struct mbstat mbs;
>  		unsigned int i;
>  
>  		memset(&mbs, 0, sizeof(mbs));
> -		counters_read(mbstat, counters, MBSTAT_COUNT, NULL);
> -		for (i = 0; i < MBSTAT_TYPES; i++)
> +		counters_read(mbstat, counters, mbs_ncounters, NULL);
> +		for (i = 0; i < MT_NTYPES; i++)
>  			mbs.m_mtypes[i] = counters[i];
> -
> -		mbs.m_drops = counters[MBSTAT_DROPS];
> -		mbs.m_wait = counters[MBSTAT_WAIT];
> -		mbs.m_drain = counters[MBSTAT_DRAIN];
> -		mbs.m_defrag_alloc = counters[MBSTAT_DEFRAG_ALLOC];
> -		mbs.m_prepend_alloc = counters[MBSTAT_PREPEND_ALLOC];
> -		mbs.m_pullup_alloc = counters[MBSTAT_PULLUP_ALLOC];
> -		mbs.m_pullup_copy = counters[MBSTAT_PULLUP_COPY];
> -		mbs.m_pulldown_alloc = counters[MBSTAT_PULLDOWN_ALLOC];
> -		mbs.m_pulldown_copy = counters[MBSTAT_PULLDOWN_COPY];
> -
> +#define ASSIGN(name) do { mbs.m_##name = counters[mbs_##name]; } while (0)
> +		ASSIGN(drops);
> +		ASSIGN(wait);
> +		ASSIGN(drain);
> +		ASSIGN(defrag_alloc);
> +		ASSIGN(prepend_alloc);
> +		ASSIGN(pullup_alloc);
> +		ASSIGN(pullup_copy);
> +		ASSIGN(pulldown_alloc);
> +		ASSIGN(pulldown_copy);
> +#undef ASSIGN
>  		return (sysctl_rdstruct(oldp, oldlenp, newp,
>  		    &mbs, sizeof(mbs)));
>  	}
> Index: sys/kern/uipc_mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
> diff -u -p -r1.298 uipc_mbuf.c
> --- sys/kern/uipc_mbuf.c	13 Jun 2025 07:05:27 -0000	1.298
> +++ sys/kern/uipc_mbuf.c	20 Jun 2025 20:56:56 -0000
> @@ -85,7 +85,6 @@
>  #include <sys/socket.h>
>  #include <net/if.h>
>  
> -
>  #include <uvm/uvm_extern.h>
>  
>  #ifdef DDB
> @@ -98,7 +97,7 @@
>  #endif	/* NPF > 0 */
>  
>  /* mbuf stats */
> -COUNTERS_BOOT_MEMORY(mbstat_boot, MBSTAT_COUNT);
> +COUNTERS_BOOT_MEMORY(mbstat_boot, mbs_ncounters);
>  struct cpumem *mbstat = COUNTERS_BOOT_INITIALIZER(mbstat_boot);
>  /* mbuf pools */
>  struct	pool mbpool;
> @@ -201,7 +200,7 @@ mbcpuinit(void)
>  {
>  	int i;
>  
> -	mbstat = counters_alloc_ncpus(mbstat, MBSTAT_COUNT);
> +	mbstat = counters_alloc_ncpus(mbstat, mbs_ncounters);
>  
>  	pool_cache_init(&mbpool);
>  	pool_cache_init(&mtagpool);
> @@ -235,7 +234,6 @@ struct mbuf *
>  m_get(int nowait, int type)
>  {
>  	struct mbuf *m;
> -	int s;
>  
>  	KASSERT(type >= 0 && type < MT_NTYPES);
>  
> @@ -243,9 +241,7 @@ m_get(int nowait, int type)
>  	if (m == NULL)
>  		return (NULL);
>  
> -	s = splnet();
> -	counters_inc(mbstat, type);
> -	splx(s);
> +	mbstat_inc(type);
>  
>  	m->m_type = type;
>  	m->m_next = NULL;
> @@ -264,7 +260,6 @@ struct mbuf *
>  m_gethdr(int nowait, int type)
>  {
>  	struct mbuf *m;
> -	int s;
>  
>  	KASSERT(type >= 0 && type < MT_NTYPES);
>  
> @@ -272,9 +267,7 @@ m_gethdr(int nowait, int type)
>  	if (m == NULL)
>  		return (NULL);
>  
> -	s = splnet();
> -	counters_inc(mbstat, type);
> -	splx(s);
> +	mbstat_inc(type);
>  
>  	m->m_type = type;
>  
> @@ -546,7 +539,7 @@ m_defrag(struct mbuf *m, int how)
>  
>  	KASSERT(m->m_flags & M_PKTHDR);
>  
> -	counters_inc(mbstat, MBSTAT_DEFRAG_ALLOC);
> +	mbstat_inc(mbs_defrag_alloc);
>  	if ((m0 = m_gethdr(how, m->m_type)) == NULL)
>  		return (ENOBUFS);
>  	if (m->m_pkthdr.len > MHLEN) {
> @@ -606,7 +599,7 @@ m_prepend(struct mbuf *m, int len, int h
>  		m->m_data -= len;
>  		m->m_len += len;
>  	} else {
> -		counters_inc(mbstat, MBSTAT_PREPEND_ALLOC);
> +		mbstat_inc(mbs_prepend_alloc);
>  		MGET(mn, how, m->m_type);
>  		if (mn == NULL) {
>  			m_freem(m);
> @@ -948,7 +941,7 @@ m_pullup(struct mbuf *m0, int len)
>  			m0->m_data = head;
>  		}
>  		len -= m0->m_len;
> -		counters_inc(mbstat, MBSTAT_PULLUP_COPY);
> +		mbstat_inc(mbs_pullup_copy);
>  	} else {
>  		/* the first mbuf is too small or read-only, make a new one */
>  		space = adj + len;
> @@ -959,7 +952,7 @@ m_pullup(struct mbuf *m0, int len)
>  		m0->m_next = m;
>  		m = m0;
>  
> -		counters_inc(mbstat, MBSTAT_PULLUP_ALLOC);
> +		mbstat_inc(mbs_pullup_alloc);
>  		MGET(m0, M_DONTWAIT, m->m_type);
>  		if (m0 == NULL)
>  			goto bad;
> @@ -1467,7 +1460,7 @@ m_pool_alloc(struct pool *pp, int flags,
>  		return (v);
>  
>   fail:
> -	counters_inc(mbstat, MBSTAT_DROPS);
> +	mbstat_inc(mbs_drops);
>  	atomic_sub_long(&mbuf_mem_alloc, pp->pr_pgsize);
>  	return (NULL);
>  }
> Index: sys/kern/uipc_mbuf2.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf2.c,v
> diff -u -p -r1.48 uipc_mbuf2.c
> --- sys/kern/uipc_mbuf2.c	27 May 2025 07:52:49 -0000	1.48
> +++ sys/kern/uipc_mbuf2.c	20 Jun 2025 20:56:56 -0000
> @@ -118,7 +118,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  	if (len <= n->m_len - off) {
>  		struct mbuf *mlast;
>  
> -		counters_inc(mbstat, MBSTAT_PULLDOWN_ALLOC);
> +		mbstat_inc(mbs_pulldown_alloc);
>  		o = m_dup1(n, off, n->m_len - off, M_DONTWAIT);
>  		if (o == NULL) {
>  			m_freem(m);
> @@ -160,7 +160,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  	 */
>  	if ((off == 0 || offp) && m_trailingspace(n) >= tlen &&
>  	    !sharedcluster) {
> -		counters_inc(mbstat, MBSTAT_PULLDOWN_COPY);
> +		mbstat_inc(mbs_pulldown_copy);
>  		m_copydata(n->m_next, 0, tlen, mtod(n, caddr_t) + n->m_len);
>  		n->m_len += tlen;
>  		m_adj(n->m_next, tlen);
> @@ -170,7 +170,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  	    !sharedcluster && n->m_next->m_len >= tlen) {
>  		n->m_next->m_data -= hlen;
>  		n->m_next->m_len += hlen;
> -		counters_inc(mbstat, MBSTAT_PULLDOWN_COPY);
> +		mbstat_inc(mbs_pulldown_copy);
>  		memcpy(mtod(n->m_next, caddr_t), mtod(n, caddr_t) + off, hlen);
>  		n->m_len -= hlen;
>  		n = n->m_next;
> @@ -186,7 +186,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  		m_freem(m);
>  		return (NULL);
>  	}
> -	counters_inc(mbstat, MBSTAT_PULLDOWN_ALLOC);
> +	mbstat_inc(mbs_pulldown_alloc);
>  	MGET(o, M_DONTWAIT, m->m_type);
>  	if (o && len > MLEN) {
>  		MCLGETL(o, M_DONTWAIT, len);
> Index: sys/sys/mbuf.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
> diff -u -p -r1.265 mbuf.h
> --- sys/sys/mbuf.h	5 Nov 2024 13:15:13 -0000	1.265
> +++ sys/sys/mbuf.h	20 Jun 2025 20:56:56 -0000
> @@ -361,17 +361,18 @@ u_int mextfree_register(void (*)(caddr_t
>  /* length to m_copy to copy all */
>  #define	M_COPYALL	1000000000
>  
> -#define MBSTAT_TYPES		MT_NTYPES
> -#define MBSTAT_DROPS		(MBSTAT_TYPES + 0)
> -#define MBSTAT_WAIT		(MBSTAT_TYPES + 1)
> -#define MBSTAT_DRAIN		(MBSTAT_TYPES + 2)
> -#define MBSTAT_DEFRAG_ALLOC	(MBSTAT_TYPES + 3)
> -#define MBSTAT_PREPEND_ALLOC	(MBSTAT_TYPES + 4)
> -#define MBSTAT_PULLUP_ALLOC	(MBSTAT_TYPES + 5)
> -#define MBSTAT_PULLUP_COPY	(MBSTAT_TYPES + 6)
> -#define MBSTAT_PULLDOWN_ALLOC	(MBSTAT_TYPES + 7)
> -#define MBSTAT_PULLDOWN_COPY	(MBSTAT_TYPES + 8)
> -#define MBSTAT_COUNT		(MBSTAT_TYPES + 9)
> +enum mbstat_counters {
> +	mbs_drops = MT_NTYPES,
> +	mbs_wait,
> +	mbs_drain,
> +	mbs_defrag_alloc,
> +	mbs_prepend_alloc,
> +	mbs_pullup_alloc,
> +	mbs_pullup_copy,
> +	mbs_pulldown_alloc,
> +	mbs_pulldown_copy,
> +	mbs_ncounters
> +};
>  
>  /*
>   * Mbuf statistics.
> @@ -379,11 +380,10 @@ u_int mextfree_register(void (*)(caddr_t
>   * pool headers (mbpool and mclpool).
>   */
>  struct mbstat {
> -	u_long	m_drops;	/* times failed to find space */
> -	u_long	m_wait;		/* times waited for space */
> -	u_long	m_drain;	/* times drained protocols for space */
> -	u_long	m_mtypes[MBSTAT_TYPES];
> -				/* type specific mbuf allocations */
> +	u_long	m_drops;		/* times failed to find space */
> +	u_long	m_wait;			/* times waited for space */
> +	u_long	m_drain;		/* times drained protocols for space */
> +	u_long	m_mtypes[MT_NTYPES];	/* type specific mbuf allocations */
>  	u_long	m_defrag_alloc;
>  	u_long	m_prepend_alloc;
>  	u_long	m_pullup_alloc;
> @@ -464,6 +464,16 @@ m_freemp(struct mbuf **mp)
>  
>  	*mp = NULL;
>  	return m_freem(m);
> +}
> +
> +#include <sys/percpu.h>
> +
> +static inline void
> +mbstat_inc(enum mbstat_counters c)
> +{
> +	int s = splnet();
> +	counters_inc(mbstat, c);
> +	splx(s);
>  }
>  
>  /* Packet tag routines */
> Index: usr.bin/netstat/mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/netstat/mbuf.c,v
> diff -u -p -r1.46 mbuf.c
> --- usr.bin/netstat/mbuf.c	29 Aug 2024 10:44:40 -0000	1.46
> +++ usr.bin/netstat/mbuf.c	20 Jun 2025 20:56:36 -0000
> @@ -79,7 +79,7 @@ static struct mbtypes {
>  };
>  
>  int nmbtypes = sizeof(mbstat.m_mtypes) / sizeof(u_long);
> -bool seen[MBSTAT_TYPES];		/* "have we seen this type yet?" */
> +bool seen[MT_NTYPES];		/* "have we seen this type yet?" */
>  
>  /*
>   * Print mbuf statistics.
> @@ -93,7 +93,7 @@ mbpr(void)
>  	struct mbtypes *mp;
>  	size_t size;
>  
> -	if (nmbtypes != MBSTAT_TYPES) {
> +	if (nmbtypes != MT_NTYPES) {
>  		fprintf(stderr,
>  		    "%s: unexpected change to mbstat; check source\n",
>  		    __progname);
>