From: Jeremie Courreges-Anglas Subject: Re: Newest uvm_purge() to try To: Mark Kettenis Cc: Martin Pieuchot , tech@openbsd.org Date: Thu, 22 May 2025 19:27:43 +0200 On Thu, May 22, 2025 at 02:45:54PM +0200, Mark Kettenis wrote: > > Date: Thu, 22 May 2025 11:58:38 +0200 > > From: Martin Pieuchot > > > > Here's the latest version incorporating kettenis@'s pmap_purge() and > > claudio@'s feedbacks. I observed a performance improvement of 10-15% > > on workloads using 24 to 48 CPUs. %sys time obviously increases now > > that tearing down the VM space is accounted for. sys time doesn't increase as much as I would have expected on this mac mini M2 pro. make -j10 in libc: 20250521 1m10.05s real 1m50.54s user 5m54.05s system 1m10.81s real 1m50.38s user 5m57.66s system 1m10.44s real 1m49.87s user 5m55.47s system 1m10.99s real 1m50.18s user 5m54.85s system 1m10.74s real 1m48.01s user 5m57.09s system 1m11.04s real 1m49.45s user 5m56.35s system 1m11.14s real 1m48.92s user 5m58.13s system 1m11.00s real 1m48.48s user 5m59.10s system 1m11.37s real 1m49.11s user 5m56.68s system 1m11.06s real 1m49.18s user 5m55.19s system 1m11.06s real 1m49.67s user 5m52.18s system 1m10.97s real 1m48.85s user 5m54.57s system 1m11.44s real 1m49.09s user 5m58.65s system 1m11.90s real 1m48.90s user 5m56.11s system After your diff: 1m05.25s real 1m50.20s user 5m58.32s system 1m05.55s real 1m50.74s user 5m59.72s system 1m05.50s real 1m51.12s user 5m59.94s system 1m05.44s real 1m50.80s user 6m01.66s system 1m05.65s real 1m50.54s user 6m00.15s system 1m05.81s real 1m50.88s user 5m59.62s system 1m06.00s real 1m50.12s user 6m02.71s system 1m05.61s real 1m51.20s user 6m02.00s system 1m05.52s real 1m51.79s user 5m59.73s system 1m05.55s real 1m50.19s user 5m59.93s system 1m05.46s real 1m51.53s user 6m00.99s system 1m05.79s real 1m51.27s user 6m01.16s system 1m06.04s real 1m51.83s user 6m02.29s system 1m05.30s real 1m51.02s user 6m01.64s system 1m05.63s real 1m51.30s user 6m02.40s system 1m05.67s real 1m50.10s user 6m03.47s system 1m05.56s real 1m52.64s user 5m59.89s system 1m05.76s real 1m51.73s user 6m00.28s system 1m05.51s real 1m53.57s user 5m57.90s system Maybe the recent uvm and arm64 pmap optimizations are neutering the increase? > > Please test and report back. Sure. > Going to play with this! Yep. I'm happy you folks found a satisfactory way forward. At first I was dubious about this uvm_purge() proposal. > I have a question I wanted to ask about the diff though... Please also find one typo fix inline. > > Index: kern/kern_exit.c > > =================================================================== > > RCS file: /cvs/src/sys/kern/kern_exit.c,v > > diff -u -p -r1.248 kern_exit.c > > --- kern/kern_exit.c 21 May 2025 09:42:59 -0000 1.248 > > +++ kern/kern_exit.c 22 May 2025 09:30:43 -0000 > > @@ -242,9 +242,14 @@ exit1(struct proc *p, int xexit, int xsi > > if (pr->ps_pptr->ps_sigacts->ps_sigflags & SAS_NOCLDWAIT) > > atomic_setbits_int(&pr->ps_flags, PS_NOZOMBIE); > > > > -#ifdef __HAVE_PMAP_PURGE > > - pmap_purge(p); > > + /* Teardown the virtual address space. */ > > + if ((p->p_flag & P_SYSTEM) == 0) { > > +#ifdef MULTIPROCESSOR > > + __mp_release_all(&kernel_lock); > > Why do we need an __mp_release_all() here? Is that because we have > multiple paths into exit1() that have different recurse counts for the > kernel lock? FWIW in Bruges we learnt that a exit1() => single_thread_smth() => exit1() recursion was possible. It feels like this is no longer the case after one of claudio's commits, but maybe mpi has other paths in mind? > > #endif > > + uvm_purge(); > > + KERNEL_LOCK(); > > + } > > } > > > > p->p_fd = NULL; /* zap the thread's copy */ > > Index: uvm/uvm_extern.h > > =================================================================== > > RCS file: /cvs/src/sys/uvm/uvm_extern.h,v > > diff -u -p -r1.183 uvm_extern.h > > --- uvm/uvm_extern.h 21 May 2025 16:59:32 -0000 1.183 > > +++ uvm/uvm_extern.h 22 May 2025 09:30:43 -0000 > > @@ -268,6 +268,7 @@ int uvm_fault(vm_map_t, vaddr_t, vm_fa > > > > vaddr_t uvm_uarea_alloc(void); > > void uvm_uarea_free(struct proc *); > > +void uvm_purge(void); > > void uvm_exit(struct process *); > > void uvm_init_limits(struct plimit *); > > boolean_t uvm_kernacc(caddr_t, size_t, int); > > @@ -401,6 +402,7 @@ void uvmspace_init(struct vmspace *, s > > void uvmspace_exec(struct proc *, vaddr_t, vaddr_t); > > struct vmspace *uvmspace_fork(struct process *); > > void uvmspace_addref(struct vmspace *); > > +void uvmspace_purge(struct vmspace *); > > void uvmspace_free(struct vmspace *); > > struct vmspace *uvmspace_share(struct process *); > > int uvm_sysctl(int *, u_int, void *, size_t *, > > Index: uvm/uvm_glue.c > > =================================================================== > > RCS file: /cvs/src/sys/uvm/uvm_glue.c,v > > diff -u -p -r1.90 uvm_glue.c > > --- uvm/uvm_glue.c 20 May 2025 07:02:20 -0000 1.90 > > +++ uvm/uvm_glue.c 22 May 2025 09:30:43 -0000 > > @@ -286,6 +286,25 @@ uvm_uarea_free(struct proc *p) > > } > > > > /* > > + * uvm_purge: teardown a virtual address space. > > + * > > + * If multi-threaded, must be called by the last thread of a process. > > + */ > > +void > > +uvm_purge(void) > > +{ > > + struct proc *p = curproc; > > + struct vmspace *vm = p->p_vmspace; > > + > > + KERNEL_ASSERT_UNLOCKED(); > > + > > +#ifdef __HAVE_PMAP_PURGE > > + pmap_purge(p); > > +#endif > > + uvmspace_purge(vm); > > +} > > + > > +/* > > * uvm_exit: exit a virtual address space > > */ > > void > > Index: uvm/uvm_map.c > > =================================================================== > > RCS file: /cvs/src/sys/uvm/uvm_map.c,v > > diff -u -p -r1.344 uvm_map.c > > --- uvm/uvm_map.c 21 May 2025 16:59:32 -0000 1.344 > > +++ uvm/uvm_map.c 22 May 2025 09:43:53 -0000 > > @@ -2491,6 +2491,24 @@ uvm_map_teardown(struct vm_map *map) > > entry = TAILQ_NEXT(entry, dfree.deadq); > > } > > > > +#ifdef VMMAP_DEBUG > > + numt = numq = 0; > > + RBT_FOREACH(entry, uvm_map_addr, &map->addr) > > + numt++; > > + TAILQ_FOREACH(entry, &dead_entries, dfree.deadq) > > + numq++; > > + KASSERT(numt == numq); > > + /* > > + * Let VMMAP_DEBUG checks work on an empty map if uvm_map_teardown() > > + * is called twice. > > + */ > > + map->size = 0; > > + map->min_offset = 0; > > + map->max_offset = 0; > > + map->flags &= ~VM_MAP_ISVMSPACE; > > +#endif > > + /* reinit RB tree since the above removal leaves the tree corrupted. */ > > + RBT_INIT(uvm_map_addr, &map->addr); > > vm_map_unlock(map); > > > > /* Remove address selectors. */ > > @@ -2503,18 +2521,7 @@ uvm_map_teardown(struct vm_map *map) > > uvm_addr_destroy(map->uaddr_brk_stack); > > map->uaddr_brk_stack = NULL; > > > > -#ifdef VMMAP_DEBUG > > - numt = numq = 0; > > - RBT_FOREACH(entry, uvm_map_addr, &map->addr) > > - numt++; > > - TAILQ_FOREACH(entry, &dead_entries, dfree.deadq) > > - numq++; > > - KASSERT(numt == numq); > > -#endif > > uvm_unmap_detach(&dead_entries, 0); > > - > > - pmap_destroy(map->pmap); > > - map->pmap = NULL; > > } > > > > /* > > @@ -3395,6 +3402,24 @@ uvmspace_addref(struct vmspace *vm) > > atomic_inc_int(&vm->vm_refcnt); > > } > > > > +void > > +uvmspace_purge(struct vmspace *vm) > > +{ > > +#ifdef SYSVSHM > > + /* Get rid of any SYSV shared memory segments. */ > > + if (vm->vm_shm != NULL) { > > + KERNEL_LOCK(); > > + shmexit(vm); > > + KERNEL_UNLOCK(); > > + } > > +#endif > > + /* > > + * Lock the map, to wait out all other references to it. delete > > + * all of the mappings and pages they hold. > > + */ > > + uvm_map_teardown(&vm->vm_map); > > +} > > + > > /* > > * uvmspace_free: free a vmspace data structure > > */ > > @@ -3403,20 +3428,15 @@ uvmspace_free(struct vmspace *vm) > > { > > if (atomic_dec_int_nv(&vm->vm_refcnt) == 0) { > > /* > > - * lock the map, to wait out all other references to it. delete > > - * all of the mappings and pages they hold, then call the pmap > > - * module to reclaim anything left. > > + * Sanity check. Kernel threads never end up here and > > + * userland ones allready teardown there VM space in => * userland ones already tear down their VM space in > > + * exit1(). > > */ > > -#ifdef SYSVSHM > > - /* Get rid of any SYSV shared memory segments. */ > > - if (vm->vm_shm != NULL) { > > - KERNEL_LOCK(); > > - shmexit(vm); > > - KERNEL_UNLOCK(); > > - } > > -#endif > > + uvmspace_purge(vm); > > + > > + pmap_destroy(vm->vm_map.pmap); > > + vm->vm_map.pmap = NULL; > > > > - uvm_map_teardown(&vm->vm_map); > > pool_put(&uvm_vmspace_pool, vm); > > } > > } > > > > > > > -- jca