On Wed, 19 Dec 2018 00:13:13 -0500
George Koehler <[email protected]> wrote:
> By my guess, pmap_is_modified() is broken on powerpc. It seems to
> fail to return true, so uvm wouldn't know that the page is modified,
> and then uvm wouldn't write the modified page to swap.
Turns out that pmap_page_protect(pg, PROT_NONE) forgot to check if
the page was modified. The below diff seems to fix it.
pmap_is_modified(pg) calls pmap_test_attrs(pg, PG_PMAP_MOD), which
iterates pg->mdpage.pv_list to check if the hardware had set
PTE_CHG_32 on any mapping of this physical page. The list is empty
because uvm seems to remove all the mappings before it asks if the
page is modified, then swaps out the page.
A function that does LIST_REMOVE a mapping with PTE_CHG_32 must set
PG_PMAP_MOD in pg->pg_flags, but pmap_page_protect(pg, PROT_NONE)
fails to do so. This function does
pted->pted_va &= ~PTED_VA_MANAGED_M;
LIST_REMOVE(pted, pted_pv_list);
before
pmap_remove_pted(pm, pted);
Then pmap_remove_pted() calls pte_zap(), which would check PTE_CHG_32
if PTED_MANAGED(pted) is true. The check is skipped because we had
unset PTED_VA_MANAGED_M.
The following diff fixes my test program on my single G4 processor
(MPC7447A), where ppc_proc_is_64b is false. I will use this diff
until someone gives me a better diff.
Index: pmap.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/powerpc/pmap.c,v
retrieving revision 1.168
diff -u -p -r1.168 pmap.c
--- pmap.c 22 Oct 2018 17:31:25 -0000 1.168
+++ pmap.c 20 Dec 2018 03:16:13 -0000
@@ -540,6 +540,28 @@ pmap_attr_save(paddr_t pa, u_int32_t bit
atomic_setbits_int(&pg->pg_flags, pmap_pte2flags(bits));
}
+/* Fixes pg->pg_flags by checking REF and CHG. */
+void
+pmap_fix_page_attrs(struct vm_page *pg, struct pte_desc *pted)
+{
+ void *pte;
+ u_int bits;
+ int s;
+
+ PMAP_HASH_LOCK(s);
+ if ((pte = pmap_ptedinhash(pted)) != NULL) {
+ if (ppc_proc_is_64b) {
+ struct pte_64 *pte64 = pte;
+ bits = pte64->pte_lo & (PTE_REF_64|PTE_CHG_64);
+ } else {
+ struct pte_32 *pte32 = pte;
+ bits = pte32->pte_lo & (PTE_REF_32|PTE_CHG_32);
+ }
+ atomic_setbits_int(&pg->pg_flags, pmap_pte2flags(bits));
+ }
+ PMAP_HASH_UNLOCK(s);
+}
+
int
pmap_enter(pmap_t pm, vaddr_t va, paddr_t pa, vm_prot_t prot, int flags)
{
@@ -2101,6 +2123,13 @@ pmap_page_protect(struct vm_page *pg, vm
pted->pted_va &= ~PTED_VA_MANAGED_M;
LIST_REMOVE(pted, pted_pv_list);
mtx_leave(&pg->mdpage.pv_mtx);
+
+ /*
+ * Check REF and CHG now, because
+ * pmap_remove_pted() won't check them when
+ * not PTED_VA_MANAGED_M.
+ */
+ pmap_fix_page_attrs(pg, pted);
pmap_remove_pted(pm, pted);
--
George Koehler <[email protected]>
