git: 37321270b630 - main - amd64/arm64 pmap: consistently clear PGA_WRITEABLE
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 07 Sep 2026 17:49:37 UTC
The branch main has been updated by alc:
URL: https://cgit.FreeBSD.org/src/commit/?id=37321270b630d891e48851f81dd6d077ebd6b635
commit 37321270b630d891e48851f81dd6d077ebd6b635
Author: Alan Cox <alc@FreeBSD.org>
AuthorDate: 2026-09-07 07:31:45 +0000
Commit: Alan Cox <alc@FreeBSD.org>
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),