From owner-svn-src-all@freebsd.org Sun Jul 8 16:51:55 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 46346103BCDC; Sun, 8 Jul 2018 16:51:55 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E94907E66D; Sun, 8 Jul 2018 16:51:54 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id CB3CAA34; Sun, 8 Jul 2018 16:51:54 +0000 (UTC) (envelope-from alc@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w68GpsfQ074429; Sun, 8 Jul 2018 16:51:54 GMT (envelope-from alc@FreeBSD.org) Received: (from alc@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w68Gpsde074428; Sun, 8 Jul 2018 16:51:54 GMT (envelope-from alc@FreeBSD.org) Message-Id: <201807081651.w68Gpsde074428@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: alc set sender to alc@FreeBSD.org using -f From: Alan Cox Date: Sun, 8 Jul 2018 16:51:54 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r336092 - head/sys/i386/i386 X-SVN-Group: head X-SVN-Commit-Author: alc X-SVN-Commit-Paths: head/sys/i386/i386 X-SVN-Commit-Revision: 336092 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.27 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: Sun, 08 Jul 2018 16:51:55 -0000 Author: alc Date: Sun Jul 8 16:51:54 2018 New Revision: 336092 URL: https://svnweb.freebsd.org/changeset/base/336092 Log: Invalidate the mapping before updating its physical address. Doing so ensures that all threads sharing the pmap have a consistent view of the mapping. This fixes the problem described in the commit log messages for r329254 without the overhead of an extra fault in the common case. Once other pmap_enter() implementations are similarly modified, the workaround added in r329254 can be removed, reducing the overhead of CoW faults. See also r335784 for amd64. The i386 implementation of pmap_enter() already reused the PV entry from the old mapping. Reviewed by: kib, markj Tested by: pho MFC after: 3 weeks Differential Revision: https://reviews.freebsd.org/D16133 Modified: head/sys/i386/i386/pmap.c Modified: head/sys/i386/i386/pmap.c ============================================================================== --- head/sys/i386/i386/pmap.c Sun Jul 8 16:37:50 2018 (r336091) +++ head/sys/i386/i386/pmap.c Sun Jul 8 16:51:54 2018 (r336092) @@ -3636,7 +3636,6 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, v } pa = VM_PAGE_TO_PHYS(m); - om = NULL; origpte = *pte; opa = origpte & PG_FRAME; @@ -3661,10 +3660,8 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, v if (mpte) mpte->wire_count--; - if (origpte & PG_MANAGED) { - om = m; + if (origpte & PG_MANAGED) pa |= PG_MANAGED; - } goto validate; } @@ -3672,15 +3669,42 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, v /* * Mapping has changed, invalidate old range and fall through to - * handle validating new mapping. + * handle validating new mapping. This ensures that all threads + * sharing the pmap keep a consistent view of the mapping, which is + * necessary for the correct handling of COW faults. It + * also permits reuse of the old mapping's PV entry, + * avoiding an allocation. + * + * For consistency, handle unmanaged mappings the same way. */ if (opa) { + origpte = pte_load_clear(pte); + KASSERT((origpte & PG_FRAME) == opa, + ("pmap_enter: unexpected pa update for %#x", va)); if (origpte & PG_W) pmap->pm_stats.wired_count--; if (origpte & PG_MANAGED) { om = PHYS_TO_VM_PAGE(opa); + + /* + * The pmap lock is sufficient to synchronize with + * concurrent calls to pmap_page_test_mappings() and + * pmap_ts_referenced(). + */ + if ((origpte & (PG_M | PG_RW)) == (PG_M | PG_RW)) + vm_page_dirty(om); + if ((origpte & PG_A) != 0) + vm_page_aflag_set(om, PGA_REFERENCED); pv = pmap_pvh_remove(&om->md, pmap, va); + if ((om->aflags & PGA_WRITEABLE) != 0 && + TAILQ_EMPTY(&om->md.pv_list) && + ((om->flags & PG_FICTITIOUS) != 0 || + TAILQ_EMPTY(&pa_to_pvh(opa)->pv_list))) + vm_page_aflag_clear(om, PGA_WRITEABLE); } + if ((origpte & PG_A) != 0) + pmap_invalidate_page(pmap, va); + origpte = 0; if (mpte != NULL) { mpte->wire_count--; KASSERT(mpte->wire_count > 0, @@ -3697,9 +3721,10 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, v KASSERT(pmap != kernel_pmap || va < kmi.clean_sva || va >= kmi.clean_eva, ("pmap_enter: managed mapping within the clean submap")); - if (pv == NULL) + if (pv == NULL) { pv = get_pv_entry(pmap, FALSE); - pv->pv_va = va; + pv->pv_va = va; + } TAILQ_INSERT_TAIL(&m->md.pv_list, pv, pv_next); pa |= PG_MANAGED; } else if (pv != NULL) @@ -3741,28 +3766,19 @@ validate: if (origpte & PG_V) { invlva = FALSE; origpte = pte_load_store(pte, newpte); - if (origpte & PG_A) { - if (origpte & PG_MANAGED) - vm_page_aflag_set(om, PGA_REFERENCED); - if (opa != VM_PAGE_TO_PHYS(m)) - invlva = TRUE; -#if defined(PAE) || defined(PAE_TABLES) - if ((origpte & PG_NX) == 0 && - (newpte & PG_NX) != 0) - invlva = TRUE; -#endif - } - if ((origpte & (PG_M | PG_RW)) == (PG_M | PG_RW)) { + KASSERT((origpte & PG_FRAME) == VM_PAGE_TO_PHYS(m), + ("pmap_enter: unexpected pa update for %#x", va)); + if ((origpte & (PG_M | PG_RW)) == (PG_M | PG_RW) && + (newpte & PG_M) == 0) { if ((origpte & PG_MANAGED) != 0) - vm_page_dirty(om); - if ((prot & VM_PROT_WRITE) == 0) - invlva = TRUE; + vm_page_dirty(m); + invlva = TRUE; } - if ((origpte & PG_MANAGED) != 0 && - TAILQ_EMPTY(&om->md.pv_list) && - ((om->flags & PG_FICTITIOUS) != 0 || - TAILQ_EMPTY(&pa_to_pvh(opa)->pv_list))) - vm_page_aflag_clear(om, PGA_WRITEABLE); +#if defined(PAE) || defined(PAE_TABLES) + else if ((origpte & (PG_A | PG_NX)) == PG_A && + (newpte & PG_NX) != 0) + invlva = TRUE; +#endif if (invlva) pmap_invalidate_page(pmap, va); } else