From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: anon pool cache
To: Martin Pieuchot <mpi@grenadille.net>, dlg@openbsd.org
Cc: tech@openbsd.org
Date: Mon, 14 Apr 2025 23:25:21 +0200

> Date: Mon, 14 Apr 2025 09:48:09 +0200
> From: Martin Pieuchot <mpi@grenadille.net>
> 
> On 10/03/25(Mon) 20:04, Martin Pieuchot wrote:
> > The mutex of the anon pool is one of the most contended global locks in
> > UVM.  Diff below makes it use a pool cache on MP kernels.  Without this,
> > running the upper part of the fault handler in parallel doesn't improve
> > anything.  This change already gives a small boost on its own.
> > 
> > Note that on LP64 "struct vm_anon" needs to grow a little bit...
> > 
> > ok?
> 
> This also help single-threaded performances...  Anyone?

In principle ok kettenis@

> > Index: uvm/uvm_anon.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_anon.c,v
> > diff -u -p -r1.62 uvm_anon.c
> > --- uvm/uvm_anon.c	10 Mar 2025 14:13:58 -0000	1.62
> > +++ uvm/uvm_anon.c	10 Mar 2025 18:45:51 -0000
> > @@ -50,6 +50,14 @@ uvm_anon_init(void)
> >  	pool_sethiwat(&uvm_anon_pool, uvmexp.free / 16);
> >  }
> >  
> > +void
> > +uvm_anon_init_percpu(void)
> > +{
> > +#ifdef MULTIPROCESSOR
> > +	pool_cache_init(&uvm_anon_pool);
> > +#endif
> > +}
> > +
> >  /*
> >   * uvm_analloc: allocate a new anon.
> >   *
> > Index: uvm/uvm_anon.h
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_anon.h,v
> > diff -u -p -r1.22 uvm_anon.h
> > --- uvm/uvm_anon.h	19 Jan 2021 13:21:36 -0000	1.22
> > +++ uvm/uvm_anon.h	10 Mar 2025 18:46:25 -0000
> > @@ -47,6 +47,10 @@ struct vm_anon {
> >  	 * Drum swap slot # (if != 0) [if we hold an_page, PG_BUSY]
> >  	 */
> >  	int an_swslot;
> > +
> > +#if defined(MULTIPROCESSOR) && defined(__LP64__)
> > +	long unused;			/* to match pool_cache_item's size */
> > +#endif

It took me a while to realize what you're trying to say here.  But the
per-CPU caching code requires to pool item to be at least the size of
struct pool_cache_item.  SO maybe spell it out like that?

That said, maybe we should add a PR_PERCPUCACHE flag to pool_init()
and bump up the size as required?  And check that the flag is set in
pool_cache_init() of course.

> >  };
> >  
> >  /*
> > @@ -84,6 +88,7 @@ void		 uvm_anfree_list(struct vm_anon *,
> >  void		 uvm_anon_release(struct vm_anon *);
> >  void		 uvm_anwait(void);
> >  void		 uvm_anon_init(void);
> > +void		 uvm_anon_init_percpu(void);
> >  void		 uvm_anon_dropswap(struct vm_anon *);
> >  boolean_t	 uvm_anon_pagein(struct vm_amap *, struct vm_anon *);
> >  
> > Index: uvm/uvm_init.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_init.c,v
> > diff -u -p -r1.42 uvm_init.c
> > --- uvm/uvm_init.c	20 Mar 2021 10:24:21 -0000	1.42
> > +++ uvm/uvm_init.c	10 Mar 2025 18:37:33 -0000
> > @@ -194,4 +194,6 @@ void
> >  uvm_init_percpu(void)
> >  {
> >  	uvmexp_counters = counters_alloc_ncpus(uvmexp_counters, exp_ncounters);
> > +
> > +	uvm_anon_init_percpu();
> >  }
> > 
> > 
> 
> 
>