Fix two problems with pmap_clear_modify().

First, pmap_clear_modify() is write protecting all mappings to the specified
page, not just clearing the modified bit.  Specifically, it sets PTE_RO on
the PTE, which is wrong.  Moreover, it is calling vm_page_dirty(), which is
not the expected behavior for pmap_clear_modify().  Generally speaking, the
machine-independent VM layer masks these mistakes.  For example, setting
PTE_RO will result in additional soft faults, but not a catastrophe.

Second, pmap_clear_modify() may not clear the modified bits because it only
iterates over the PV list when the page has the PV_TABLE_MOD flag set and
elsewhere the pmap clears the PV_TABLE_MOD flag anytime a modified mapping
is write protected or destroyed.  However, the page may still have other
mappings with the modified bit set.

Eliminate a stale comment.

1 files changed, 13 insertions, 51 deletions

diff --git a/sys/mips/mips/pmap.c b/sys/mips/mips/pmap.c
index 80be862..3ecc9e8 100644
--- a/sys/mips/mips/pmap.c
+++ b/sys/mips/mips/pmap.c
@@ -179,7 +179,6 @@ static vm_page_t pmap_pv_reclaim(pmap_t locked_pmap);
 static void pmap_pvh_free(struct md_page *pvh, pmap_t pmap, vm_offset_t va);
 static pv_entry_t pmap_pvh_remove(struct md_page *pvh, pmap_t pmap,
     vm_offset_t va);
-static __inline void pmap_changebit(vm_page_t m, int bit, boolean_t setem);
 static vm_page_t pmap_enter_quick_locked(pmap_t pmap, vm_offset_t va,
     vm_page_t m, vm_prot_t prot, vm_page_t mpte);
 static int pmap_remove_pte(struct pmap *pmap, pt_entry_t *ptq, vm_offset_t va,
@@ -2664,8 +2663,6 @@ pmap_remove_pages(pmap_t pmap)
 
 /*
  * pmap_testbit tests bits in pte's
- * note that the testbit/changebit routines are inline,
- * and a lot of things compile-time evaluate.
  */
 static boolean_t
 pmap_testbit(vm_page_t m, int bit)
@@ -2692,51 +2689,6 @@ pmap_testbit(vm_page_t m, int bit)
 }
 
 /*
- * this routine is used to clear dirty bits in ptes
- */
-static __inline void
-pmap_changebit(vm_page_t m, int bit, boolean_t setem)
-{
-	pv_entry_t pv;
-	pmap_t pmap;
-	pt_entry_t *pte;
-
-	if (m->oflags & VPO_UNMANAGED)
-		return;
-
-	rw_assert(&pvh_global_lock, RA_WLOCKED);
-	/*
-	 * Loop over all current mappings setting/clearing as appropos If
-	 * setting RO do we need to clear the VAC?
-	 */
-	TAILQ_FOREACH(pv, &m->md.pv_list, pv_list) {
-		pmap = PV_PMAP(pv);
-		PMAP_LOCK(pmap);
-		pte = pmap_pte(pmap, pv->pv_va);
-		if (setem) {
-			*pte |= bit;
-			pmap_update_page(pmap, pv->pv_va, *pte);
-		} else {
-			pt_entry_t pbits = *pte;
-
-			if (pbits & bit) {
-				if (bit == PTE_D) {
-					if (pbits & PTE_D)
-						vm_page_dirty(m);
-					*pte = (pbits & ~PTE_D) | PTE_RO;
-				} else {
-					*pte = pbits & ~bit;
-				}
-				pmap_update_page(pmap, pv->pv_va, *pte);
-			}
-		}
-		PMAP_UNLOCK(pmap);
-	}
-	if (!setem && bit == PTE_D)
-		vm_page_aflag_clear(m, PGA_WRITEABLE);
-}
-
-/*
  *	pmap_page_wired_mappings:
  *
  *	Return the number of managed mappings to the given physical page
@@ -2896,6 +2848,9 @@ pmap_is_prefaultable(pmap_t pmap, vm_offset_t addr)
 void
 pmap_clear_modify(vm_page_t m)
 {
+	pmap_t pmap;
+	pt_entry_t *pte;
+	pv_entry_t pv;
 
 	KASSERT((m->oflags & VPO_UNMANAGED) == 0,
 	    ("pmap_clear_modify: page %p is not managed", m));
@@ -2911,10 +2866,17 @@ pmap_clear_modify(vm_page_t m)
 	if ((m->aflags & PGA_WRITEABLE) == 0)
 		return;
 	rw_wlock(&pvh_global_lock);
-	if (m->md.pv_flags & PV_TABLE_MOD) {
-		pmap_changebit(m, PTE_D, FALSE);
-		m->md.pv_flags &= ~PV_TABLE_MOD;
+	TAILQ_FOREACH(pv, &m->md.pv_list, pv_list) {
+		pmap = PV_PMAP(pv);
+		PMAP_LOCK(pmap);
+		pte = pmap_pte(pmap, pv->pv_va);
+		if (pte_test(pte, PTE_D)) {
+			pte_clear(pte, PTE_D);
+			pmap_update_page(pmap, pv->pv_va, *pte);
+		}
+		PMAP_UNLOCK(pmap);
 	}
+	m->md.pv_flags &= ~PV_TABLE_MOD;
 	rw_wunlock(&pvh_global_lock);
 }