Download raw body.
sysctl(2): unlock KERN_SPLASSERT
> On 30 Dec 2024, at 02:00, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
>> Date: Sun, 29 Dec 2024 20:43:40 +0300
>> From: Vitaliy Makkoveev <mvs@openbsd.org>
>>
>> `splassert_ctl' is atomically accessed integer which accessed lockless
>> for a long time.
>
> Sorry, but no, I don't think this is ok. This solves nothing, but it
> does add an extra include to <machine/intr.h> that increases the risk
> that folks start failing to explicitly include <sys/atomic.h> in
> places where it is needed.
>
sys/atomic.h has #ifndef _SYS_ATOMIC_H_ guard against it.
>> Index: sys/arch/alpha/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/alpha/include/intr.h,v
>> diff -u -p -r1.50 intr.h
>> --- sys/arch/alpha/include/intr.h 17 May 2024 20:07:33 -0000 1.50
>> +++ sys/arch/alpha/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -147,6 +147,9 @@ void intr_barrier(void *);
>>
>> /* SPL asserts */
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -156,7 +159,7 @@ extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) \
>> do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/amd64/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/amd64/include/intr.h,v
>> diff -u -p -r1.34 intr.h
>> --- sys/arch/amd64/include/intr.h 26 May 2024 13:37:31 -0000 1.34
>> +++ sys/arch/amd64/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -152,6 +152,9 @@ void softintr(int);
>>
>> /* SPL asserts */
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -160,7 +163,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/arm64/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/arm64/include/intr.h,v
>> diff -u -p -r1.23 intr.h
>> --- sys/arch/arm64/include/intr.h 14 Oct 2024 10:08:13 -0000 1.23
>> +++ sys/arch/arm64/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -199,6 +199,9 @@ extern void (*intr_send_ipi_func)(struct
>> #define ARM_IPI_HALT 2
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -207,7 +210,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void arm_splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> arm_splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/armv7/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/armv7/include/intr.h,v
>> diff -u -p -r1.15 intr.h
>> --- sys/arch/armv7/include/intr.h 14 Oct 2024 10:08:13 -0000 1.15
>> +++ sys/arch/armv7/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -202,6 +202,9 @@ extern void (*intr_send_ipi_func)(struct
>> #define ARM_IPI_DDB 1
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -210,8 +213,8 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void arm_splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> - arm_splassert_check(__wantipl, __func__); \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> + arm_splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> #define splsoftassert(wantipl) splassert(wantipl)
>> Index: sys/arch/hppa/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/hppa/include/intr.h,v
>> diff -u -p -r1.44 intr.h
>> --- sys/arch/hppa/include/intr.h 13 Jan 2018 15:18:11 -0000 1.44
>> +++ sys/arch/hppa/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -71,11 +71,14 @@
>> extern volatile u_long imask[NIPL];
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> void splassert_fail(int, int, const char *);
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/i386/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/i386/include/intr.h,v
>> diff -u -p -r1.50 intr.h
>> --- sys/arch/i386/include/intr.h 26 May 2024 13:37:32 -0000 1.50
>> +++ sys/arch/i386/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -64,6 +64,9 @@ extern void softintr(int);
>>
>> /* SPL asserts */
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -72,7 +75,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/landisk/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/landisk/include/intr.h,v
>> diff -u -p -r1.14 intr.h
>> --- sys/arch/landisk/include/intr.h 20 Aug 2018 15:02:07 -0000 1.14
>> +++ sys/arch/landisk/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -66,6 +66,9 @@
>> #define splx(x) _cpu_intr_resume(x)
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -75,7 +78,7 @@ extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) \
>> do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/loongson/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/loongson/include/intr.h,v
>> diff -u -p -r1.18 intr.h
>> --- sys/arch/loongson/include/intr.h 5 Sep 2019 05:31:38 -0000 1.18
>> +++ sys/arch/loongson/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -129,6 +129,9 @@ void softintr_schedule(void *);
>> void splinit(void);
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -137,7 +140,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/m88k/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/m88k/include/intr.h,v
>> diff -u -p -r1.14 intr.h
>> --- sys/arch/m88k/include/intr.h 20 Aug 2018 15:02:07 -0000 1.14
>> +++ sys/arch/m88k/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -72,6 +72,9 @@ int spl0(void);
>>
>> /* SPL asserts */
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -80,7 +83,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/octeon/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/octeon/include/intr.h,v
>> diff -u -p -r1.22 intr.h
>> --- sys/arch/octeon/include/intr.h 5 Sep 2019 05:31:38 -0000 1.22
>> +++ sys/arch/octeon/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -128,6 +128,9 @@ void softintr_schedule(void *);
>> void splinit(void);
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -136,7 +139,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/powerpc/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/powerpc/include/intr.h,v
>> diff -u -p -r1.56 intr.h
>> --- sys/arch/powerpc/include/intr.h 20 Aug 2018 15:02:07 -0000 1.56
>> +++ sys/arch/powerpc/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -90,6 +90,9 @@ void do_pending_int(void);
>>
>> /* SPL asserts */
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -98,7 +101,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/powerpc64/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/powerpc64/include/intr.h,v
>> diff -u -p -r1.14 intr.h
>> --- sys/arch/powerpc64/include/intr.h 30 May 2021 15:06:53 -0000 1.14
>> +++ sys/arch/powerpc64/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -65,6 +65,9 @@ void splx(int);
>> #define splhigh() splraise(IPL_HIGH)
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -73,7 +76,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/riscv64/include/intr.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/riscv64/include/intr.h,v
>> diff -u -p -r1.8 intr.h
>> --- sys/arch/riscv64/include/intr.h 16 Oct 2024 02:32:27 -0000 1.8
>> +++ sys/arch/riscv64/include/intr.h 29 Dec 2024 17:16:16 -0000
>> @@ -207,6 +207,9 @@ void riscv_intr_cpu_enable(void);
>> void intr_send_ipi(struct cpu_info *, int);
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -215,7 +218,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void riscv_splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> riscv_splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/arch/sparc64/include/psl.h
>> ===================================================================
>> RCS file: /cvs/src/sys/arch/sparc64/include/psl.h,v
>> diff -u -p -r1.37 psl.h
>> --- sys/arch/sparc64/include/psl.h 6 Nov 2024 07:11:14 -0000 1.37
>> +++ sys/arch/sparc64/include/psl.h 29 Dec 2024 17:16:16 -0000
>> @@ -205,6 +205,9 @@
>> #if defined(_KERNEL) && !defined(_LOCORE)
>>
>> #ifdef DIAGNOSTIC
>> +
>> +#include <sys/atomic.h>
>> +
>> /*
>> * Although this function is implemented in MI code, it must be in this MD
>> * header because we don't want this header to include MI includes.
>> @@ -213,7 +216,7 @@ void splassert_fail(int, int, const char
>> extern int splassert_ctl;
>> void splassert_check(int, const char *);
>> #define splassert(__wantipl) do { \
>> - if (splassert_ctl > 0) { \
>> + if (atomic_load_int(&splassert_ctl) > 0) { \
>> splassert_check(__wantipl, __func__); \
>> } \
>> } while (0)
>> Index: sys/dev/ic/iosf.c
>> ===================================================================
>> RCS file: /cvs/src/sys/dev/ic/iosf.c,v
>> diff -u -p -r1.2 iosf.c
>> --- sys/dev/ic/iosf.c 26 Jun 2024 01:40:49 -0000 1.2
>> +++ sys/dev/ic/iosf.c 29 Dec 2024 17:16:16 -0000
>> @@ -258,7 +258,7 @@ iosf_mbi_assert_punit_acquired(void)
>> {
>> int s;
>>
>> - if (splassert_ctl == 0)
>> + if (atomic_load_int(&splassert_ctl) == 0)
>> return;
>>
>> s = rw_status(&iosf_lock);
>> Index: sys/kern/kern_sysctl.c
>> ===================================================================
>> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
>> diff -u -p -r1.459 kern_sysctl.c
>> --- sys/kern/kern_sysctl.c 28 Dec 2024 20:34:05 -0000 1.459
>> +++ sys/kern/kern_sysctl.c 29 Dec 2024 17:16:17 -0000
>> @@ -614,6 +614,7 @@ kern_sysctl(int *name, u_int namelen, vo
>> case KERN_FSCALE:
>> case KERN_CCPU:
>> case KERN_NPROCS:
>> + case KERN_SPLASSERT:
>> case KERN_WXABORT:
>> case KERN_NETLIVELOCKS:
>> case KERN_GLOBAL_PTRACE:
>> Index: sys/kern/subr_prf.c
>> ===================================================================
>> RCS file: /cvs/src/sys/kern/subr_prf.c,v
>> diff -u -p -r1.106 subr_prf.c
>> --- sys/kern/subr_prf.c 14 Aug 2022 01:58:28 -0000 1.106
>> +++ sys/kern/subr_prf.c 29 Dec 2024 17:16:17 -0000
>> @@ -89,6 +89,11 @@ struct mutex kprintf_mutex =
>> MUTEX_INITIALIZER_FLAGS(IPL_HIGH, "kprintf", MTX_NOWITNESS);
>>
>> /*
>> + * Locks used to protect data:
>> + * a atomic
>> + */
>> +
>> +/*
>> * globals
>> */
>>
>> @@ -119,9 +124,9 @@ int db_console = 0;
>> * panic on spl assertion failure?
>> */
>> #ifdef SPLASSERT_WATCH
>> -int splassert_ctl = 3;
>> +int splassert_ctl = 3; /* [a] */
>> #else
>> -int splassert_ctl = 1;
>> +int splassert_ctl = 1; /* [a] */
>> #endif
>>
>> /*
>> @@ -199,7 +204,7 @@ panic(const char *fmt, ...)
>> bootopt |= RB_NOSYNC;
>>
>> /* do not trigger assertions, we know that we are inconsistent */
>> - splassert_ctl = 0;
>> + atomic_store_int(&splassert_ctl, 0);
>>
>> #ifdef BOOT_QUIET
>> printf_flags |= TOCONS; /* make sure we see kernel printf output */
>> @@ -243,7 +248,7 @@ splassert_fail(int wantipl, int haveipl,
>> return;
>>
>> printf("splassert: %s: want %d have %d\n", func, wantipl, haveipl);
>> - switch (splassert_ctl) {
>> + switch (atomic_load_int(&splassert_ctl)) {
>> case 1:
>> break;
>> case 2:
>> Index: sys/kern/uipc_socket2.c
>> ===================================================================
>> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
>> diff -u -p -r1.161 uipc_socket2.c
>> --- sys/kern/uipc_socket2.c 26 Dec 2024 12:16:17 -0000 1.161
>> +++ sys/kern/uipc_socket2.c 29 Dec 2024 17:16:17 -0000
>> @@ -460,7 +460,7 @@ soassertlocked(struct socket *so)
>> if (rw_status(&netlock) == RW_READ) {
>> NET_ASSERT_LOCKED();
>>
>> - if (splassert_ctl > 0 &&
>> + if (atomic_load_int(&splassert_ctl) > 0 &&
>> rw_status(&so->so_lock) != RW_WRITE)
>> splassert_fail(0, RW_WRITE, __func__);
>> } else
>> @@ -503,7 +503,8 @@ void
>> sbmtxassertlocked(struct socket *so, struct sockbuf *sb)
>> {
>> if (sb->sb_flags & SB_MTXLOCK) {
>> - if (splassert_ctl > 0 && mtx_owned(&sb->sb_mtx) == 0)
>> + if (atomic_load_int(&splassert_ctl) > 0 &&
>> + mtx_owned(&sb->sb_mtx) == 0)
>> splassert_fail(0, RW_WRITE, __func__);
>> } else
>> soassertlocked(so);
>> Index: sys/sys/systm.h
>> ===================================================================
>> RCS file: /cvs/src/sys/sys/systm.h,v
>> diff -u -p -r1.171 systm.h
>> --- sys/sys/systm.h 28 May 2024 12:50:23 -0000 1.171
>> +++ sys/sys/systm.h 29 Dec 2024 17:16:17 -0000
>> @@ -347,21 +347,22 @@ extern struct rwlock netlock;
>> #define NET_ASSERT_UNLOCKED() \
>> do { \
>> int _s = rw_status(&netlock); \
>> - if ((splassert_ctl > 0) && (_s == RW_WRITE)) \
>> + if ((atomic_load_int(&splassert_ctl) > 0) && (_s == RW_WRITE)) \
>> splassert_fail(0, RW_WRITE, __func__); \
>> } while (0)
>>
>> #define NET_ASSERT_LOCKED() \
>> do { \
>> int _s = rw_status(&netlock); \
>> - if ((splassert_ctl > 0) && (_s != RW_WRITE && _s != RW_READ)) \
>> + if ((atomic_load_int(&splassert_ctl) > 0) && \
>> + (_s != RW_WRITE && _s != RW_READ)) \
>> splassert_fail(RW_READ, _s, __func__); \
>> } while (0)
>>
>> #define NET_ASSERT_LOCKED_EXCLUSIVE() \
>> do { \
>> int _s = rw_status(&netlock); \
>> - if ((splassert_ctl > 0) && (_s != RW_WRITE)) \
>> + if ((atomic_load_int(&splassert_ctl) > 0) && (_s != RW_WRITE)) \
>> splassert_fail(RW_WRITE, _s, __func__); \
>> } while (0)
>>
>>
>>
>
sysctl(2): unlock KERN_SPLASSERT