From: Theo Buehler <tb@openbsd.org>
Subject: Re: Make pdaemon understandable
To: sthen@openbsd.org, bluhm@openbsd.org, tech@openbsd.org
Date: Mon, 22 Dec 2025 20:39:27 +0100

On Mon, Dec 22, 2025 at 12:11:47PM +0100, Martin Pieuchot wrote:
> Diff below has been generated with "-b" to be easily reviewable.
> Attached is the one I intend to commit which mostly includes indent
> changes.
> 
> It moves the final pageout of swap cluster outside of the loop.  This
> allows to remove the NULL checks and make the body of the loop readable.
> 
> I intentionally didn't change anything *inside* the loop even if we now
> see incoherency.
> 

This reads mostly fine. One thing:

> @@ -633,14 +633,7 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
>  
>  	/* Insert iterator. */
>  	TAILQ_INSERT_AFTER(pglst, p, &iter, pageq);
> -	for (; p != NULL || swc.swc_slot != 0; p = uvmpd_iterator(pglst, p, &iter)) {
> -		/*
> -		 * note that p can be NULL iff we have traversed the whole
> -		 * list and need to do one final swap-backed clustered pageout.
> -		 */
> -		uobj = NULL;
> -		anon = NULL;
> -		if (p) {
> +	while ((p = uvmpd_iterator(pglst, p, &iter)) != NULL) {

I'm confused by this change. I would have expected a

do { ... } while ((p = uvmpd_iterator(pglst, p, &iter)) != NULL);

loop. The first time around in the for loop things are applied to p
itself, not to nextpg as returned by the iterator.