Download raw body.
sysctl int atomic
> Date: Tue, 30 Apr 2024 13:08:55 +0200
> From: Alexander Bluhm <bluhm@openbsd.org>
>
> On Fri, Apr 26, 2024 at 10:49:16PM +0200, Mark Kettenis wrote:
> > > Date: Fri, 26 Apr 2024 18:13:03 +0200
> > > From: Alexander Bluhm <bluhm@openbsd.org>
> > >
> > > Hi,
> > >
> > > Interger load and store are atomic, so it is not necessary to hold
> > > a lock when accessing them from sysctl(2) system call. I added
> > > some read and write once macros and memory barriers to the sysctl
> > > implementation. The sysctl_lock will keep before -> after output
> > > of sysctl(8) consistent.
> > >
> > > Most of the diff is mechanical. sysctl_rdint() has a pointer
> > > parameter and all callers have to adapt. If we want to go this
> > > way, I will compile test more architectures.
> > >
> > > The sysctl_int_...() functions don't call each other anymore, but
> > > are kept very simialar. When comparing you see the differences.
> > >
> > > After this diff has been commited, the code that uses the sysctl
> > > variables can be checked if lockless accces is possible. Then a
> > > bunch of net lock in sysctl(2) can go away.
> > >
> > > ok?
> >
> > Sorry, but I don't think this is sensible. You've added a
> > membar_consumer() in sysctl_rdint(), but where is the matching
> > membar_producer()? Or in other words what are you ordering against?
>
> You are right, the membars are not necessary. I removed them.
>
> > I suspect in most cases, no ordering is actually required. And in the
> > few cases where it might matter, you're probably better off making
> > sure a consistent value is passed to sysctl_rdint() by using the
> > appropriate memory barriers, atomic operations or mutex in the caller.
>
> Atomic read or write operations on intergers cost nothing. They
> are basically a volatile cast to avoid compiler optimiations.
> Presumably the compiler does not optimize anything, but I want to
> be sure.
That isn't true. The volatile cast forces a memory access to be made.
Whereas in the current code the integer is passed by value, so in a
register on any architecture that isn't retarded...
Of course the value has to be read from memory at some point, and
there may not be all that much opportunity for the compiler to
optimize this.
> Usually I use READ_ONCE and WRITE_ONCE to mark that access is MP
> safe. And interger sysctl is MP safe with this diff. If more
> locking around sysctl_int_...() is needed, it can be kept. But all
> the other locks can be removed step by step in following diffs.
>
> I prefer to make the underlying functions MP safe instead of caring
> about all the callers.
>
> ok?
Still not entirely convinced this is a smart move. For sysctl_rdint()
the change is probably fine and not a significant pessimization.
Except in the cases where you now have to store a value on the stack
just to be able to pass an address... And I don't think the same
pattern is going to work for the non-readonly sysctls.
> Index: arch/alpha/alpha/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/alpha/alpha/machdep.c,v
> diff -u -p -r1.203 machdep.c
> --- arch/alpha/alpha/machdep.c 11 Apr 2023 00:45:06 -0000 1.203
> +++ arch/alpha/alpha/machdep.c 23 Apr 2024 01:12:41 -0000
> @@ -1506,6 +1506,9 @@ cpu_sysctl(int *name, u_int namelen, voi
> size_t newlen, struct proc *p)
> {
> dev_t consdev;
> +#ifndef APERTURE
> + int val;
> +#endif
> #if NIOASIC > 0
> int oldval, ret;
> #endif
> @@ -1546,7 +1549,8 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &allowaperture));
> #else
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #endif
> #if NIOASIC > 0
> case CPU_LED_BLINK:
> Index: arch/alpha/pci/pci_machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/alpha/pci/pci_machdep.c,v
> diff -u -p -r1.21 pci_machdep.c
> --- arch/alpha/pci/pci_machdep.c 8 Sep 2017 05:36:51 -0000 1.21
> +++ arch/alpha/pci/pci_machdep.c 23 Apr 2024 01:13:13 -0000
> @@ -122,7 +122,7 @@ alpha_sysctl_chipset(int *name, u_int na
> alpha_pci_chipset->pc_name));
> case CPU_CHIPSET_BWX:
> return (sysctl_rdint(where, sizep, NULL,
> - alpha_pci_chipset->pc_bwx));
> + &alpha_pci_chipset->pc_bwx));
> case CPU_CHIPSET_MEM:
> return (sysctl_rdquad(where, sizep, NULL,
> alpha_pci_chipset->pc_mem));
> Index: arch/amd64/amd64/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/machdep.c,v
> diff -u -p -r1.293 machdep.c
> --- arch/amd64/amd64/machdep.c 29 Apr 2024 00:29:48 -0000 1.293
> +++ arch/amd64/amd64/machdep.c 30 Apr 2024 09:40:55 -0000
> @@ -466,7 +466,7 @@ bios_sysctl(int *name, u_int namelen, vo
> if ((pdi = bios_getdiskinfo(bootdev)) == NULL)
> return ENXIO;
> biosdev = pdi->bios_number;
> - return sysctl_rdint(oldp, oldlenp, newp, biosdev);
> + return sysctl_rdint(oldp, oldlenp, newp, &biosdev);
> case BIOS_DISKINFO:
> if (namelen != 2)
> return ENOTDIR;
> @@ -474,7 +474,7 @@ bios_sysctl(int *name, u_int namelen, vo
> return ENXIO;
> return sysctl_rdstruct(oldp, oldlenp, newp, pdi, sizeof(*pdi));
> case BIOS_CKSUMLEN:
> - return sysctl_rdint(oldp, oldlenp, newp, bios_cksumlen);
> + return sysctl_rdint(oldp, oldlenp, newp, &bios_cksumlen);
> default:
> return EOPNOTSUPP;
> }
> @@ -502,6 +502,9 @@ int
> cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> +#ifndef APERTURE
> + int val;
> +#endif
> extern uint64_t tsc_frequency;
> dev_t consdev;
> dev_t dev;
> @@ -540,7 +543,8 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &allowaperture));
> #else
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #endif
> #if NPCKBC > 0 && NUKBD > 0
> case CPU_FORCEUKBD:
> @@ -548,7 +552,7 @@ cpu_sysctl(int *name, u_int namelen, voi
> int error;
>
> if (forceukbd)
> - return (sysctl_rdint(oldp, oldlenp, newp, forceukbd));
> + return (sysctl_rdint(oldp, oldlenp, newp, &forceukbd));
>
> error = sysctl_int(oldp, oldlenp, newp, newlen, &forceukbd);
> if (forceukbd)
> Index: arch/i386/i386/bios.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/bios.c,v
> diff -u -p -r1.129 bios.c
> --- arch/i386/i386/bios.c 15 Mar 2023 08:20:52 -0000 1.129
> +++ arch/i386/i386/bios.c 30 Apr 2024 09:40:55 -0000
> @@ -783,7 +783,7 @@ bios_sysctl(int *name, u_int namelen, vo
> if ((pdi = bios_getdiskinfo(bootdev)) == NULL)
> return ENXIO;
> biosdev = pdi->bios_number;
> - return sysctl_rdint(oldp, oldlenp, newp, biosdev);
> + return sysctl_rdint(oldp, oldlenp, newp, &biosdev);
> case BIOS_DISKINFO:
> if (namelen != 2)
> return ENOTDIR;
> @@ -791,7 +791,7 @@ bios_sysctl(int *name, u_int namelen, vo
> return ENXIO;
> return sysctl_rdstruct(oldp, oldlenp, newp, pdi, sizeof(*pdi));
> case BIOS_CKSUMLEN:
> - return sysctl_rdint(oldp, oldlenp, newp, bios_cksumlen);
> + return sysctl_rdint(oldp, oldlenp, newp, &bios_cksumlen);
> default:
> return EOPNOTSUPP;
> }
> Index: arch/i386/i386/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/machdep.c,v
> diff -u -p -r1.670 machdep.c
> --- arch/i386/i386/machdep.c 29 Apr 2024 00:29:48 -0000 1.670
> +++ arch/i386/i386/machdep.c 30 Apr 2024 10:53:05 -0000
> @@ -3495,6 +3495,7 @@ int
> cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> + int val;
> dev_t dev;
>
> switch (name[0]) {
> @@ -3531,12 +3532,14 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &allowaperture));
> #else
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #endif
> case CPU_CPUVENDOR:
> return (sysctl_rdstring(oldp, oldlenp, newp, cpu_vendor));
> case CPU_CPUFEATURE:
> - return (sysctl_rdint(oldp, oldlenp, newp, curcpu()->ci_feature_flags));
> + val = curcpu()->ci_feature_flags;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case CPU_KBDRESET:
> return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen,
> &kbd_reset));
> @@ -3546,7 +3549,7 @@ cpu_sysctl(int *name, u_int namelen, voi
> int error;
>
> if (forceukbd)
> - return (sysctl_rdint(oldp, oldlenp, newp, forceukbd));
> + return (sysctl_rdint(oldp, oldlenp, newp, &forceukbd));
>
> error = sysctl_int(oldp, oldlenp, newp, newlen, &forceukbd);
> if (forceukbd)
> Index: arch/luna88k/luna88k/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/luna88k/luna88k/machdep.c,v
> diff -u -p -r1.144 machdep.c
> --- arch/luna88k/luna88k/machdep.c 24 Oct 2023 13:20:10 -0000 1.144
> +++ arch/luna88k/luna88k/machdep.c 23 Apr 2024 01:18:56 -0000
> @@ -940,7 +940,7 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_rdstruct(oldp, oldlenp, newp, &consdev,
> sizeof consdev));
> case CPU_CPUTYPE:
> - return (sysctl_rdint(oldp, oldlenp, newp, cputyp));
> + return (sysctl_rdint(oldp, oldlenp, newp, &cputyp));
> default:
> return (EOPNOTSUPP);
> }
> Index: arch/macppc/macppc/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/macppc/macppc/machdep.c,v
> diff -u -p -r1.200 machdep.c
> --- arch/macppc/macppc/machdep.c 24 Oct 2023 13:20:10 -0000 1.200
> +++ arch/macppc/macppc/machdep.c 26 Apr 2024 15:57:33 -0000
> @@ -544,6 +544,10 @@ int
> cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> +#ifndef APERTURE
> + int val;
> +#endif
> +
> /* all sysctl names at this level are terminal */
> if (namelen != 1)
> return ENOTDIR;
> @@ -557,7 +561,8 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &allowaperture));
> #else
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #endif
> default:
> return (sysctl_bounded_arr(cpuctl_vars, nitems(cpuctl_vars),
> Index: arch/powerpc64/powerpc64/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/powerpc64/powerpc64/machdep.c,v
> diff -u -p -r1.74 machdep.c
> --- arch/powerpc64/powerpc64/machdep.c 7 Jan 2023 17:29:37 -0000 1.74
> +++ arch/powerpc64/powerpc64/machdep.c 23 Apr 2024 01:19:41 -0000
> @@ -1095,7 +1095,7 @@ cpu_sysctl(int *name, u_int namelen, voi
>
> switch (name[0]) {
> case CPU_ALTIVEC:
> - return (sysctl_rdint(oldp, oldlenp, newp, altivec));
> + return (sysctl_rdint(oldp, oldlenp, newp, &altivec));
> default:
> return EOPNOTSUPP;
> }
> Index: arch/sparc64/sparc64/machdep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc64/sparc64/machdep.c,v
> diff -u -p -r1.216 machdep.c
> --- arch/sparc64/sparc64/machdep.c 29 Mar 2024 21:29:34 -0000 1.216
> +++ arch/sparc64/sparc64/machdep.c 23 Apr 2024 01:21:04 -0000
> @@ -341,6 +341,9 @@ int
> cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> +#ifndef APERTURE
> + int val;
> +#endif
> int oldval, ret;
>
> /* all sysctl names at this level are terminal */
> @@ -367,12 +370,13 @@ cpu_sysctl(int *name, u_int namelen, voi
> return (sysctl_int(oldp, oldlenp, newp, newlen,
> &allowaperture));
> #else
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #endif
> case CPU_CPUTYPE:
> - return (sysctl_rdint(oldp, oldlenp, newp, cputyp));
> + return (sysctl_rdint(oldp, oldlenp, newp, &cputyp));
> case CPU_CECCERRORS:
> - return (sysctl_rdint(oldp, oldlenp, newp, ceccerrs));
> + return (sysctl_rdint(oldp, oldlenp, newp, &ceccerrs));
> case CPU_CECCLAST:
> return (sysctl_rdquad(oldp, oldlenp, newp, cecclast));
> default:
> Index: ddb/db_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/ddb/db_usrreq.c,v
> diff -u -p -r1.22 db_usrreq.c
> --- ddb/db_usrreq.c 9 Jan 2021 20:58:12 -0000 1.22
> +++ ddb/db_usrreq.c 30 Apr 2024 09:40:55 -0000
> @@ -48,6 +48,8 @@ int
> ddb_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> + int val;
> +
> /* All sysctl names at this level are terminal. */
> if (namelen != 1)
> return (ENOTDIR);
> @@ -83,7 +85,8 @@ ddb_sysctl(int *name, u_int namelen, voi
> } else
> return (ENODEV);
> }
> - return (sysctl_rdint(oldp, oldlenp, newp, 0));
> + val = 0;
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> #if defined(DDBPROF)
> case DBCTL_PROFILE:
> if (securelevel > 0)
> Index: kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.427 kern_sysctl.c
> --- kern/kern_sysctl.c 12 Apr 2024 16:07:09 -0000 1.427
> +++ kern/kern_sysctl.c 30 Apr 2024 10:53:05 -0000
> @@ -157,6 +157,7 @@ int (*cpu_cpuspeed)(int *);
> /*
> * Lock to avoid too many processes vslocking a large amount of memory
> * at the same time.
> + * Also provide consistent transitions from before to after values.
> */
> struct rwlock sysctl_lock = RWLOCK_INITIALIZER("sysctllk");
> struct rwlock sysctl_disklock = RWLOCK_INITIALIZER("sysctldlk");
> @@ -448,7 +449,7 @@ int
> kern_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> size_t newlen, struct proc *p)
> {
> - int error, level, inthostid, stackgap;
> + int val, error, level, inthostid, stackgap;
> dev_t dev;
> extern int pool_debug;
>
> @@ -467,8 +468,9 @@ kern_sysctl(int *name, u_int namelen, vo
> return (sysctl_rdstring(oldp, oldlenp, newp, osversion));
> case KERN_VERSION:
> return (sysctl_rdstring(oldp, oldlenp, newp, version));
> - case KERN_NUMVNODES: /* XXX numvnodes is a long */
> - return (sysctl_rdint(oldp, oldlenp, newp, numvnodes));
> + case KERN_NUMVNODES:
> + val = numvnodes; /* XXX numvnodes is a long */
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case KERN_SECURELVL:
> level = securelevel;
> if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &level)) ||
> @@ -543,7 +545,8 @@ kern_sysctl(int *name, u_int namelen, vo
> */
> if (!mp || mp->msg_magic != MSG_MAGIC)
> return (ENXIO);
> - return (sysctl_rdint(oldp, oldlenp, newp, mp->msg_bufs));
> + val = mp->msg_bufs; /* XXX msg_bufs is a long */
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> }
> case KERN_CONSBUF:
> if ((error = suser(p)))
> @@ -681,7 +684,7 @@ hw_sysctl(int *name, u_int namelen, void
> size_t newlen, struct proc *p)
> {
> extern char machine[], cpu_model[];
> - int err, cpuspeed;
> + int val, err, cpuspeed;
>
> /*
> * all sysctl names at this level except sensors and battery
> @@ -696,13 +699,14 @@ hw_sysctl(int *name, u_int namelen, void
> case HW_MODEL:
> return (sysctl_rdstring(oldp, oldlenp, newp, cpu_model));
> case HW_NCPUONLINE:
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - sysctl_hwncpuonline()));
> + val = sysctl_hwncpuonline();
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case HW_PHYSMEM:
> - return (sysctl_rdint(oldp, oldlenp, newp, ptoa(physmem)));
> + val = ptoa(physmem); /* XXX unsigned long */
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case HW_USERMEM:
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - ptoa(physmem - uvmexp.wired)));
> + val = ptoa(physmem - uvmexp.wired); /* XXX unsigned long */
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case HW_DISKNAMES:
> err = sysctl_diskinit(0, p);
> if (err)
> @@ -724,7 +728,7 @@ hw_sysctl(int *name, u_int namelen, void
> err = cpu_cpuspeed(&cpuspeed);
> if (err)
> return err;
> - return (sysctl_rdint(oldp, oldlenp, newp, cpuspeed));
> + return (sysctl_rdint(oldp, oldlenp, newp, &cpuspeed));
> #ifndef SMALL_KERNEL
> case HW_SENSORS:
> return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
> @@ -933,17 +937,32 @@ int
> sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
> int *valp)
> {
> - unsigned int oval = *valp, val = *valp;
> + unsigned int oldval, newval;
> int error;
>
> - if (newp == NULL)
> - return (sysctl_rdint(oldp, oldlenp, newp, val));
> + if (oldp && *oldlenp < sizeof(int))
> + return (ENOMEM);
> + if (newp && newlen != sizeof(int))
> + return (EINVAL);
> + *oldlenp = sizeof(int);
>
> - if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
> - return (error);
> - if (val > oval)
> - return (EPERM); /* do not allow raising */
> - *(unsigned int *)valp = val;
> + /* copyin() may sleep, call it first */
> + if (newp) {
> + if ((error = copyin(newp, &newval, sizeof(int))))
> + return (error);
> + }
> + oldval = READ_ONCE(*valp);
> + if (newp) {
> + /* do not allow raising */
> + if (newval > oldval)
> + return (EPERM);
> + }
> + if (oldp) {
> + if ((error = copyout(&oldval, oldp, sizeof(int))))
> + return (error);
> + }
> + if (newp)
> + WRITE_ONCE(*valp, newval);
> return (0);
> }
>
> @@ -954,26 +973,37 @@ sysctl_int_lower(void *oldp, size_t *old
> int
> sysctl_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int *valp)
> {
> - int error = 0;
> + int oldval, newval;
> + int error;
>
> if (oldp && *oldlenp < sizeof(int))
> return (ENOMEM);
> if (newp && newlen != sizeof(int))
> return (EINVAL);
> *oldlenp = sizeof(int);
> - if (oldp)
> - error = copyout(valp, oldp, sizeof(int));
> - if (error == 0 && newp)
> - error = copyin(newp, valp, sizeof(int));
> - return (error);
> +
> + /* copyin() may sleep, call it first */
> + if (newp) {
> + if ((error = copyin(newp, &newval, sizeof(int))))
> + return (error);
> + }
> + if (oldp) {
> + oldval = READ_ONCE(*valp);
> + if ((error = copyout(&oldval, oldp, sizeof(int))))
> + return (error);
> + }
> + if (newp)
> + WRITE_ONCE(*valp, newval);
> + return (0);
> }
>
> /*
> * As above, but read-only.
> */
> int
> -sysctl_rdint(void *oldp, size_t *oldlenp, void *newp, int val)
> +sysctl_rdint(void *oldp, size_t *oldlenp, void *newp, const int *valp)
> {
> + int oldval;
> int error = 0;
>
> if (oldp && *oldlenp < sizeof(int))
> @@ -981,8 +1011,11 @@ sysctl_rdint(void *oldp, size_t *oldlenp
> if (newp)
> return (EPERM);
> *oldlenp = sizeof(int);
> - if (oldp)
> - error = copyout((caddr_t)&val, oldp, sizeof(int));
> +
> + if (oldp) {
> + oldval = READ_ONCE(*valp);
> + error = copyout(&oldval, oldp, sizeof(int));
> + }
> return (error);
> }
>
> @@ -994,7 +1027,7 @@ sysctl_securelevel_int(void *oldp, size_
> int *valp)
> {
> if (securelevel > 0)
> - return (sysctl_rdint(oldp, oldlenp, newp, *valp));
> + return (sysctl_rdint(oldp, oldlenp, newp, valp));
> return (sysctl_int(oldp, oldlenp, newp, newlen, valp));
> }
>
> @@ -1005,19 +1038,34 @@ int
> sysctl_int_bounded(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
> int *valp, int minimum, int maximum)
> {
> - int val = *valp;
> + int oldval, newval;
> int error;
>
> /* read only */
> - if (newp == NULL || minimum > maximum)
> - return (sysctl_rdint(oldp, oldlenp, newp, val));
> + if (minimum > maximum)
> + return (sysctl_rdint(oldp, oldlenp, newp, valp));
>
> - if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
> - return (error);
> - /* outside limits */
> - if (val < minimum || maximum < val)
> + if (oldp && *oldlenp < sizeof(int))
> + return (ENOMEM);
> + if (newp && newlen != sizeof(int))
> return (EINVAL);
> - *valp = val;
> + *oldlenp = sizeof(int);
> +
> + /* copyin() may sleep, call it first */
> + if (newp) {
> + if ((error = copyin(newp, &newval, sizeof(int))))
> + return (error);
> + /* outside limits */
> + if (newval < minimum || maximum < newval)
> + return (EINVAL);
> + }
> + if (oldp) {
> + oldval = READ_ONCE(*valp);
> + if ((error = copyout(&oldval, oldp, sizeof(int))))
> + return (error);
> + }
> + if (newp)
> + WRITE_ONCE(*valp, newval);
> return (0);
> }
>
> @@ -1923,11 +1971,11 @@ sysctl_proc_args(int *name, u_int namele
> goto out;
>
> if (op == KERN_PROC_NARGV) {
> - error = sysctl_rdint(oldp, oldlenp, NULL, pss.ps_nargvstr);
> + error = sysctl_rdint(oldp, oldlenp, NULL, &pss.ps_nargvstr);
> goto out;
> }
> if (op == KERN_PROC_NENV) {
> - error = sysctl_rdint(oldp, oldlenp, NULL, pss.ps_nenvstr);
> + error = sysctl_rdint(oldp, oldlenp, NULL, &pss.ps_nenvstr);
> goto out;
> }
>
> Index: kern/sched_bsd.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/sched_bsd.c,v
> diff -u -p -r1.91 sched_bsd.c
> --- kern/sched_bsd.c 30 Mar 2024 13:33:20 -0000 1.91
> +++ kern/sched_bsd.c 30 Apr 2024 09:40:55 -0000
> @@ -662,7 +662,7 @@ sysctl_hwsetperf(void *oldp, size_t *old
> return EOPNOTSUPP;
>
> if (perfpolicy != PERFPOL_MANUAL)
> - return sysctl_rdint(oldp, oldlenp, newp, perflevel);
> + return sysctl_rdint(oldp, oldlenp, newp, &perflevel);
>
> err = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> &perflevel, 0, 100);
> Index: kern/subr_evcount.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_evcount.c,v
> diff -u -p -r1.16 subr_evcount.c
> --- kern/subr_evcount.c 16 Sep 2023 09:33:27 -0000 1.16
> +++ kern/subr_evcount.c 30 Apr 2024 09:40:55 -0000
> @@ -123,7 +123,7 @@ evcount_sysctl(int *name, u_int namelen,
>
> switch (name[0]) {
> case KERN_INTRCNT_NUM:
> - error = sysctl_rdint(oldp, oldlenp, NULL, nintr);
> + error = sysctl_rdint(oldp, oldlenp, NULL, &nintr);
> break;
> case KERN_INTRCNT_CNT:
> if (ec == NULL)
> @@ -145,8 +145,7 @@ evcount_sysctl(int *name, u_int namelen,
> case KERN_INTRCNT_VECTOR:
> if (ec == NULL || ec->ec_data == NULL)
> return (ENOENT);
> - error = sysctl_rdint(oldp, oldlenp, NULL,
> - *((int *)ec->ec_data));
> + error = sysctl_rdint(oldp, oldlenp, NULL, ec->ec_data);
> break;
> default:
> error = EOPNOTSUPP;
> Index: kern/subr_pool.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_pool.c,v
> diff -u -p -r1.236 subr_pool.c
> --- kern/subr_pool.c 14 Aug 2022 01:58:28 -0000 1.236
> +++ kern/subr_pool.c 30 Apr 2024 09:40:55 -0000
> @@ -1473,7 +1473,7 @@ sysctl_dopool(int *name, u_int namelen,
> case KERN_POOL_NPOOLS:
> if (namelen != 1)
> return (ENOTDIR);
> - return (sysctl_rdint(oldp, oldlenp, NULL, pool_count));
> + return (sysctl_rdint(oldp, oldlenp, NULL, &pool_count));
>
> case KERN_POOL_NAME:
> case KERN_POOL_POOL:
> Index: kern/uipc_mbuf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
> diff -u -p -r1.290 uipc_mbuf.c
> --- kern/uipc_mbuf.c 5 Mar 2024 18:52:41 -0000 1.290
> +++ kern/uipc_mbuf.c 30 Apr 2024 09:40:55 -0000
> @@ -1786,7 +1786,7 @@ sysctl_mq(int *name, u_int namelen, void
> void *newp, size_t newlen, struct mbuf_queue *mq)
> {
> unsigned int maxlen;
> - int error;
> + int val, error;
>
> /* All sysctl names at this level are terminal. */
> if (namelen != 1)
> @@ -1794,7 +1794,8 @@ sysctl_mq(int *name, u_int namelen, void
>
> switch (name[0]) {
> case IFQCTL_LEN:
> - return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq)));
> + val = mq_len(mq);
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> case IFQCTL_MAXLEN:
> maxlen = mq->mq_maxlen;
> error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
> @@ -1802,7 +1803,8 @@ sysctl_mq(int *name, u_int namelen, void
> mq_set_maxlen(mq, maxlen);
> return (error);
> case IFQCTL_DROPS:
> - return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));
> + val = mq_drops(mq);
> + return (sysctl_rdint(oldp, oldlenp, newp, &val));
> default:
> return (EOPNOTSUPP);
> }
> Index: kern/uipc_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v
> diff -u -p -r1.204 uipc_usrreq.c
> --- kern/uipc_usrreq.c 10 Apr 2024 12:04:41 -0000 1.204
> +++ kern/uipc_usrreq.c 30 Apr 2024 09:40:55 -0000
> @@ -733,7 +733,7 @@ uipc_sysctl(int *name, u_int namelen, vo
> case NET_UNIX_DEFERRED:
> if (namelen != 1)
> return (ENOTDIR);
> - return sysctl_rdint(oldp, oldlenp, newp, *valp);
> + return sysctl_rdint(oldp, oldlenp, newp, valp);
> default:
> return (ENOPROTOOPT);
> }
> Index: kern/vfs_subr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_subr.c,v
> diff -u -p -r1.319 vfs_subr.c
> --- kern/vfs_subr.c 3 Feb 2024 18:51:58 -0000 1.319
> +++ kern/vfs_subr.c 30 Apr 2024 09:40:55 -0000
> @@ -1390,7 +1390,7 @@ vfs_sysctl(int *name, u_int namelen, voi
>
> switch (name[1]) {
> case VFS_MAXTYPENUM:
> - return (sysctl_rdint(oldp, oldlenp, newp, maxvfsconf));
> + return (sysctl_rdint(oldp, oldlenp, newp, &maxvfsconf));
>
> case VFS_CONF:
> if (namelen < 3)
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.393 ip_input.c
> --- netinet/ip_input.c 16 Apr 2024 12:56:39 -0000 1.393
> +++ netinet/ip_input.c 30 Apr 2024 09:40:55 -0000
> @@ -1741,8 +1741,7 @@ ip_sysctl(int *name, u_int namelen, void
> return (sysctl_niq(name + 1, namelen - 1,
> oldp, oldlenp, newp, newlen, &arpinq));
> case IPCTL_ARPQUEUED:
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - atomic_load_int(&la_hold_total)));
> + return (sysctl_rdint(oldp, oldlenp, newp, &la_hold_total));
> case IPCTL_STATS:
> return (ip_sysctl_ipstat(oldp, oldlenp, newp));
> #ifdef MROUTING
> Index: netinet6/icmp6.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
> diff -u -p -r1.252 icmp6.c
> --- netinet6/icmp6.c 21 Apr 2024 17:32:10 -0000 1.252
> +++ netinet6/icmp6.c 30 Apr 2024 09:40:55 -0000
> @@ -1906,8 +1906,7 @@ icmp6_sysctl(int *name, u_int namelen, v
> break;
>
> case ICMPV6CTL_ND6_QUEUED:
> - error = sysctl_rdint(oldp, oldlenp, newp,
> - atomic_load_int(&ln_hold_total));
> + error = sysctl_rdint(oldp, oldlenp, newp, &ln_hold_total);
> break;
>
> default:
> Index: sys/sysctl.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/sysctl.h,v
> diff -u -p -r1.235 sysctl.h
> --- sys/sysctl.h 1 Oct 2023 15:58:12 -0000 1.235
> +++ sys/sysctl.h 30 Apr 2024 10:53:05 -0000
> @@ -1056,7 +1056,7 @@ typedef int (sysctlfn)(int *, u_int, voi
>
> int sysctl_int_lower(void *, size_t *, void *, size_t, int *);
> int sysctl_int(void *, size_t *, void *, size_t, int *);
> -int sysctl_rdint(void *, size_t *, void *, int);
> +int sysctl_rdint(void *, size_t *, void *, const int *);
> int sysctl_securelevel_int(void *, size_t *, void *, size_t, int *);
> int sysctl_int_bounded(void *, size_t *, void *, size_t, int *, int, int);
> int sysctl_bounded_arr(const struct sysctl_bounded_args *, u_int,
> Index: uvm/uvm_meter.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/uvm/uvm_meter.c,v
> diff -u -p -r1.50 uvm_meter.c
> --- uvm/uvm_meter.c 16 Sep 2023 09:33:27 -0000 1.50
> +++ uvm/uvm_meter.c 30 Apr 2024 09:40:55 -0000
> @@ -114,7 +114,7 @@ uvm_sysctl(int *name, u_int namelen, voi
> sizeof(uexp)));
>
> case VM_NKMEMPAGES:
> - return (sysctl_rdint(oldp, oldlenp, newp, nkmempages));
> + return (sysctl_rdint(oldp, oldlenp, newp, &nkmempages));
>
> case VM_PSSTRINGS:
> return (sysctl_rdstruct(oldp, oldlenp, newp, &pr->ps_strings,
> @@ -160,10 +160,11 @@ uvm_sysctl(int *name, u_int namelen, voi
> return rv;
>
> case VM_MAXSLP:
> - return (sysctl_rdint(oldp, oldlenp, newp, maxslp));
> + return (sysctl_rdint(oldp, oldlenp, newp, &maxslp));
>
> case VM_USPACE:
> - return (sysctl_rdint(oldp, oldlenp, newp, USPACE));
> + t = USPACE;
> + return (sysctl_rdint(oldp, oldlenp, newp, &t));
>
> case VM_MALLOC_CONF:
> return (sysctl_string(oldp, oldlenp, newp, newlen,
> Index: uvm/uvm_swap_encrypt.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/uvm/uvm_swap_encrypt.c,v
> diff -u -p -r1.24 uvm_swap_encrypt.c
> --- uvm/uvm_swap_encrypt.c 12 Mar 2021 14:15:49 -0000 1.24
> +++ uvm/uvm_swap_encrypt.c 30 Apr 2024 09:40:55 -0000
> @@ -79,9 +79,9 @@ swap_encrypt_ctl(int *name, u_int namele
> return (0);
> }
> case SWPENC_CREATED:
> - return (sysctl_rdint(oldp, oldlenp, newp, uvm_swpkeyscreated));
> + return (sysctl_rdint(oldp, oldlenp, newp, &uvm_swpkeyscreated));
> case SWPENC_DELETED:
> - return (sysctl_rdint(oldp, oldlenp, newp, uvm_swpkeysdeleted));
> + return (sysctl_rdint(oldp, oldlenp, newp, &uvm_swpkeysdeleted));
> default:
> return (EOPNOTSUPP);
> }
>
>
sysctl int atomic