Download raw body.
Parrallel upper fault
On 12/03/25(Wed) 14:33, Martin Pieuchot wrote:
> Diff below includes all the necessary bits, ported mostly from NetBSD,
> to run the upper part of the fault handler in parallel. It has been
> tested by many as part of a larger diff and I'd like to commit it
> without enabling it by default.
>
> Running the fault handler in parallel does work and it reduces the
> contention and sleeping time related to access of UVM data structures.
>
> That said, running the handler in parallel leads to a contention shifts
> on the KERNEL_LOCK at the boundary of the VFS. Obviously this doesn't
> help performances or even degrades them if the number of CPUs is too high.
>
> If you want to properly test this diff you'll need "anon pool cache"
> diff as well, otherwise all anon allocations will content on the pool's
> mutex.
>
> Comments? Oks?
Anyone?
> 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 12 Mar 2025 13:14:33 -0000
> @@ -177,6 +177,8 @@ uvm_anon_pagein(struct vm_amap *amap, st
> * anon was freed.
> */
> return FALSE;
> + case ENOLCK:
> + /* Should not be possible. */
> default:
> #ifdef DIAGNOSTIC
> panic("anon_pagein: uvmfault_anonget -> %d", rv);
> Index: uvm/uvm_fault.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> diff -u -p -r1.165 uvm_fault.c
> --- uvm/uvm_fault.c 10 Mar 2025 14:13:58 -0000 1.165
> +++ uvm/uvm_fault.c 12 Mar 2025 13:19:00 -0000
> @@ -277,6 +277,7 @@ uvmfault_anonget(struct uvm_faultinfo *u
> struct vm_anon *anon)
> {
> struct vm_page *pg;
> + int lock_type;
> int error;
>
> KASSERT(rw_lock_held(anon->an_lock));
> @@ -305,6 +306,7 @@ uvmfault_anonget(struct uvm_faultinfo *u
> /*
> * Is page resident? Make sure it is not busy/released.
> */
> + lock_type = rw_status(anon->an_lock);
> if (pg) {
> KASSERT(pg->pg_flags & PQ_ANON);
> KASSERT(pg->uanon == anon);
> @@ -326,8 +328,13 @@ uvmfault_anonget(struct uvm_faultinfo *u
> uvm_pagewait(pg, anon->an_lock, "anonget");
> } else {
> /*
> - * No page, therefore allocate one.
> + * No page, therefore allocate one. A write lock is
> + * required for this. If the caller didn't supply
> + * one, fail now and have them retry.
> */
> + if (lock_type == RW_READ) {
> + return ENOLCK;
> + }
> pg = uvm_pagealloc(NULL, 0, anon, 0);
> if (pg == NULL) {
> /* Out of memory. Wait a little. */
> @@ -498,6 +505,7 @@ uvmfault_promote(struct uvm_faultinfo *u
> if (uobjpage != PGO_DONTCARE)
> uobj = uobjpage->uobject;
>
> + KASSERT(rw_write_held(amap->am_lock));
> KASSERT(uobj == NULL || rw_lock_held(uobj->vmobjlock));
>
> anon = uvm_analloc();
> @@ -609,6 +617,7 @@ struct uvm_faultctx {
> boolean_t wired;
> paddr_t pa_flags;
> boolean_t promote;
> + int upper_lock_type;
> int lower_lock_type;
> };
>
> @@ -653,6 +662,7 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
> flt.access_type = access_type;
> flt.narrow = FALSE; /* assume normal fault for now */
> flt.wired = FALSE; /* assume non-wired fault for now */
> + flt.upper_lock_type = RW_READ;
> #if notyet
> flt.lower_lock_type = RW_READ; /* shared lock for now */
> #else
> @@ -840,7 +850,13 @@ uvm_fault_check(struct uvm_faultinfo *uf
> * if we've got an amap then lock it and extract current anons.
> */
> if (amap) {
> - amap_lock(amap, RW_WRITE);
> + if ((flt->access_type & PROT_WRITE) != 0) {
> + /*
> + * assume we're about to COW.
> + */
> + flt->upper_lock_type = RW_WRITE;
> + }
> + amap_lock(amap, flt->upper_lock_type);
> amap_lookups(&ufi->entry->aref,
> flt->startva - ufi->entry->start, *ranons, flt->npages);
> } else {
> @@ -892,6 +908,36 @@ uvm_fault_check(struct uvm_faultinfo *uf
> }
>
> /*
> + * uvm_fault_upper_upgrade: upgrade upper lock, reader -> writer
> + */
> +static inline int
> +uvm_fault_upper_upgrade(struct uvm_faultctx *flt, struct vm_amap *amap)
> +{
> + KASSERT(flt->upper_lock_type == rw_status(amap->am_lock));
> +
> + /*
> + * fast path.
> + */
> + if (flt->upper_lock_type == RW_WRITE) {
> + return 0;
> + }
> +
> + /*
> + * otherwise try for the upgrade. if we don't get it, unlock
> + * everything, restart the fault and next time around get a writer
> + * lock.
> + */
> + flt->upper_lock_type = RW_WRITE;
> + if (rw_enter(amap->am_lock, RW_UPGRADE|RW_NOSLEEP)) {
> + counters_inc(uvmexp_counters, flt_noup);
> + return ERESTART;
> + }
> + counters_inc(uvmexp_counters, flt_up);
> + KASSERT(flt->upper_lock_type == rw_status(amap->am_lock));
> + return 0;
> +}
> +
> +/*
> * uvm_fault_upper_lookup: look up existing h/w mapping and amap.
> *
> * iterate range of interest:
> @@ -914,9 +960,8 @@ uvm_fault_upper_lookup(struct uvm_faulti
> paddr_t pa;
> int lcv, entered = 0;
>
> - /* locked: maps(read), amap(if there) */
> KASSERT(amap == NULL ||
> - rw_write_held(amap->am_lock));
> + rw_status(amap->am_lock) == flt->upper_lock_type);
>
> /*
> * map in the backpages and frontpages we found in the amap in hopes
> @@ -998,8 +1043,7 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> struct vm_page *pg = NULL;
> int error, ret;
>
> - /* locked: maps(read), amap, anon */
> - KASSERT(rw_write_held(amap->am_lock));
> + KASSERT(rw_status(amap->am_lock) == flt->upper_lock_type);
> KASSERT(anon->an_lock == amap->am_lock);
>
> /*
> @@ -1012,6 +1056,7 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> * if it succeeds, locks are still valid and locked.
> * also, if it is OK, then the anon's page is on the queues.
> */
> +retry:
> error = uvmfault_anonget(ufi, amap, anon);
> switch (error) {
> case 0:
> @@ -1020,11 +1065,21 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> case ERESTART:
> return ERESTART;
>
> + case ENOLCK:
> + /* it needs a write lock: retry */
> + error = uvm_fault_upper_upgrade(flt, amap);
> + if (error != 0) {
> + uvmfault_unlockall(ufi, amap, NULL);
> + return error;
> + }
> + KASSERT(rw_write_held(amap->am_lock));
> + goto retry;
> +
> default:
> return error;
> }
>
> - KASSERT(rw_write_held(amap->am_lock));
> + KASSERT(rw_status(amap->am_lock) == flt->upper_lock_type);
> KASSERT(anon->an_lock == amap->am_lock);
>
> /*
> @@ -1039,9 +1094,13 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> *
> * if we are out of anon VM we wait for RAM to become available.
> */
> -
> if ((flt->access_type & PROT_WRITE) != 0 && anon->an_ref > 1) {
> /* promoting requires a write lock. */
> + error = uvm_fault_upper_upgrade(flt, amap);
> + if (error != 0) {
> + uvmfault_unlockall(ufi, amap, NULL);
> + return error;
> + }
> KASSERT(rw_write_held(amap->am_lock));
>
> counters_inc(uvmexp_counters, flt_acow);
> @@ -1064,6 +1123,14 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> KASSERT(oanon->an_ref > 1);
> oanon->an_ref--;
>
> + /*
> + * note: oanon is still locked, as is the new anon. we
> + * need to check for this later when we unlock oanon; if
> + * oanon != anon, we'll have to unlock anon, too.
> + */
> + KASSERT(anon->an_lock == amap->am_lock);
> + KASSERT(oanon->an_lock == amap->am_lock);
> +
> #if defined(MULTIPROCESSOR) && !defined(__HAVE_PMAP_MPSAFE_ENTER_COW)
> /*
> * If there are multiple threads, either uvm or the
> @@ -1078,12 +1145,6 @@ uvm_fault_upper(struct uvm_faultinfo *uf
> flt->access_type &= ~PROT_WRITE;
> }
> #endif
> -
> - /*
> - * note: anon is _not_ locked, but we have the sole references
> - * to in from amap.
> - * thus, no one can get at it until we are done with it.
> - */
> } else {
> counters_inc(uvmexp_counters, flt_anon);
> oanon = anon;
> @@ -1246,10 +1307,8 @@ uvm_fault_lower_lookup(
> * uvm_fault_lower_upgrade: upgrade lower lock, reader -> writer
> */
> static inline int
> -uvm_fault_lower_upgrade(struct uvm_faultinfo *ufi, struct uvm_faultctx *flt,
> - struct vm_amap *amap, struct uvm_object *uobj)
> +uvm_fault_lower_upgrade(struct uvm_faultctx *flt, struct uvm_object *uobj)
> {
> - KASSERT(uobj != NULL);
> KASSERT(flt->lower_lock_type == rw_status(uobj->vmobjlock));
>
> /*
> @@ -1265,7 +1324,6 @@ uvm_fault_lower_upgrade(struct uvm_fault
> */
> flt->lower_lock_type = RW_WRITE;
> if (rw_enter(uobj->vmobjlock, RW_UPGRADE|RW_NOSLEEP)) {
> - uvmfault_unlockall(ufi, amap, uobj);
> counters_inc(uvmexp_counters, flt_noup);
> return ERESTART;
> }
> @@ -1319,11 +1377,8 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> * made it BUSY.
> */
>
> - /*
> - * locked:
> - */
> KASSERT(amap == NULL ||
> - rw_write_held(amap->am_lock));
> + rw_status(amap->am_lock) == flt->upper_lock_type);
> KASSERT(uobj == NULL ||
> rw_status(uobj->vmobjlock) == flt->lower_lock_type);
>
> @@ -1392,6 +1447,11 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> KASSERT(amap != NULL);
>
> /* promoting requires a write lock. */
> + error = uvm_fault_upper_upgrade(flt, amap);
> + if (error != 0) {
> + uvmfault_unlockall(ufi, amap, uobj);
> + return error;
> + }
> KASSERT(rw_write_held(amap->am_lock));
> KASSERT(uobj == NULL ||
> rw_status(uobj->vmobjlock) == flt->lower_lock_type);
> @@ -1468,7 +1528,7 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> * Note: pg is either the uobjpage or the new page in the new anon.
> */
> KASSERT(amap == NULL ||
> - rw_write_held(amap->am_lock));
> + rw_status(amap->am_lock) == flt->upper_lock_type);
> KASSERT(uobj == NULL ||
> rw_status(uobj->vmobjlock) == flt->lower_lock_type);
> KASSERT(anon == NULL || anon->an_lock == amap->am_lock);
> @@ -1572,9 +1632,11 @@ uvm_fault_lower_io(
> advice = ufi->entry->advice;
>
> /* Upgrade to a write lock if needed. */
> - error = uvm_fault_lower_upgrade(ufi, flt, amap, uobj);
> - if (error != 0)
> + error = uvm_fault_lower_upgrade(flt, uobj);
> + if (error != 0) {
> + uvmfault_unlockall(ufi, amap, uobj);
> return error;
> + }
> uvmfault_unlockall(ufi, amap, NULL);
>
> /* update rusage counters */
> @@ -1610,7 +1672,7 @@ uvm_fault_lower_io(
> /* re-verify the state of the world. */
> locked = uvmfault_relock(ufi);
> if (locked && amap != NULL)
> - amap_lock(amap, RW_WRITE);
> + amap_lock(amap, flt->upper_lock_type);
>
> /* might be changed */
> if (pg != PGO_DONTCARE) {
> Index: uvm/uvm_page.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> diff -u -p -r1.182 uvm_page.c
> --- uvm/uvm_page.c 10 Mar 2025 14:13:58 -0000 1.182
> +++ uvm/uvm_page.c 12 Mar 2025 13:14:33 -0000
> @@ -1366,7 +1366,9 @@ uvm_page_owner_locked_p(struct vm_page *
> : rw_lock_held(pg->uobject->vmobjlock);
> }
> if (pg->uanon != NULL) {
> - return rw_write_held(pg->uanon->an_lock);
> + return exclusive
> + ? rw_write_held(pg->uanon->an_lock)
> + : rw_lock_held(pg->uanon->an_lock);
> }
> return 1;
> }
>
>
Parrallel upper fault