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_ */