From 554cf9ac8ff4af8eedd0f5896dfa5d2e5f61038d Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Thu, 23 Aug 2012 20:44:53 -0700 Subject: [PATCH] kernel - Fix pmap_remove() issue. * When the inner loop of pmap_remove() broke out due to pmap_remove_pte() blocking it 'jumped' the sindex to pdnxt, thus any remaining pages in that page table page would get skipped. * pmap_remove_pte() and friends used to return non-zero if they 'blocked'. Unfortunately they could actually block whether they returned 0 or non-zero, causing the problem. Remove the check entirely. * Adjust misc functions which we no longer need the return value for. * I don't know if this had anything to do with the wire_count panic. The failure mode should have been caught by numerous other assertions in the code but wasn't. get_ptbase() relies on a test of pmap->pm_cached to determine if a page directory page mapping changes so it should not have been possible for it to continue the loop and access a stale ptep. If it were possible it might account for a wire_count panic later on but it doesn't seem like it should be possible. --- sys/platform/pc32/i386/pmap.c | 94 +++++++++++++++++------------------ 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/sys/platform/pc32/i386/pmap.c b/sys/platform/pc32/i386/pmap.c index 4e56652d4e..3d362ea55f 100644 --- a/sys/platform/pc32/i386/pmap.c +++ b/sys/platform/pc32/i386/pmap.c @@ -195,12 +195,12 @@ static pv_entry_t get_pv_entry (void); static void i386_protection_init (void); static __inline void pmap_clearbit (vm_page_t m, int bit); -static void pmap_remove_all (vm_page_t m); -static int pmap_remove_pte (struct pmap *pmap, unsigned *ptq, +static void pmap_remove_all (vm_page_t m); +static void pmap_remove_pte (struct pmap *pmap, unsigned *ptq, vm_offset_t sva, pmap_inval_info_t info); static void pmap_remove_page (struct pmap *pmap, vm_offset_t va, pmap_inval_info_t info); -static int pmap_remove_entry (struct pmap *pmap, vm_page_t m, +static void pmap_remove_entry (struct pmap *pmap, vm_page_t m, vm_offset_t va, pmap_inval_info_t info); static boolean_t pmap_testbit (vm_page_t m, int bit); static void pmap_insert_entry (pmap_t pmap, pv_entry_t pv, @@ -212,7 +212,7 @@ static int pmap_release_free_page (pmap_t pmap, vm_page_t p); static vm_page_t _pmap_allocpte (pmap_t pmap, unsigned ptepindex); static unsigned * pmap_pte_quick (pmap_t pmap, vm_offset_t va); static vm_page_t pmap_page_lookup (vm_object_t object, vm_pindex_t pindex); -static int pmap_unuse_pt (pmap_t, vm_offset_t, vm_page_t, pmap_inval_info_t); +static void pmap_unuse_pt (pmap_t, vm_offset_t, vm_page_t, pmap_inval_info_t); static vm_offset_t pmap_kmem_choose(vm_offset_t addr); static unsigned pdir4mb; @@ -1105,7 +1105,7 @@ pmap_unwire_pte(pmap_t pmap, vm_page_t m, pmap_inval_info_t info) * The caller must hold vm_token. * This function can block regardless. */ -static int +static void pmap_unuse_pt(pmap_t pmap, vm_offset_t va, vm_page_t mpte, pmap_inval_info_t info) { @@ -1114,7 +1114,7 @@ pmap_unuse_pt(pmap_t pmap, vm_offset_t va, vm_page_t mpte, ASSERT_LWKT_TOKEN_HELD(vm_object_token(pmap->pm_pteobj)); if (va >= UPT_MIN_ADDRESS) - return 0; + return; if (mpte == NULL) { ptepindex = (va >> PDRSHIFT); @@ -1128,8 +1128,7 @@ pmap_unuse_pt(pmap_t pmap, vm_offset_t va, vm_page_t mpte, vm_page_wakeup(mpte); } } - - return pmap_unwire_pte(pmap, mpte, info); + pmap_unwire_pte(pmap, mpte, info); } /* @@ -1750,12 +1749,11 @@ pmap_collect(void) * * The caller must hold vm_token. */ -static int +static void pmap_remove_entry(struct pmap *pmap, vm_page_t m, vm_offset_t va, pmap_inval_info_t info) { pv_entry_t pv; - int rtval; /* * Cannot block @@ -1780,7 +1778,6 @@ pmap_remove_entry(struct pmap *pmap, vm_page_t m, /* * Cannot block */ - rtval = 0; test_m_maps_pv(m, pv); TAILQ_REMOVE(&m->md.pv_list, pv, pv_list); m->md.pv_list_count--; @@ -1795,11 +1792,9 @@ pmap_remove_entry(struct pmap *pmap, vm_page_t m, * This can block. */ vm_object_hold(pmap->pm_pteobj); - rtval = pmap_unuse_pt(pmap, va, pv->pv_ptem, info); + pmap_unuse_pt(pmap, va, pv->pv_ptem, info); vm_object_drop(pmap->pm_pteobj); free_pv_entry(pv); - - return rtval; } /* @@ -1836,7 +1831,7 @@ pmap_insert_entry(pmap_t pmap, pv_entry_t pv, vm_offset_t va, * callers using temporary page table mappings must reload * them. */ -static int +static void pmap_remove_pte(struct pmap *pmap, unsigned *ptq, vm_offset_t va, pmap_inval_info_t info) { @@ -1875,12 +1870,10 @@ pmap_remove_pte(struct pmap *pmap, unsigned *ptq, vm_offset_t va, } if (oldpte & PG_A) vm_page_flag_set(m, PG_REFERENCED); - return pmap_remove_entry(pmap, m, va, info); + pmap_remove_entry(pmap, m, va, info); } else { - return pmap_unuse_pt(pmap, va, NULL, info); + pmap_unuse_pt(pmap, va, NULL, info); } - - return 0; } /* @@ -1957,23 +1950,31 @@ pmap_remove(struct pmap *pmap, vm_offset_t sva, vm_offset_t eva) sindex = i386_btop(sva); eindex = i386_btop(eva); - for (; sindex < eindex; sindex = pdnxt) { + while (sindex < eindex) { unsigned pdirindex; /* - * Calculate index for next page table. + * Stop scanning if no pages are left */ - pdnxt = ((sindex + NPTEPG) & ~(NPTEPG - 1)); if (pmap->pm_stats.resident_count == 0) break; + /* + * Calculate index for next page table, limited by eindex. + */ + pdnxt = ((sindex + NPTEPG) & ~(NPTEPG - 1)); + if (pdnxt > eindex) + pdnxt = eindex; + pdirindex = sindex / NPDEPG; - if (((ptpaddr = (unsigned) pmap->pm_pdir[pdirindex]) & PG_PS) != 0) { + ptpaddr = (unsigned)pmap->pm_pdir[pdirindex]; + if (ptpaddr & PG_PS) { pmap_inval_interlock(&info, pmap, -1); pmap->pm_pdir[pdirindex] = 0; pmap->pm_stats.resident_count -= NBPDR / PAGE_SIZE; pmap->pm_cached = 0; pmap_inval_deinterlock(&info, pmap); + sindex = pdnxt; continue; } @@ -1981,31 +1982,31 @@ pmap_remove(struct pmap *pmap, vm_offset_t sva, vm_offset_t eva) * Weed out invalid mappings. Note: we assume that the page * directory table is always allocated, and in kernel virtual. */ - if (ptpaddr == 0) + if (ptpaddr == 0) { + sindex = pdnxt; continue; - - /* - * Limit our scan to either the end of the va represented - * by the current page table page, or to the end of the - * range being removed. - */ - if (pdnxt > eindex) { - pdnxt = eindex; } /* - * NOTE: pmap_remove_pte() can block and wipe the temporary - * ptbase. + * Sub-scan the page table page. pmap_remove_pte() can + * block on us, invalidating ptbase, so we must reload + * ptbase and we must also check whether the page directory + * page is still present. */ - for (; sindex != pdnxt; sindex++) { + while (sindex < pdnxt) { vm_offset_t va; ptbase = get_ptbase(pmap); - if (ptbase[sindex] == 0) - continue; - va = i386_ptob(sindex); - if (pmap_remove_pte(pmap, ptbase + sindex, va, &info)) + if (ptbase[sindex]) { + va = i386_ptob(sindex); + pmap_remove_pte(pmap, ptbase + sindex, + va, &info); + } + if (pmap->pm_pdir[pdirindex] == 0 || + (pmap->pm_pdir[pdirindex] & PG_PS)) { break; + } + ++sindex; } } pmap_inval_done(&info); @@ -2042,9 +2043,7 @@ pmap_remove_all(vm_page_t m) pmap_inval_deinterlock(&info, pv->pv_pmap); if (tpte & PG_A) vm_page_flag_set(m, PG_REFERENCED); -#ifdef PMAP_DEBUG KKASSERT(PHYS_TO_VM_PAGE(tpte) == m); -#endif /* * Update the vm_page_t clean and reference bits. @@ -2314,13 +2313,9 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, vm_prot_t prot, * that case too. */ while (opa) { - int err; - KKASSERT((origpte & PG_FRAME) == (*(vm_offset_t *)pte & PG_FRAME)); - err = pmap_remove_pte(pmap, pte, va, &info); - if (err) - panic("pmap_enter: pte vanished, va: %p", (void *)va); + pmap_remove_pte(pmap, pte, va, &info); pte = pmap_pte(pmap, va); origpte = *(vm_offset_t *)pte; opa = origpte & PG_FRAME; @@ -2512,18 +2507,19 @@ pmap_enter_quick(pmap_t pmap, vm_offset_t va, vm_page_t m) * we do not disturb it. */ pte = (unsigned *)vtopte(va); - if (*pte & PG_V) { + if (*pte) { + KKASSERT(*pte & PG_V); pa = VM_PAGE_TO_PHYS(m); KKASSERT(((*pte ^ pa) & PG_FRAME) == 0); pmap_inval_done(&info); if (mpte) pmap_unwire_pte(pmap, mpte, &info); - lwkt_reltoken(&vm_token); - vm_object_drop(pmap->pm_pteobj); if (pv) { free_pv_entry(pv); /* pv = NULL; */ } + lwkt_reltoken(&vm_token); + vm_object_drop(pmap->pm_pteobj); return; } -- 2.41.0