Download raw body.
vm_upgrade
On Fri, 15 Aug 2025 09:45:22 +0200,
Martin Pieuchot <mpi@grenadille.net> 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?
>
I had run this diff for last few days on my laptop.
So far I wasn't able to hit swap.
Is it expected?
> > Index: uvm/uvm_fault.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> > diff -u -p -r1.169 uvm_fault.c
> > --- uvm/uvm_fault.c 2 May 2025 10:19:09 -0000 1.169
> > +++ uvm/uvm_fault.c 23 Jun 2025 18:48:03 -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)
> > @@ -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);
> > + /* modifing `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 23 Jun 2025 18:48:03 -0000
> > @@ -5225,6 +5225,56 @@ vm_map_unlock_read_ln(struct vm_map *map
> > 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;
> > +}
> > +
> > +boolean_t
> > +vm_map_downgrade_ln(struct vm_map *map, char *file, int line)
> > +{
> > + int rv;
> > +
> > + if (map->flags & VM_MAP_INTRSAFE) {
> > + MUTEX_ASSERT_LOCKED(&map->mtx);
> > + } else {
> > + rv = rw_enter(&map->lock, RW_DOWNGRADE);
> > + if (rv != 0)
> > + return FALSE;
> > +
> > + }
> > +
> > + map->timestamp++;
> > + LPRINTF(("map downgrade: %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_busy_ln(struct vm_map *map, char *file, int line)
> > {
> > 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 23 Jun 2025 18:48:03 -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);
> > +boolean_t 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) \
> >
> >
>
>
--
wbr, Kirill
vm_upgrade