On Thu, 2018-04-26 at 16:19 +0200, Joerg Roedel wrote:
Hi Toshi, Andrew,
this patch(-set) is broken in several ways, please see below.
On Wed, Mar 14, 2018 at 12:01:55PM -0600, Toshi Kani wrote:
Implement pud_free_pmd_page() and pmd_free_pte_page() on x86, which clear a given pud/pmd entry and free up lower level page table(s). Address range associated with the pud/pmd entry must have been purged by INVLPG.
An INVLPG before actually unmapping the page is useless, as other cores or even speculative instruction execution can bring the TLB entry back before the code actually unmaps the page.
Hi Joerg,
All pages under the pmd had been unmapped and then lazy TLB purged with INVLPG before coming to this code path. Speculation is not allowed to pages without mapping.
int pud_free_pmd_page(pud_t *pud) {
- return pud_none(*pud);
- pmd_t *pmd;
- int i;
- if (pud_none(*pud))
return 1;- pmd = (pmd_t *)pud_page_vaddr(*pud);
- for (i = 0; i < PTRS_PER_PMD; i++)
if (!pmd_free_pte_page(&pmd[i]))return 0;- pud_clear(pud);
TLB flush needed here, before the page is freed.
- free_page((unsigned long)pmd);
- return 1;
} /** @@ -724,6 +739,15 @@ int pud_free_pmd_page(pud_t *pud) */ int pmd_free_pte_page(pmd_t *pmd) {
- return pmd_none(*pmd);
- pte_t *pte;
- if (pmd_none(*pmd))
return 1;- pte = (pte_t *)pmd_page_vaddr(*pmd);
- pmd_clear(pmd);
Same here, TLB flush needed.
Further this needs synchronization with other page-tables in the system when the kernel PMDs are not shared between processes. In x86-32 with PAE this causes a BUG_ON() being triggered at arch/x86/mm/fault.c:268 because the page-tables are not correctly synchronized.
I think this is an issue with pmd mapping support on x86-32-PAE, not with this patch. I think the code needed to be updated to sync at the pud level.
Thanks, -Toshi