From owner-svn-src-all@FreeBSD.ORG Fri Aug 17 05:02:29 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A84DC1065672; Fri, 17 Aug 2012 05:02:29 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 933C38FC08; Fri, 17 Aug 2012 05:02:29 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q7H52Tfu020264; Fri, 17 Aug 2012 05:02:29 GMT (envelope-from alc@svn.freebsd.org) Received: (from alc@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q7H52TBC020262; Fri, 17 Aug 2012 05:02:29 GMT (envelope-from alc@svn.freebsd.org) Message-Id: <201208170502.q7H52TBC020262@svn.freebsd.org> From: Alan Cox Date: Fri, 17 Aug 2012 05:02:29 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r239352 - head/sys/mips/mips X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Aug 2012 05:02:29 -0000 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); }