> Date: Wed, 23 Oct 2024 18:17:50 +0200
> From: Martin Pieuchot <mpi@grenadille.net>
> 
> On 22/10/24(Tue) 21:14, Mark Kettenis wrote:
> > > Date: Sun, 20 Oct 2024 11:25:41 +0200
> > > From: Martin Pieuchot <mpi@grenadille.net>
> > > 
> > > Return the number of freed pages in uvm_swapout_threads().  This is
> > > part of my ongoing work to reduce accesses to global variables inside 
> > > the page deamon loop.
> > > 
> > > ok?
> > 
> > Hmm, but this still accesses uvmexp.free inside uvm_swapout_threads().
> > So I'm not sure this is an improvement.  And there is no guarantee
> > that the returned number of freed pages is accurate.
> 
> I agree this could be much better.  
> 
> The logic in the page daemon already access `uvmexp.free' multiple
> times.  This isn't an improvement but isn't worst.  It is good enough
> to get uvm_swapout_threads() out of my way for now because it is not
> used on amd64.
> 
> > Wouldn't it be better to make pmap_collect() return the number of
> > freed pages?  This is only actually implemented on i386 and it looks
> > fairly simple to count the number of ptp pages freed in
> > pmap_do_remove_86() and pmap_do_remove_pae() and have them return that
> > number.
> 
> I'd appreciate any help with that.   However there are other archs apart 
> from i386, what about sparc64, mips64, m88k and alpha?

Hmm, so I grepped for pmap_collect in arch/*/include*.  And I should
have grepped for PMAP_COLLECT.  I guess we need to recruit miod@ for
testing a diff.

> Can I get an ok for this diff so I can make progress with my work then
> we get rid of the `uvmexp.free' access with the changes you suggest?

How important is it that the returned value is correct?  I mean, your
implementation may return a non-zero value even if pmap_collect()
wasn't called.

But if it doesn't matter, sure.  But please add an XXX comment.

> > 
> > > Index: uvm/uvm_glue.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/uvm/uvm_glue.c,v
> > > diff -u -p -r1.85 uvm_glue.c
> > > --- uvm/uvm_glue.c	8 Oct 2024 02:29:10 -0000	1.85
> > > +++ uvm/uvm_glue.c	20 Oct 2024 09:20:09 -0000
> > > @@ -339,13 +339,13 @@ int	swapdebug = 0;
> > >   *   are swapped... otherwise the longest-sleeping or stopped process
> > >   *   is swapped, otherwise the longest resident process...
> > >   */
> > > -void
> > > +int
> > >  uvm_swapout_threads(void)
> > >  {
> > >  	struct process *pr;
> > >  	struct proc *p, *slpp;
> > >  	struct process *outpr;
> > > -	int outpri;
> > > +	int free, outpri;
> > >  	int didswap = 0;
> > >  	extern int maxslp; 
> > >  	/* XXXCDC: should move off to uvmexp. or uvm., also in uvm_meter */
> > > @@ -355,6 +355,8 @@ uvm_swapout_threads(void)
> > >  		return;
> > >  #endif
> > >  
> > > +	free = uvmexp.free;
> > > +
> > >  	/*
> > >  	 * outpr/outpri  : stop/sleep process whose most active thread has
> > >  	 *	the largest sleeptime < maxslp
> > > @@ -403,8 +405,7 @@ next_process:	;
> > >  	 * if we are real low on memory since we don't gain much by doing
> > >  	 * it.
> > >  	 */
> > > -	if (didswap == 0 && uvmexp.free <= atop(round_page(USPACE)) &&
> > > -	    outpr != NULL) {
> > > +	if (didswap == 0 && free <= atop(round_page(USPACE)) && outpr != NULL) {
> > >  #ifdef DEBUG
> > >  		if (swapdebug & SDB_SWAPOUT)
> > >  			printf("swapout_threads: no duds, try procpr %p\n",
> > > @@ -412,6 +413,8 @@ next_process:	;
> > >  #endif
> > >  		pmap_collect(outpr->ps_vmspace->vm_map.pmap);
> > >  	}
> > > +
> > > +	return (uvmexp.free - free);
> > >  }
> > >  
> > >  #endif	/* __HAVE_PMAP_COLLECT */
> > > Index: uvm/uvm_glue.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/uvm/uvm_glue.h,v
> > > diff -u -p -r1.9 uvm_glue.h
> > > --- uvm/uvm_glue.h	11 Jul 2014 16:35:40 -0000	1.9
> > > +++ uvm/uvm_glue.h	20 Oct 2024 09:20:09 -0000
> > > @@ -37,7 +37,7 @@
> > >   * uvm_glue.h
> > >   */
> > >  
> > > -void uvm_swapout_threads(void);
> > > +int uvm_swapout_threads(void);
> > >  
> > >  struct vm_page	*uvm_atopg(vaddr_t);
> > >  
> > > 
> > > 
>