Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 17 Aug 2012 05:02:29 +0000 (UTC)
From:      Alan Cox <alc@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r239352 - head/sys/mips/mips
Message-ID:  <201208170502.q7H52TBC020262@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: alc
Date: Fri Aug 17 05:02:29 2012
New Revision: 239352
URL: http://svn.freebsd.org/changeset/base/239352

Log:
  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.

Modified:
  head/sys/mips/mips/pmap.c

Modified: head/sys/mips/mips/pmap.c
==============================================================================
--- head/sys/mips/mips/pmap.c	Fri Aug 17 04:44:57 2012	(r239351)
+++ head/sys/mips/mips/pmap.c	Fri Aug 17 05:02:29 2012	(r239352)
@@ -179,7 +179,6 @@ static vm_page_t pmap_pv_reclaim(pmap_t 
 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_off
 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);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208170502.q7H52TBC020262>