From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: sysctl int atomic
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 26 Apr 2024 22:49:16 +0200

> 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?

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.

> 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.292 machdep.c
> --- arch/amd64/amd64/machdep.c	3 Apr 2024 02:01:21 -0000	1.292
> +++ arch/amd64/amd64/machdep.c	26 Apr 2024 15:57:10 -0000
> @@ -469,7 +469,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;
> @@ -477,7 +477,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;
>  	}
> @@ -505,6 +505,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;
> @@ -543,7 +546,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:
> @@ -551,7 +555,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	26 Apr 2024 15:57:10 -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.669 machdep.c
> --- arch/i386/i386/machdep.c	23 Aug 2023 01:55:46 -0000	1.669
> +++ arch/i386/i386/machdep.c	26 Apr 2024 15:57:10 -0000
> @@ -3500,6 +3500,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]) {
> @@ -3536,12 +3537,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));
> @@ -3551,7 +3554,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	26 Apr 2024 15:57:10 -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	26 Apr 2024 16:00:13 -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,35 @@ 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);
> +	membar_consumer();
> +	if (newp) {
> +		/* do not allow raising */
> +		if (newval > oldval)
> +			return (EPERM);
> +	}
> +	if (oldp) {
> +		if ((error = copyout(&oldval, oldp, sizeof(int))))
> +			return (error);
> +	}
> +	if (newp) {
> +		membar_producer();
> +		WRITE_ONCE(*valp, newval);
> +	}
>  	return (0);
>  }
>  
> @@ -954,26 +976,40 @@ 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);
> +		membar_consumer();
> +		if ((error = copyout(&oldval, oldp, sizeof(int))))
> +			return (error);
> +	}
> +	if (newp) {
> +		membar_producer();
> +		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 +1017,12 @@ 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);
> +		membar_consumer();
> +		error = copyout(&oldval, oldp, sizeof(int));
> +	}
>  	return (error);
>  }
>  
> @@ -994,7 +1034,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 +1045,37 @@ 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);
> +		membar_consumer();
> +		if ((error = copyout(&oldval, oldp, sizeof(int))))
> +			return (error);
> +	}
> +	if (newp) {
> +		membar_producer();
> +		WRITE_ONCE(*valp, newval);
> +	}
>  	return (0);
>  }
>  
> @@ -1923,11 +1981,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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 16:00:28 -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	26 Apr 2024 15:57:10 -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	26 Apr 2024 15:57:10 -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);
>  	}
> 
>