From: Claudio Jeker <claudio@openbsd.org>
Subject: Re: vmstat(8): get rid of KERN_MALLOC_BUCKETS sysctl(2) call
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 14 Jan 2025 13:14:30 +0100

On Tue, Jan 14, 2025 at 02:08:38PM +0300, Vitaliy Makkoveev wrote:
> claudio@ pointed that KERN_MALLOC_BUCKETS aka kern.malloc.buckets is just
> the dummy string representation of (1 << i) for MINBUCKET to MINBUCKET +
> 16 integers, so no reason to keep it in kernel. Also tb@ pointed, that
> Debian code search has no hits, so it's probably unused.
> 
> Let's try to remove it. Our vmstat(8) and systat(a) are the only
> userland users and they could generate desired bucket names by them self
> instead of KERN_MALLOC_BUCKETS strtonum(3) parsing.
> 
> This diff converts vmstat(8). I'll convert systat(1) separately if this
> conversion will be accepted.
> 
> Note, MINBUCKET and MAXBUCKET are defined in kernel headers, the
> "MINBUCKET + 16" is already hard-coded to vmstat.c.
> 
> Note, sysctl_malloc() has the similar KERN_MALLOC_KMEMNAMES string,
> which is unused in our userland and also was not found by Debian code
> search.

One thing this breaks is -M / -N support for kernels that have a different
MINBUCKET define. Do we care about that?
 
> Index: usr.bin/vmstat/vmstat.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vmstat/vmstat.c,v
> retrieving revision 1.158
> diff -u -p -r1.158 vmstat.c
> --- usr.bin/vmstat/vmstat.c	1 May 2024 12:54:27 -0000	1.158
> +++ usr.bin/vmstat/vmstat.c	14 Jan 2025 10:39:58 -0000
> @@ -735,43 +735,22 @@ domem(void)
>  	struct kmemstats kmemstats[M_LAST], *ks;
>  	int i, j, len, size, first, mib[4];
>  	u_long totuse = 0, totfree = 0;
> -	char buf[BUFSIZ], *bufp, *ap;
>  	unsigned long long totreq = 0;
>  	const char *name;
>  	size_t siz;
>  
> -	if (memf == NULL && nlistf == NULL) {
> -		mib[0] = CTL_KERN;
> -		mib[1] = KERN_MALLOCSTATS;
> -		mib[2] = KERN_MALLOC_BUCKETS;
> -		siz = sizeof(buf);
> -		if (sysctl(mib, 3, buf, &siz, NULL, 0) == -1) {
> -			warnx("could not read kern.malloc.buckets");
> -			return;
> -		}
> +	mib[0] = CTL_KERN;
> +	mib[1] = KERN_MALLOCSTATS;
> +	mib[2] = KERN_MALLOC_BUCKET;
> +	siz = sizeof(struct kmembuckets);
>  
> -		bufp = buf;
> -		mib[2] = KERN_MALLOC_BUCKET;
> -		siz = sizeof(struct kmembuckets);
> -		i = 0;
> -		while ((ap = strsep(&bufp, ",")) != NULL) {
> -			const char *errstr;
> +	for (i = MINBUCKET; i < MINBUCKET + 16; i++) {
> +		mib[3] = (1<<i);
>  
> -			mib[3] = strtonum(ap, 0, INT_MAX, &errstr);
> -			if (errstr) {
> -				warnx("kernel lied about %d being a number", mib[3]);
> -				return;
> -			}
> -
> -			if (sysctl(mib, 4, &buckets[MINBUCKET + i], &siz,
> -			    NULL, 0) == -1) {
> -				warn("could not read kern.malloc.bucket.%d", mib[3]);
> -				return;
> -			}
> -			i++;
> +		if (sysctl(mib, 4, &buckets[i], &siz, NULL, 0) == -1) {
> +			warn("could not read kern.malloc.bucket.%d", mib[3]);
> +			return;
>  		}
> -	} else {
> -		kread(X_KMEMBUCKETS, buckets, sizeof(buckets));
>  	}
>  
>  	for (first = 1, i = MINBUCKET, kp = &buckets[i]; i < MINBUCKET + 16;

-- 
:wq Claudio