On Tue, Aug 13, 2024 at 02:24:34PM +0300, Vitaliy Makkoveev wrote:
> Make sysctl_int() mp-safe in the sysctl_int_bounded() way and unlock
> KERN_HOSTID. Except bootstrap called clockattach() of sparc64, `hostid'
> never used outside kern_sysctl().
> 
> mp-safe sysctl_int() is meaningless for sysctl_int_lower(), so rework it
> too. This time all affected paths are kernel locked, but this doesn't
> make sysctl_int_lower() worse.
> 
> ok?
> 

I want to change `hostid' type to the type of int. It only stored but
never used within kernel, userland accesses it through sysctl_int().
Nothing changes, but variable becomes consistent with sysctl_int().

Also, the only difference between sysctl_int() and sysctl_int_bounded()
is the range check, so sysctl_int() is just sysctl_int_bounded(..., 
INT_MIN, INT_MAX). sysctl_int() is not the fast path, so this useless
check should not be significant.

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
diff -u -p -r1.437 kern_sysctl.c
--- sys/kern/kern_sysctl.c	11 Aug 2024 15:10:53 -0000	1.437
+++ sys/kern/kern_sysctl.c	14 Aug 2024 11:46:41 -0000
@@ -306,7 +306,7 @@ char hostname[MAXHOSTNAMELEN];
 int hostnamelen;
 char domainname[MAXHOSTNAMELEN];
 int domainnamelen;
-long hostid;
+int hostid;
 char *disknames = NULL;
 size_t disknameslen;
 struct diskstats *diskstats = NULL;
@@ -507,6 +507,8 @@ kern_sysctl(int *name, u_int namelen, vo
 		return (sysctl_rdstring(oldp, oldlenp, newp, version));
 	case KERN_NUMVNODES:  /* XXX numvnodes is a long */
 		return (sysctl_rdint(oldp, oldlenp, newp, numvnodes));
+	case KERN_HOSTID:
+		return (sysctl_int(oldp, oldlenp, newp, newlen, &hostid));
 	case KERN_CLOCKRATE:
 		return (sysctl_clockrate(oldp, oldlenp, newp));
 	case KERN_BOOTTIME: {
@@ -585,7 +587,7 @@ int
 kern_sysctl_locked(int *name, u_int namelen, void *oldp, size_t *oldlenp,
     void *newp, size_t newlen, struct proc *p)
 {
-	int error, level, inthostid, stackgap;
+	int error, level, stackgap;
 	dev_t dev;
 	extern int pool_debug;
 
@@ -623,11 +625,6 @@ kern_sysctl_locked(int *name, u_int name
 		if (newp && !error)
 			domainnamelen = newlen;
 		return (error);
-	case KERN_HOSTID:
-		inthostid = hostid;  /* XXX assumes sizeof long <= sizeof int */
-		error =  sysctl_int(oldp, oldlenp, newp, newlen, &inthostid);
-		hostid = inthostid;
-		return (error);
 	case KERN_CONSBUF:
 		if ((error = suser(p)))
 			return (error);
@@ -1055,17 +1052,36 @@ 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 (newp) {
+		if ((error = copyin(newp, &newval, sizeof(int))))
+			return (error);
+		do {
+			oldval = atomic_load_int(valp);
+			if (oldval < (unsigned int)newval)
+				return (EPERM);	/* do not allow raising */
+		} while (atomic_cas_uint(valp, oldval, newval) != oldval);
+
+		if (oldp) {
+			/* new value has been set although user gets error */
+			if ((error = copyout(&oldval, oldp, sizeof(int))))
+				return (error);
+		}
+	} else if (oldp) {
+		oldval = atomic_load_int(valp);
+
+		if ((error = copyout(&oldval, oldp, sizeof(int))))
+			return (error);	
+	}
 
-	if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
-		return (error);
-	if (val > oval)
-		return (EPERM);		/* do not allow raising */
-	*(unsigned int *)valp = val;
 	return (0);
 }
 
@@ -1076,18 +1092,8 @@ 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;
-
-	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);
+	return (sysctl_int_bounded(oldp, oldlenp, newp, newlen, valp,
+	    INT_MIN, INT_MAX));
 }
 
 /*
Index: sys/sys/kernel.h
===================================================================
RCS file: /cvs/src/sys/sys/kernel.h,v
diff -u -p -r1.26 kernel.h
--- sys/sys/kernel.h	3 Mar 2023 20:16:44 -0000	1.26
+++ sys/sys/kernel.h	14 Aug 2024 11:46:41 -0000
@@ -40,7 +40,7 @@
 /* Global variables for the kernel. */
 
 /* 1.1 */
-extern long hostid;
+extern int hostid;
 extern char hostname[MAXHOSTNAMELEN];
 extern int hostnamelen;
 extern char domainname[MAXHOSTNAMELEN];