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?

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?

> 
> > 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);
> >  
> > 
> >