From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: btrace netlock
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 24 Jun 2025 22:03:53 +0300

> Or should I put it in rw_enter_write() to have generic lock analysis?
> We have something similar in refcnt.
> 

The generic analysis looks better.

On Tue, Jun 24, 2025 at 08:50:46PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> I would like to have a trace point in net lock to find the hot spots
> that have not been unlocked yet.  Diff below produces output like
> this.
> 
> @[
> nd6_timer+0x130
> softclock_thread+0x11b
> proc_trampoline+0x10
> kernel
> ]: 475
> @[
> igmp_slowtimo+0x89
> pfslowtimo+0x107
> softclock_thread+0x11b
> proc_trampoline+0x10
> kernel
> ]: 18
> ...
> 
> What I don't like is adding more includes into systm.h.  If we agree
> on the feature, I can try to shuffle the declarations around.
> 
> Or should I put it in rw_enter_write() to have generic lock analysis?
> We have something similar in refcnt.
> 
> bluhm
> 
> Index: sys/dev/dt/dt_prov_static.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
> diff -u -p -r1.25 dt_prov_static.c
> --- sys/dev/dt/dt_prov_static.c	10 Mar 2025 09:28:56 -0000	1.25
> +++ sys/dev/dt/dt_prov_static.c	24 Jun 2025 14:10:12 -0000
> @@ -94,6 +94,11 @@ DT_STATIC_PROBE0(smr, wakeup);
>  DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
>  
>  /*
> + * Network
> + */
> +DT_STATIC_PROBE0(net, lock);
> +
> +/*
>   * reference counting, keep in sync with sys/refcnt.h
>   */
>  DT_STATIC_PROBE0(refcnt, none);
> @@ -151,6 +156,8 @@ struct dt_probe *const dtps_static[] = {
>  	&_DT_STATIC_P(smr, barrier_exit),
>  	&_DT_STATIC_P(smr, wakeup),
>  	&_DT_STATIC_P(smr, thread),
> +	/* Network */
> +	&_DT_STATIC_P(net, lock),
>  	/* refcnt */
>  	&_DT_STATIC_P(refcnt, none),
>  	&_DT_STATIC_P(refcnt, ethmulti),
> Index: sys/sys/systm.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/systm.h,v
> diff -u -p -r1.176 systm.h
> --- sys/sys/systm.h	19 Jun 2025 12:01:08 -0000	1.176
> +++ sys/sys/systm.h	24 Jun 2025 18:40:22 -0000
> @@ -325,6 +325,9 @@ int	uiomove(void *, size_t, struct uio *
>  #if defined(_KERNEL)
>  
>  #include <sys/rwlock.h>
> +#include <sys/clockintr.h>
> +#include <sys/syslimits.h>
> +#include <sys/tracepoint.h>
>  
>  extern struct rwlock netlock;
>  
> @@ -333,8 +336,12 @@ extern struct rwlock netlock;
>   * by the NET_LOCK().  It's a single non-recursive lock for the whole
>   * subsystem.
>   */
> -#define	NET_LOCK()	do { rw_enter_write(&netlock); } while (0)
> -#define	NET_UNLOCK()	do { rw_exit_write(&netlock); } while (0)
> +#define NET_LOCK()							\
> +do {									\
> +	TRACEPOINT(net, lock, NULL);					\
> +	rw_enter_write(&netlock);					\
> +} while (0)
> +#define NET_UNLOCK()	do { rw_exit_write(&netlock); } while (0)
>  
>  /*
>   * Reader version of NET_LOCK().
> Index: share/btrace/netlock.bt
> ===================================================================
> RCS file: share/btrace/netlock.bt
> diff -N share/btrace/netlock.bt
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ share/btrace/netlock.bt	24 Jun 2025 18:34:38 -0000
> @@ -0,0 +1,12 @@
> +/*	$OpenBSD$	*/
> +
> +/*
> + * netlock.bt	Stack sampling of exclusive net lock.
> + *
> + * To produce a FlameGraph process the output with stackcollapse-bpftrace.pl
> + * and flamegraph.pl found in:
> + *	https://github.com/brendangregg/FlameGraph
> + */
> +tracepoint:net:lock {
> +	@[kstack] = count();
> +}
>