From: Martin Pieuchot <mpi@openbsd.org>
Subject: Re: sysctl(2): unlock KERN_MAXPROC
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 19 Aug 2024 14:46:31 +0200

On 19/08/24(Mon) 13:28, Vitaliy Makkoveev wrote:
> On Mon, Aug 19, 2024 at 11:56:40AM +0200, Martin Pieuchot wrote:
> > On 18/08/24(Sun) 23:51, Vitaliy Makkoveev wrote:
> > > I want to unlock atomically accessed integers from `kern_vars' and
> > > remove corresponding cases from the 'switch' block.
> > > 
> > > `maxprocess' is atomically accessed integer. procinit() called during
> > > bootstrap, so no atomic_load_int(9) required.
> > 
> > Could you keep maxthread in sync?
> >
> 
> Sure.

`maxthread' is already accessed with atomic_load_int() via KERN_NTHREADS
& sysctl_bounded_arr(), right?  Is there other global variables in this
array whose accesses should be converted to atomic_load_int() as well?

> > What about the hashinit() calls in kern/kern_proc.c ?
> > 
> 
> procinit() called during kernel bootstrap, you can't have concurrent
> sysctl(2) threads this time. This is also true for pmap_bootstrap().

Diff is ok mpi@.

> Index: sys/conf/param.c
> ===================================================================
> RCS file: /cvs/src/sys/conf/param.c,v
> diff -u -p -r1.50 param.c
> --- sys/conf/param.c	5 May 2024 06:14:37 -0000	1.50
> +++ sys/conf/param.c	19 Aug 2024 10:23:58 -0000
> @@ -50,6 +50,11 @@
>  #endif
>  
>  /*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
> +/*
>   * System parameter formulae.
>   *
>   * This file is copied into each directory where we compile
> @@ -67,8 +72,8 @@ int	utc_offset = 0;
>  #define	NTEXT (80 + NPROCESS / 8)		/* actually the object cache */
>  #define	NVNODE (NPROCESS * 2 + NTEXT + 100)
>  int	initialvnodes = NVNODE;
> -int	maxprocess = NPROCESS;
> -int	maxthread = 2 * NPROCESS;
> +int	maxprocess = NPROCESS;				/* [a] */
> +int	maxthread = 2 * NPROCESS;			/* [a] */
>  int	maxfiles = 5 * (NPROCESS + MAXUSERS) + 80;
>  long	nmbclust = NMBCLUSTERS;
>  
> Index: sys/kern/kern_fork.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_fork.c,v
> diff -u -p -r1.263 kern_fork.c
> --- sys/kern/kern_fork.c	16 Aug 2024 16:19:03 -0000	1.263
> +++ sys/kern/kern_fork.c	19 Aug 2024 10:23:59 -0000
> @@ -307,7 +307,7 @@ struct timeval fork_tfmrate = { 10, 0 };
>  int
>  fork_check_maxthread(uid_t uid)
>  {
> -	int val;
> +	int maxthread_local, val;
>  
>  	/*
>  	 * Although process entries are dynamically created, we still keep
> @@ -318,8 +318,10 @@ fork_check_maxthread(uid_t uid)
>  	 * the variable nthreads is the current number of procs, maxthread is
>  	 * the limit.
>  	 */
> +	maxthread_local = atomic_load_int(&maxthread);
>  	val = atomic_inc_int_nv(&nthreads);
> -	if ((val > maxthread - 5 && uid != 0) || val > maxthread) {
> +	if ((val > maxthread_local - 5 && uid != 0) ||
> +	    val > maxthread_local) {
>  		static struct timeval lasttfm;
>  
>  		if (ratecheck(&lasttfm, &fork_tfmrate))
> @@ -353,7 +355,7 @@ fork1(struct proc *curp, int flags, void
>  	struct proc *p;
>  	uid_t uid = curp->p_ucred->cr_ruid;
>  	struct vmspace *vm;
> -	int count;
> +	int count, maxprocess_local;
>  	vaddr_t uaddr;
>  	int error;
>  	struct  ptrace_state *newptstat = NULL;
> @@ -366,8 +368,9 @@ fork1(struct proc *curp, int flags, void
>  	if ((error = fork_check_maxthread(uid)))
>  		return error;
>  
> -	if ((nprocesses >= maxprocess - 5 && uid != 0) ||
> -	    nprocesses >= maxprocess) {
> +	maxprocess_local = atomic_load_int(&maxprocess);
> +	if ((nprocesses >= maxprocess_local - 5 && uid != 0) ||
> +	    nprocesses >= maxprocess_local) {
>  		static struct timeval lasttfm;
>  
>  		if (ratecheck(&lasttfm, &fork_tfmrate))
> Index: sys/kern/kern_resource.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_resource.c,v
> diff -u -p -r1.86 kern_resource.c
> --- sys/kern/kern_resource.c	9 Jul 2024 15:20:15 -0000	1.86
> +++ sys/kern/kern_resource.c	19 Aug 2024 10:23:59 -0000
> @@ -278,7 +278,7 @@ dosetrlimit(struct proc *p, u_int which,
>  		maxlim = maxfiles;
>  		break;
>  	case RLIMIT_NPROC:
> -		maxlim = maxprocess;
> +		maxlim = atomic_load_int(&maxprocess);
>  		break;
>  	default:
>  		maxlim = RLIM_INFINITY;
> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.439 kern_sysctl.c
> --- sys/kern/kern_sysctl.c	14 Aug 2024 17:52:47 -0000	1.439
> +++ sys/kern/kern_sysctl.c	19 Aug 2024 10:23:59 -0000
> @@ -549,6 +549,7 @@ kern_sysctl(int *name, u_int namelen, vo
>  		return (sysctl_rdint(oldp, oldlenp, newp, mp->msg_bufs));
>  	}
>  	case KERN_OSREV:
> +	case KERN_MAXPROC:
>  	case KERN_NFILES:
>  	case KERN_TTYCOUNT:
>  	case KERN_ARGMAX:
> @@ -558,6 +559,7 @@ kern_sysctl(int *name, u_int namelen, vo
>  	case KERN_SAVED_IDS:
>  	case KERN_MAXPARTITIONS:
>  	case KERN_RAWPARTITION:
> +	case KERN_MAXTHREAD:
>  	case KERN_NTHREADS:
>  	case KERN_SOMAXCONN:
>  	case KERN_SOMINCONN:
>