From: Theo Buehler <tb@openbsd.org>
Subject: Re: Make pdaemon understandable
To: sthen@openbsd.org, bluhm@openbsd.org, tech@openbsd.org
Date: Tue, 23 Dec 2025 12:58:58 +0100

On Tue, Dec 23, 2025 at 12:54:34PM +0100, Martin Pieuchot wrote:
> On 22/12/25(Mon) 20:39, Theo Buehler wrote:
> > 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.
> 
> Thanks a lot.  You're right, otherwise we skip the first page.  Here's
> an updated version of the diff that keeps the for () loop for now.

ok tb