Module Name:    src
Committed By:   riastradh
Date:           Sun Feb 27 14:16:12 UTC 2022

Modified Files:
        src/external/cddl/osnet/dev/lockstat: lockstat.c
        src/sys/dev: lockstat.c lockstat.h

Log Message:
lockstat(4): Membar audit.

- Serialize updates to lockstat_enabled, lockstat_dev_enabled, and
  lockstat_dtrace_enabled with a new __cpu_simple_lock.

- Use xc_barrier to obviate any need for additional membars in
  lockstat_event.

- Use atomic_load/store_* for access that might not be serialized by
  lockstat_lock or lockstat_enabled_lock.


To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 src/external/cddl/osnet/dev/lockstat/lockstat.c
cvs rdiff -u -r1.27 -r1.28 src/sys/dev/lockstat.c
cvs rdiff -u -r1.14 -r1.15 src/sys/dev/lockstat.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/external/cddl/osnet/dev/lockstat/lockstat.c
diff -u src/external/cddl/osnet/dev/lockstat/lockstat.c:1.10 src/external/cddl/osnet/dev/lockstat/lockstat.c:1.11
--- src/external/cddl/osnet/dev/lockstat/lockstat.c:1.10	Tue Feb 12 14:31:45 2019
+++ src/external/cddl/osnet/dev/lockstat/lockstat.c	Sun Feb 27 14:16:12 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: lockstat.c,v 1.10 2019/02/12 14:31:45 rin Exp $	*/
+/*	$NetBSD: lockstat.c,v 1.11 2022/02/27 14:16:12 riastradh Exp $	*/
 
 /*
  * CDDL HEADER START
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lockstat.c,v 1.10 2019/02/12 14:31:45 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lockstat.c,v 1.11 2022/02/27 14:16:12 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/proc.h>
@@ -72,11 +72,13 @@ lockstat_enable(void *arg, dtrace_id_t i
 
 	ASSERT(!lockstat_probemap[LS_COMPRESS(probe->lsp_probe)]);
 
-	lockstat_probemap[LS_COMPRESS(probe->lsp_probe)] = id;
 	if (lockstat_dtrace_count++ == 0) {
+		LOCKSTAT_ENABLED_UPDATE_BEGIN();
 		lockstat_dtrace_enabled = LB_DTRACE;
-		LOCKSTAT_ENABLED_UPDATE();
+		LOCKSTAT_ENABLED_UPDATE_END();
 	}
+	atomic_store_relaxed(&lockstat_probemap[LS_COMPRESS(probe->lsp_probe)],
+	    id);
 
 	return 0;
 }
@@ -89,12 +91,19 @@ lockstat_disable(void *arg, dtrace_id_t 
 
 	ASSERT(lockstat_probemap[LS_COMPRESS(probe->lsp_probe)]);
 
+	atomic_store_relaxed(&lockstat_probemap[LS_COMPRESS(probe->lsp_probe)],
+	    0);
 	if (--lockstat_dtrace_count == 0) {
+		LOCKSTAT_ENABLED_UPDATE_BEGIN();
 		lockstat_dtrace_enabled = 0;
-		LOCKSTAT_ENABLED_UPDATE();
-	}
+		LOCKSTAT_ENABLED_UPDATE_END();
 
-	lockstat_probemap[LS_COMPRESS(probe->lsp_probe)] = 0;
+		/*
+		 * Wait for all lockstat dtrace probe on all CPUs to
+		 * finish, now that they've been disabled.
+		 */
+		xc_barrier(0);
+	}
 }
 
 /*ARGSUSED*/
@@ -149,13 +158,6 @@ static dtrace_pops_t lockstat_pops = {
 	lockstat_destroy
 };
 
-static void
-lockstat_barrier_xc(void *arg0 __unused, void *arg1 __unused)
-{
-
-	membar_consumer();
-}
-
 typedef void (*dtrace_probe_func_t)(dtrace_id_t, uintptr_t, uintptr_t,
     uintptr_t, uintptr_t, uintptr_t);
 
@@ -169,8 +171,14 @@ lockstat_cas_probe(dtrace_probe_func_t o
 		return false;
 
 	lockstat_probe_func = new;
-	membar_producer();
-	xc_wait(xc_broadcast(0, lockstat_barrier_xc, NULL, NULL));
+
+	/*
+	 * Make sure that the probe function is initialized on all CPUs
+	 * before we enable the lockstat probe by setting
+	 * lockstat_probemap[...].
+	 */
+	xc_barrier(0);
+
 	return true;
 }
 

Index: src/sys/dev/lockstat.c
diff -u src/sys/dev/lockstat.c:1.27 src/sys/dev/lockstat.c:1.28
--- src/sys/dev/lockstat.c:1.27	Sat May 23 23:42:42 2020
+++ src/sys/dev/lockstat.c	Sun Feb 27 14:16:12 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: lockstat.c,v 1.27 2020/05/23 23:42:42 ad Exp $	*/
+/*	$NetBSD: lockstat.c,v 1.28 2022/02/27 14:16:12 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2006, 2007, 2019 The NetBSD Foundation, Inc.
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: lockstat.c,v 1.27 2020/05/23 23:42:42 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: lockstat.c,v 1.28 2022/02/27 14:16:12 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: lockstat.c,v
 #include <sys/cpu.h>
 #include <sys/syslog.h>
 #include <sys/atomic.h>
+#include <sys/xcall.h>
 
 #include <dev/lockstat.h>
 
@@ -101,6 +102,7 @@ dev_type_ioctl(lockstat_ioctl);
 
 volatile u_int	lockstat_enabled;
 volatile u_int	lockstat_dev_enabled;
+__cpu_simple_lock_t lockstat_enabled_lock;
 uintptr_t	lockstat_csstart;
 uintptr_t	lockstat_csend;
 uintptr_t	lockstat_csmask;
@@ -154,6 +156,7 @@ lockstatattach(int nunits)
 	(void)nunits;
 
 	__cpu_simple_lock_init(&lockstat_lock);
+	__cpu_simple_lock_init(&lockstat_enabled_lock);
 }
 
 /*
@@ -235,10 +238,26 @@ lockstat_start(lsenable_t *le)
 	lockstat_lockstart = le->le_lockstart;
 	lockstat_lockstart = le->le_lockstart;
 	lockstat_lockend = le->le_lockend;
-	membar_sync();
+
+	/*
+	 * Ensure everything is initialized on all CPUs, by issuing a
+	 * null xcall with the side effect of a release barrier on this
+	 * CPU and an acquire barrier on all other CPUs, before they
+	 * can witness any flags set in lockstat_dev_enabled -- this
+	 * way we don't need to add any barriers in lockstat_event.
+	 */
+	xc_barrier(0);
+
+	/*
+	 * Start timing after the xcall, so we don't spuriously count
+	 * xcall communication time, but before flipping the switch, so
+	 * we don't dirty sample with locks taken in the timecounter.
+	 */
 	getnanotime(&lockstat_stime);
-	lockstat_dev_enabled = le->le_mask;
-	LOCKSTAT_ENABLED_UPDATE();
+
+	LOCKSTAT_ENABLED_UPDATE_BEGIN();
+	atomic_store_relaxed(&lockstat_dev_enabled, le->le_mask);
+	LOCKSTAT_ENABLED_UPDATE_END();
 }
 
 /*
@@ -258,13 +277,13 @@ lockstat_stop(lsdisable_t *ld)
 	KASSERT(lockstat_dev_enabled);
 
 	/*
-	 * Set enabled false, force a write barrier, and wait for other CPUs
-	 * to exit lockstat_event().
+	 * Disable and wait for other CPUs to exit lockstat_event().
 	 */
-	lockstat_dev_enabled = 0;
-	LOCKSTAT_ENABLED_UPDATE();
+	LOCKSTAT_ENABLED_UPDATE_BEGIN();
+	atomic_store_relaxed(&lockstat_dev_enabled, 0);
+	LOCKSTAT_ENABLED_UPDATE_END();
 	getnanotime(&ts);
-	tsleep(&lockstat_stop, PPAUSE, "lockstat", mstohz(10));
+	xc_barrier(0);
 
 	/*
 	 * Did we run out of buffers while tracing?
@@ -370,12 +389,14 @@ lockstat_event(uintptr_t lock, uintptr_t
 #ifdef KDTRACE_HOOKS
 	uint32_t id;
 	CTASSERT((LS_NPROBES & (LS_NPROBES - 1)) == 0);
-	if ((id = lockstat_probemap[LS_COMPRESS(flags)]) != 0)
+	if ((id = atomic_load_relaxed(&lockstat_probemap[LS_COMPRESS(flags)]))
+	    != 0)
 		(*lockstat_probe_func)(id, lock, callsite, flags, count,
 		    cycles);
 #endif
 
-	if ((flags & lockstat_dev_enabled) != flags || count == 0)
+	if ((flags & atomic_load_relaxed(&lockstat_dev_enabled)) != flags ||
+	    count == 0)
 		return;
 	if (lock < lockstat_lockstart || lock > lockstat_lockend)
 		return;
@@ -487,7 +508,7 @@ lockstat_ioctl(dev_t dev, u_long cmd, vo
 			error = ENODEV;
 			break;
 		}
-		if (lockstat_dev_enabled) {
+		if (atomic_load_relaxed(&lockstat_dev_enabled)) {
 			error = EBUSY;
 			break;
 		}
@@ -524,7 +545,7 @@ lockstat_ioctl(dev_t dev, u_long cmd, vo
 		break;
 
 	case IOC_LOCKSTAT_DISABLE:
-		if (!lockstat_dev_enabled)
+		if (!atomic_load_relaxed(&lockstat_dev_enabled))
 			error = EINVAL;
 		else
 			error = lockstat_stop((lsdisable_t *)data);

Index: src/sys/dev/lockstat.h
diff -u src/sys/dev/lockstat.h:1.14 src/sys/dev/lockstat.h:1.15
--- src/sys/dev/lockstat.h:1.14	Sun Jan 24 01:01:11 2016
+++ src/sys/dev/lockstat.h	Sun Feb 27 14:16:12 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: lockstat.h,v 1.14 2016/01/24 01:01:11 christos Exp $	*/
+/*	$NetBSD: lockstat.h,v 1.15 2022/02/27 14:16:12 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2006 The NetBSD Foundation, Inc.
@@ -38,7 +38,9 @@
 #endif
 
 #include <sys/types.h>
+
 #include <sys/ioccom.h>
+#include <sys/lock.h>
 #include <sys/queue.h>
 #include <sys/time.h>
 
@@ -159,7 +161,7 @@ do {									\
 #define	LOCKSTAT_TIMER(name)	uint64_t name = 0
 #define	LOCKSTAT_COUNTER(name)	uint64_t name = 0
 #define	LOCKSTAT_FLAG(name)	int name
-#define	LOCKSTAT_ENTER(name)	name = lockstat_enabled
+#define	LOCKSTAT_ENTER(name)	name = atomic_load_relaxed(&lockstat_enabled)
 #define	LOCKSTAT_EXIT(name)
 
 #define	LOCKSTAT_START_TIMER(flag, name)				\
@@ -214,13 +216,22 @@ void		lockstat_probe_stub(uint32_t, uint
 #endif
 
 #if defined(_KERNEL) && NLOCKSTAT > 0
+extern __cpu_simple_lock_t lockstat_enabled_lock;
 extern volatile u_int	lockstat_enabled;
 extern volatile u_int	lockstat_dev_enabled;
 
-#define LOCKSTAT_ENABLED_UPDATE() do { \
-	lockstat_enabled = lockstat_dev_enabled | KDTRACE_LOCKSTAT_ENABLED; \
-	membar_producer(); \
-    } while (/*CONSTCOND*/0)
+#define LOCKSTAT_ENABLED_UPDATE_BEGIN() do				    \
+{									    \
+	__cpu_simple_lock(&lockstat_enabled_lock);			    \
+} while (/*CONSTCOND*/0)
+
+#define LOCKSTAT_ENABLED_UPDATE_END() do				    \
+{									    \
+	atomic_store_relaxed(&lockstat_enabled,				    \
+	    lockstat_dev_enabled | KDTRACE_LOCKSTAT_ENABLED);		    \
+	__cpu_simple_unlock(&lockstat_enabled_lock);			    \
+} while (/*CONSTCOND*/0)
+
 #endif
 
 #endif	/* _SYS_LOCKSTAT_H_ */

Reply via email to