From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: sysctl: push locking down to HW_BATTERY
To: tech@openbsd.org
Date: Sun, 4 May 2025 21:03:07 +0300

sysctl(2) is full of unnecessary pages wiring, and we want to decrease
this pressure.

Avoid locking and pages wiring in read-only and error paths. Keep
current `sysctl_lock' + kernel lock locking to serialize modification
path, but drop unnecessary pages wiring.

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.466
diff -u -p -r1.466 kern_sysctl.c
--- sys/kern/kern_sysctl.c	4 May 2025 13:42:07 -0000	1.466
+++ sys/kern/kern_sysctl.c	4 May 2025 17:53:23 -0000
@@ -825,7 +825,6 @@ hw_sysctl(int *name, u_int namelen, void
 	case HW_SENSORS:
 	case HW_SETPERF:
 	case HW_PERFPOLICY:
-	case HW_BATTERY:
 #endif /* !SMALL_KERNEL */
 	case HW_ALLOWPOWERDOWN:
 	case HW_UCOMNAMES:
@@ -874,6 +873,11 @@ hw_sysctl(int *name, u_int namelen, void
 	case HW_USERMEM64:
 		return (sysctl_rdquad(oldp, oldlenp, newp,
 		    ptoa((psize_t)physmem - uvmexp.wired)));
+#ifndef SMALL_KERNEL
+	case HW_BATTERY:
+		return (sysctl_hwbattery(name + 1, namelen - 1, oldp, oldlenp,
+		    newp, newlen));
+#endif
 	default:
 		return sysctl_bounded_arr(hw_vars, nitems(hw_vars), name,
 		    namelen, oldp, oldlenp, newp, newlen);
@@ -933,11 +937,6 @@ hw_sysctl_locked(int *name, u_int namele
 	case HW_SMT:
 		return (sysctl_hwsmt(oldp, oldlenp, newp, newlen));
 #endif
-#ifndef SMALL_KERNEL
-	case HW_BATTERY:
-		return (sysctl_hwbattery(name + 1, namelen - 1, oldp, oldlenp,
-		    newp, newlen));
-#endif
 	default:
 		return (EOPNOTSUPP);
 	}
@@ -956,19 +955,25 @@ int (*hw_battery_setchargestop)(int);
 int
 sysctl_hwchargemode(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
 {
-	int mode = hw_battery_chargemode;
-	int error;
+	int mode, error;
 
 	if (!hw_battery_setchargemode)
 		return EOPNOTSUPP;
 
+	mode = atomic_load_int(&hw_battery_chargemode);
 	error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
 	    &mode, -1, 1);
 	if (error)
 		return error;
 
-	if (newp != NULL)
+	if (newp != NULL) {
+		if ((error = rw_enter(&sysctl_lock, RW_WRITE|RW_INTR)))
+			return error;
+		KERNEL_LOCK();
 		error = hw_battery_setchargemode(mode);
+		KERNEL_UNLOCK();
+		rw_exit(&sysctl_lock);
+	}
 
 	return error;
 }
@@ -976,19 +981,25 @@ sysctl_hwchargemode(void *oldp, size_t *
 int
 sysctl_hwchargestart(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
 {
-	int start = hw_battery_chargestart;
-	int error;
+	int start, error;
 
 	if (!hw_battery_setchargestart)
 		return EOPNOTSUPP;
 
+	start = atomic_load_int(&hw_battery_chargestart);
 	error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
 	    &start, 0, 100);
 	if (error)
 		return error;
 
-	if (newp != NULL)
+	if (newp != NULL) {
+		if ((error = rw_enter(&sysctl_lock, RW_WRITE|RW_INTR)))
+			return error;
+		KERNEL_LOCK();
 		error = hw_battery_setchargestart(start);
+		KERNEL_UNLOCK();
+		rw_exit(&sysctl_lock);
+	}
 
 	return error;
 }
@@ -996,19 +1007,25 @@ sysctl_hwchargestart(void *oldp, size_t 
 int
 sysctl_hwchargestop(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
 {
-	int stop = hw_battery_chargestop;
-	int error;
+	int stop, error;
 
 	if (!hw_battery_setchargestop)
 		return EOPNOTSUPP;
 
+	stop = atomic_load_int(&hw_battery_chargestop);
 	error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
 	    &stop, 0, 100);
 	if (error)
 		return error;
 
-	if (newp != NULL)
+	if (newp != NULL) {
+		if ((error = rw_enter(&sysctl_lock, RW_WRITE|RW_INTR)))
+			return error;
+		KERNEL_LOCK();
 		error = hw_battery_setchargestop(stop);
+		KERNEL_UNLOCK();
+		rw_exit(&sysctl_lock);
+	}
 
 	return error;
 }