Download raw body.
vm_upgrade
> Date: Thu, 4 Sep 2025 17:44:05 +0200
> From: Martin Pieuchot <mpi@grenadille.net>
>
> On 02/09/25(Tue) 20:52, Martin Pieuchot wrote:
> > On 15/08/25(Fri) 09:45, Martin Pieuchot wrote:
> > > On 22/07/25(Tue) 16:54, Martin Pieuchot wrote:
> > > > Instead of doing an unlock & relock dance before calling amap_copy() we
> > > > can simply upgrade the lock. This improves page fault performances from
> > > > 1%.
> > > >
> > > > As explained in the other thread we can't do better for the moment.
> > > >
> > > > ok?
> > >
> > > Anyone?
> >
> > Updated diff including a fix for a read vs write unlock in error path
> > found by sthen@.
>
> Updated diff after some inputs from mlarkin@:
> - fix some KNF & spelling
> - turn vm_map_downgrade() into a void, it always succeed.
> - do not modify the map's timestamp in vm_map_downgrade()
>
> ok?
I'm a bit hesitant to ok non-trivial uvm diffs at this stage in the
release cycle, but I think this one is afirly narrow.
ok kettenis@
> Index: uvm/uvm_fault.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> diff -u -p -r1.170 uvm_fault.c
> --- uvm/uvm_fault.c 14 Jul 2025 08:45:16 -0000 1.170
> +++ uvm/uvm_fault.c 4 Sep 2025 15:26:30 -0000
> @@ -158,7 +158,6 @@ static struct uvm_advice uvmadvice[MADV_
> /*
> * private prototypes
> */
> -static void uvmfault_amapcopy(struct uvm_faultinfo *);
> static inline void uvmfault_anonflush(struct vm_anon **, int);
> void uvmfault_unlockmaps(struct uvm_faultinfo *, boolean_t);
> void uvmfault_update_stats(struct uvm_faultinfo *);
> @@ -219,49 +218,6 @@ uvmfault_init(void)
> }
>
> /*
> - * uvmfault_amapcopy: clear "needs_copy" in a map.
> - *
> - * => called with VM data structures unlocked (usually, see below)
> - * => we get a write lock on the maps and clear needs_copy for a VA
> - * => if we are out of RAM we sleep (waiting for more)
> - */
> -static void
> -uvmfault_amapcopy(struct uvm_faultinfo *ufi)
> -{
> - for (;;) {
> - /*
> - * no mapping? give up.
> - */
> - if (uvmfault_lookup(ufi, TRUE) == FALSE)
> - return;
> -
> - /*
> - * copy if needed.
> - */
> - if (UVM_ET_ISNEEDSCOPY(ufi->entry))
> - amap_copy(ufi->map, ufi->entry, M_NOWAIT,
> - UVM_ET_ISSTACK(ufi->entry) ? FALSE : TRUE,
> - ufi->orig_rvaddr, ufi->orig_rvaddr + 1);
> -
> - /*
> - * didn't work? must be out of RAM. unlock and sleep.
> - */
> - if (UVM_ET_ISNEEDSCOPY(ufi->entry)) {
> - uvmfault_unlockmaps(ufi, TRUE);
> - uvm_wait("fltamapcopy");
> - continue;
> - }
> -
> - /*
> - * got it! unlock and return.
> - */
> - uvmfault_unlockmaps(ufi, TRUE);
> - return;
> - }
> - /*NOTREACHED*/
> -}
> -
> -/*
> * uvmfault_anonget: get data in an anon into a non-busy, non-released
> * page in that anon.
> *
> @@ -734,14 +690,15 @@ uvm_fault_check(struct uvm_faultinfo *uf
> struct vm_amap *amap;
> struct uvm_object *uobj;
> int nback, nforw;
> + boolean_t write_locked = FALSE;
>
> /*
> * lookup and lock the maps
> */
> - if (uvmfault_lookup(ufi, FALSE) == FALSE) {
> +lookup:
> + if (uvmfault_lookup(ufi, write_locked) == FALSE) {
> return EFAULT;
> }
> - /* locked: maps(read) */
>
> #ifdef DIAGNOSTIC
> if ((ufi->map->flags & VM_MAP_PAGEABLE) == 0)
> @@ -753,7 +710,7 @@ uvm_fault_check(struct uvm_faultinfo *uf
> * check protection
> */
> if ((ufi->entry->protection & flt->access_type) != flt->access_type) {
> - uvmfault_unlockmaps(ufi, FALSE);
> + uvmfault_unlockmaps(ufi, write_locked);
> return EACCES;
> }
>
> @@ -779,11 +736,27 @@ uvm_fault_check(struct uvm_faultinfo *uf
> if (UVM_ET_ISNEEDSCOPY(ufi->entry)) {
> if ((flt->access_type & PROT_WRITE) ||
> (ufi->entry->object.uvm_obj == NULL)) {
> - /* need to clear */
> - uvmfault_unlockmaps(ufi, FALSE);
> - uvmfault_amapcopy(ufi);
> + /* modifying `ufi->entry' requires write lock */
> + if (!write_locked) {
> + write_locked = TRUE;
> + if (!vm_map_upgrade(ufi->map)) {
> + uvmfault_unlockmaps(ufi, FALSE);
> + goto lookup;
> + }
> + }
> +
> + amap_copy(ufi->map, ufi->entry, M_NOWAIT,
> + UVM_ET_ISSTACK(ufi->entry) ? FALSE : TRUE,
> + ufi->orig_rvaddr, ufi->orig_rvaddr + 1);
> +
> + /* didn't work? must be out of RAM. */
> + if (UVM_ET_ISNEEDSCOPY(ufi->entry)) {
> + uvmfault_unlockmaps(ufi, write_locked);
> + uvm_wait("fltamapcopy");
> + return ERESTART;
> + }
> +
> counters_inc(uvmexp_counters, flt_amcopy);
> - return ERESTART;
> } else {
> /*
> * ensure that we pmap_enter page R/O since
> @@ -791,6 +764,11 @@ uvm_fault_check(struct uvm_faultinfo *uf
> */
> flt->enter_prot &= ~PROT_WRITE;
> }
> + }
> +
> + if (write_locked) {
> + vm_map_downgrade(ufi->map);
> + write_locked = FALSE;
> }
>
> /*
> Index: uvm/uvm_map.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> diff -u -p -r1.345 uvm_map.c
> --- uvm/uvm_map.c 3 Jun 2025 08:38:17 -0000 1.345
> +++ uvm/uvm_map.c 4 Sep 2025 15:38:00 -0000
> @@ -5223,6 +5225,51 @@ vm_map_unlock_read_ln(struct vm_map *map
> rw_exit_read(&map->lock);
> else
> mtx_leave(&map->mtx);
> +}
> +
> +boolean_t
> +vm_map_upgrade_ln(struct vm_map *map, char *file, int line)
> +{
> + int rv;
> +
> + if (map->flags & VM_MAP_INTRSAFE) {
> + MUTEX_ASSERT_LOCKED(&map->mtx);
> + } else {
> + struct proc *busy;
> +
> + mtx_enter(&map->flags_lock);
> + busy = map->busy;
> + mtx_leave(&map->flags_lock);
> + if (busy != NULL && busy != curproc)
> + return FALSE;
> +
> + rv = rw_enter(&map->lock, RW_UPGRADE|RW_NOSLEEP);
> + if (rv != 0)
> + return FALSE;
> + }
> +
> + map->timestamp++;
> + LPRINTF(("map upgrade: %p (at %s %d)\n", map, file, line));
> + uvm_tree_sanity(map, file, line);
> + uvm_tree_size_chk(map, file, line);
> +
> + return TRUE;
> +}
> +
> +void
> +vm_map_downgrade_ln(struct vm_map *map, char *file, int line)
> +{
> + int rv;
> +
> + uvm_tree_sanity(map, file, line);
> + uvm_tree_size_chk(map, file, line);
> + LPRINTF(("map downgrade: %p (at %s %d)\n", map, file, line));
> + if (map->flags & VM_MAP_INTRSAFE) {
> + MUTEX_ASSERT_LOCKED(&map->mtx);
> + } else {
> + rv = rw_enter(&map->lock, RW_DOWNGRADE);
> + KASSERT(rv == 0);
> + }
> }
>
> void
> Index: uvm/uvm_map.h
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_map.h,v
> diff -u -p -r1.94 uvm_map.h
> --- uvm/uvm_map.h 15 Nov 2024 02:59:23 -0000 1.94
> +++ uvm/uvm_map.h 4 Sep 2025 15:36:38 -0000
> @@ -402,6 +402,8 @@ void vm_map_lock_ln(struct vm_map*, cha
> void vm_map_lock_read_ln(struct vm_map*, char*, int);
> void vm_map_unlock_ln(struct vm_map*, char*, int);
> void vm_map_unlock_read_ln(struct vm_map*, char*, int);
> +boolean_t vm_map_upgrade_ln(struct vm_map*, char*, int);
> +void vm_map_downgrade_ln(struct vm_map*, char*, int);
> void vm_map_busy_ln(struct vm_map*, char*, int);
> void vm_map_unbusy_ln(struct vm_map*, char*, int);
> void vm_map_assert_anylock_ln(struct vm_map*, char*, int);
> @@ -413,6 +415,8 @@ void vm_map_assert_wrlock_ln(struct vm_
> #define vm_map_lock_read(map) vm_map_lock_read_ln(map, __FILE__, __LINE__)
> #define vm_map_unlock(map) vm_map_unlock_ln(map, __FILE__, __LINE__)
> #define vm_map_unlock_read(map) vm_map_unlock_read_ln(map, __FILE__, __LINE__)
> +#define vm_map_upgrade(map) vm_map_upgrade_ln(map, __FILE__, __LINE__)
> +#define vm_map_downgrade(map) vm_map_downgrade_ln(map, __FILE__, __LINE__)
> #define vm_map_busy(map) vm_map_busy_ln(map, __FILE__, __LINE__)
> #define vm_map_unbusy(map) vm_map_unbusy_ln(map, __FILE__, __LINE__)
> #define vm_map_assert_anylock(map) \
>
>
>
vm_upgrade