The branch main has been updated by alc: URL: https://cgit.FreeBSD.org/src/commit/?id=37321270b630d891e48851f81dd6d077ebd6b635
commit 37321270b630d891e48851f81dd6d077ebd6b635 Author: Alan Cox <[email protected]> AuthorDate: 2026-09-07 07:31:45 +0000 Commit: Alan Cox <[email protected]> CommitDate: 2026-09-07 17:33:56 +0000 amd64/arm64 pmap: consistently clear PGA_WRITEABLE We don't consistently clear PGA_WRITEABLE on fictitious, managed pages. Some functions do, e.g., pmap_remove_all(), but several do not. At worst, this is just a pessimization, but there is no good reason to be inconsistent. Clear PGA_WRITEABLE in those that previously did not by introducing (and using) the helper function pmap_page_is_mapped_locked() that implements the correct test. Reviewed by: kib, markj MFC after: 3 weeks Differential Revision: https://reviews.freebsd.org/D59466 --- sys/amd64/amd64/pmap.c | 53 ++++++++++++++++++++--------------------- sys/arm64/arm64/pmap.c | 64 ++++++++++++++++++++++---------------------------- 2 files changed, 53 insertions(+), 64 deletions(-) diff --git a/sys/amd64/amd64/pmap.c b/sys/amd64/amd64/pmap.c index 9cf1c77f5a9a..eee310cc44b8 100644 --- a/sys/amd64/amd64/pmap.c +++ b/sys/amd64/amd64/pmap.c @@ -1367,6 +1367,7 @@ static void pmap_invalidate_pde_page(pmap_t pmap, vm_offset_t va, static void pmap_kenter_attr(vm_offset_t va, vm_paddr_t pa, int mode); static vm_page_t pmap_large_map_getptp_unlocked(void); static vm_paddr_t pmap_large_map_kextract(vm_offset_t va); +static bool pmap_page_is_mapped_locked(vm_page_t m); #if VM_NRESERVLEVEL > 0 static bool pmap_promote_pde(pmap_t pmap, pd_entry_t *pde, vm_offset_t va, vm_page_t mpte, struct rwlock **lockp); @@ -5199,7 +5200,6 @@ reclaim_pv_chunk_domain(pmap_t locked_pmap, struct rwlock **lockp, int domain) struct pv_chunks_list *pvc; struct pv_chunk *pc, *pc_marker, *pc_marker_end; struct pv_chunk_header pc_marker_b, pc_marker_end_b; - struct md_page *pvh; pd_entry_t *pde; pmap_t next_pmap, pmap; pt_entry_t *pte, tpte; @@ -5319,14 +5319,8 @@ reclaim_pv_chunk_domain(pmap_t locked_pmap, struct rwlock **lockp, int domain) CHANGE_PV_LIST_LOCK_TO_VM_PAGE(lockp, m); TAILQ_REMOVE(&m->md.pv_list, pv, pv_next); m->md.pv_gen++; - if (TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = pa_to_pvh(VM_PAGE_TO_PHYS(m)); - if (TAILQ_EMPTY(&pvh->pv_list)) { - vm_page_aflag_clear(m, - PGA_WRITEABLE); - } - } + if (!pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); pmap_delayed_invl_page(m); pc->pc_map[field] |= 1UL << bit; pmap_unuse_pt(pmap, va, *pde, &free); @@ -6212,7 +6206,6 @@ static int pmap_remove_pte(pmap_t pmap, pt_entry_t *ptq, vm_offset_t va, pd_entry_t ptepde, struct spglist *free, struct rwlock **lockp) { - struct md_page *pvh; pt_entry_t oldpte, PG_A, PG_M, PG_RW; vm_page_t m; @@ -6233,12 +6226,8 @@ pmap_remove_pte(pmap_t pmap, pt_entry_t *ptq, vm_offset_t va, vm_page_aflag_set(m, PGA_REFERENCED); CHANGE_PV_LIST_LOCK_TO_VM_PAGE(lockp, m); pmap_pvh_free(&m->md, pmap, va); - if (TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = pa_to_pvh(VM_PAGE_TO_PHYS(m)); - if (TAILQ_EMPTY(&pvh->pv_list)) - vm_page_aflag_clear(m, PGA_WRITEABLE); - } + if (!pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); pmap_delayed_invl_page(m); } return (pmap_unuse_pt(pmap, va, ptepde, free)); @@ -7287,10 +7276,13 @@ retry: ("pmap_enter: no PV entry for %#lx", va)); if ((newpte & PG_MANAGED) == 0) free_pv_entry(pmap, pv); + + /* + * The old page is likely COW, so check "writeable" + * first. + */ if ((om->a.flags & PGA_WRITEABLE) != 0 && - TAILQ_EMPTY(&om->md.pv_list) && - ((om->flags & PG_FICTITIOUS) != 0 || - TAILQ_EMPTY(&pa_to_pvh(opa)->pv_list))) + !pmap_page_is_mapped_locked(om)) vm_page_aflag_clear(om, PGA_WRITEABLE); } else { /* @@ -8451,13 +8443,22 @@ pmap_page_is_mapped(vm_page_t m) return (false); lock = VM_PAGE_TO_PV_LIST_LOCK(m); rw_rlock(lock); - rv = !TAILQ_EMPTY(&m->md.pv_list) || - ((m->flags & PG_FICTITIOUS) == 0 && - !TAILQ_EMPTY(&pa_to_pvh(VM_PAGE_TO_PHYS(m))->pv_list)); + rv = pmap_page_is_mapped_locked(m); rw_runlock(lock); return (rv); } +/* + * The page's PV list lock must be held. + */ +static __always_inline bool +pmap_page_is_mapped_locked(vm_page_t m) +{ + return (!TAILQ_EMPTY(&m->md.pv_list) || + ((m->flags & PG_FICTITIOUS) == 0 && + !TAILQ_EMPTY(&pa_to_pvh(VM_PAGE_TO_PHYS(m))->pv_list))); +} + /* * Destroy all managed, non-wired mappings in the given user-space * pmap. This pmap cannot be active on any processor besides the @@ -8649,12 +8650,8 @@ pmap_remove_pages(pmap_t pmap) TAILQ_REMOVE(&m->md.pv_list, pv, pv_next); m->md.pv_gen++; if ((m->a.flags & PGA_WRITEABLE) != 0 && - TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = pa_to_pvh(VM_PAGE_TO_PHYS(m)); - if (TAILQ_EMPTY(&pvh->pv_list)) - vm_page_aflag_clear(m, PGA_WRITEABLE); - } + !pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); } pmap_unuse_pt(pmap, pv->pv_va, ptepde, &free); #ifdef PV_STATS diff --git a/sys/arm64/arm64/pmap.c b/sys/arm64/arm64/pmap.c index a584812df0bb..f9b3217c9678 100644 --- a/sys/arm64/arm64/pmap.c +++ b/sys/arm64/arm64/pmap.c @@ -570,6 +570,7 @@ static int pmap_insert_pt_page(pmap_t pmap, vm_page_t mpte, bool promoted, static pt_entry_t pmap_load_l3c(pt_entry_t *l3p); static void pmap_mask_set_l3c(pmap_t pmap, pt_entry_t *l3p, vm_offset_t va, vm_offset_t *vap, vm_offset_t va_next, pt_entry_t mask, pt_entry_t nbits); +static bool pmap_page_is_mapped_locked(vm_page_t m); static bool pmap_pv_insert_l3c(pmap_t pmap, vm_offset_t va, vm_page_t m, struct rwlock **lockp); static void pmap_remove_kernel_l2(pmap_t pmap, pt_entry_t *l2, vm_offset_t va); @@ -3563,7 +3564,6 @@ reclaim_pv_chunk_domain(pmap_t locked_pmap, struct rwlock **lockp, int domain) struct pv_chunks_list *pvc; struct pv_chunk *pc, *pc_marker, *pc_marker_end; struct pv_chunk_header pc_marker_b, pc_marker_end_b; - struct md_page *pvh; pd_entry_t *pde; pmap_t next_pmap, pmap; pt_entry_t *pte, tpte; @@ -3665,14 +3665,8 @@ reclaim_pv_chunk_domain(pmap_t locked_pmap, struct rwlock **lockp, int domain) CHANGE_PV_LIST_LOCK_TO_VM_PAGE(lockp, m); TAILQ_REMOVE(&m->md.pv_list, pv, pv_next); m->md.pv_gen++; - if (TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = page_to_pvh(m); - if (TAILQ_EMPTY(&pvh->pv_list)) { - vm_page_aflag_clear(m, - PGA_WRITEABLE); - } - } + if (!pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); pc->pc_map[field] |= 1UL << bit; pmap_unuse_pt(pmap, va, pmap_load(pde), &free); freed++; @@ -4278,7 +4272,6 @@ static int pmap_remove_l3(pmap_t pmap, pt_entry_t *l3, vm_offset_t va, pd_entry_t l2e, struct spglist *free, struct rwlock **lockp) { - struct md_page *pvh; pt_entry_t old_l3; vm_page_t m; @@ -4299,12 +4292,8 @@ pmap_remove_l3(pmap_t pmap, pt_entry_t *l3, vm_offset_t va, vm_page_aflag_set(m, PGA_REFERENCED); CHANGE_PV_LIST_LOCK_TO_VM_PAGE(lockp, m); pmap_pvh_free(&m->md, pmap, va); - if (TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = page_to_pvh(m); - if (TAILQ_EMPTY(&pvh->pv_list)) - vm_page_aflag_clear(m, PGA_WRITEABLE); - } + if (!pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); } return (pmap_unuse_pt(pmap, va, l2e, free)); } @@ -4406,7 +4395,6 @@ static void pmap_remove_l3_range(pmap_t pmap, pd_entry_t l2e, vm_offset_t sva, vm_offset_t eva, struct spglist *free, struct rwlock **lockp) { - struct md_page *pvh; struct rwlock *new_lock; pt_entry_t *l3, old_l3; vm_offset_t va; @@ -4487,12 +4475,8 @@ pmap_remove_l3_range(pmap_t pmap, pd_entry_t l2e, vm_offset_t sva, rw_wlock(*lockp); } pmap_pvh_free(&m->md, pmap, sva); - if (TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = page_to_pvh(m); - if (TAILQ_EMPTY(&pvh->pv_list)) - vm_page_aflag_clear(m, PGA_WRITEABLE); - } + if (!pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, PGA_WRITEABLE); } if (l3pg != NULL && pmap_unwire_l3(pmap, sva, l3pg, free)) { /* @@ -5880,10 +5864,13 @@ havel3: pv = pmap_pvh_remove(&om->md, pmap, va); if ((m->oflags & VPO_UNMANAGED) != 0) free_pv_entry(pmap, pv); + + /* + * The old page is likely COW, so check "writeable" + * first. + */ if ((om->a.flags & PGA_WRITEABLE) != 0 && - TAILQ_EMPTY(&om->md.pv_list) && - ((om->flags & PG_FICTITIOUS) != 0 || - TAILQ_EMPTY(&page_to_pvh(om)->pv_list))) + !pmap_page_is_mapped_locked(om)) vm_page_aflag_clear(om, PGA_WRITEABLE); } else { KASSERT((orig_l3 & ATTR_AF) != 0, @@ -7369,13 +7356,22 @@ pmap_page_is_mapped(vm_page_t m) return (false); lock = VM_PAGE_TO_PV_LIST_LOCK(m); rw_rlock(lock); - rv = !TAILQ_EMPTY(&m->md.pv_list) || - ((m->flags & PG_FICTITIOUS) == 0 && - !TAILQ_EMPTY(&page_to_pvh(m)->pv_list)); + rv = pmap_page_is_mapped_locked(m); rw_runlock(lock); return (rv); } +/* + * The page's PV list lock must be held. + */ +static __always_inline bool +pmap_page_is_mapped_locked(vm_page_t m) +{ + return (!TAILQ_EMPTY(&m->md.pv_list) || + ((m->flags & PG_FICTITIOUS) == 0 && + !TAILQ_EMPTY(&page_to_pvh(m)->pv_list))); +} + /* * Destroy all managed, non-wired mappings in the given user-space * pmap. This pmap cannot be active on any processor besides the @@ -7544,13 +7540,9 @@ pmap_remove_pages(pmap_t pmap) pv_next); m->md.pv_gen++; if ((m->a.flags & PGA_WRITEABLE) != 0 && - TAILQ_EMPTY(&m->md.pv_list) && - (m->flags & PG_FICTITIOUS) == 0) { - pvh = page_to_pvh(m); - if (TAILQ_EMPTY(&pvh->pv_list)) - vm_page_aflag_clear(m, - PGA_WRITEABLE); - } + !pmap_page_is_mapped_locked(m)) + vm_page_aflag_clear(m, + PGA_WRITEABLE); break; } pmap_unuse_pt(pmap, pv->pv_va, pmap_load(pde),
