On Mon, 2018-11-19 at 15:43 -0800, Dave Hansen wrote:
On 11/19/18 3:19 PM, Dan Williams wrote:
Andy wondered why a path that can sleep was using __flush_tlb_all() [1] and Dave confirmed the expectation for TLB flush is for modifying / invalidating existing pte entries, but not initial population [2].
I _think_ this is OK.
But, could we sprinkle a few WARN_ON_ONCE(p*_present()) calls in there to help us sleep at night?
Well, I'm having nightmares now because my naive patch to sprinkle some WARN_ON_ONCE() calls is leading to my VM live locking at boot... no backtrace. If I revert the patch below and just go with the __flush_tlb_all() removal it seems fine.
I'm going to set this aside for a bit, but if anyone has any thoughts in the meantime I'd appreciate it.
---
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index de95db8ac52f..ecdf917def4c 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -432,6 +432,7 @@ phys_pte_init(pte_t *pte_page, unsigned long paddr, unsigned long paddr_end, E820_TYPE_RAM) && !e820__mapped_any(paddr & PAGE_MASK, paddr_next, E820_TYPE_RESERVED_KERN)) + WARN_ON_ONCE(pte_present(*pte)); set_pte(pte, __pte(0)); continue; } @@ -452,6 +453,7 @@ phys_pte_init(pte_t *pte_page, unsigned long paddr, unsigned long paddr_end, pr_info(" pte=%p addr=%lx pte=%016lx\n", pte, paddr, pfn_pte(paddr >> PAGE_SHIFT, PAGE_KERNEL).pte); pages++; + WARN_ON_ONCE(pte_present(*pte)); set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot)); paddr_last = (paddr & PAGE_MASK) + PAGE_SIZE; } @@ -487,6 +489,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long paddr, unsigned long paddr_end, E820_TYPE_RAM) && !e820__mapped_any(paddr & PMD_MASK, paddr_next, E820_TYPE_RESERVED_KERN)) + WARN_ON_ONCE(pmd_present(*pmd)); set_pmd(pmd, __pmd(0)); continue; } @@ -524,6 +527,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long paddr, unsigned long paddr_end, if (page_size_mask & (1<<PG_LEVEL_2M)) { pages++; spin_lock(&init_mm.page_table_lock); + WARN_ON_ONCE(pmd_present(*pmd)); set_pte((pte_t *)pmd, pfn_pte((paddr & PMD_MASK) >> PAGE_SHIFT, __pgprot(pgprot_val(prot) | _PAGE_PSE))); @@ -536,6 +540,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long paddr, unsigned long paddr_end, paddr_last = phys_pte_init(pte, paddr, paddr_end, new_prot);
spin_lock(&init_mm.page_table_lock); + WARN_ON_ONCE(pmd_present(*pmd)); pmd_populate_kernel(&init_mm, pmd, pte); spin_unlock(&init_mm.page_table_lock); } @@ -573,6 +578,7 @@ phys_pud_init(pud_t *pud_page, unsigned long paddr, unsigned long paddr_end, E820_TYPE_RAM) && !e820__mapped_any(paddr & PUD_MASK, paddr_next, E820_TYPE_RESERVED_KERN)) + WARN_ON_ONCE(pud_present(*pud)); set_pud(pud, __pud(0)); continue; } @@ -610,6 +616,7 @@ phys_pud_init(pud_t *pud_page, unsigned long paddr, unsigned long paddr_end, if (page_size_mask & (1<<PG_LEVEL_1G)) { pages++; spin_lock(&init_mm.page_table_lock); + WARN_ON_ONCE(pud_present(*pud)); set_pte((pte_t *)pud, pfn_pte((paddr & PUD_MASK) >> PAGE_SHIFT, PAGE_KERNEL_LARGE)); @@ -623,6 +630,7 @@ phys_pud_init(pud_t *pud_page, unsigned long paddr, unsigned long paddr_end, page_size_mask, prot);
spin_lock(&init_mm.page_table_lock); + WARN_ON_ONCE(pud_present(*pud)); pud_populate(&init_mm, pud, pmd); spin_unlock(&init_mm.page_table_lock); } @@ -657,6 +665,7 @@ phys_p4d_init(p4d_t *p4d_page, unsigned long paddr, unsigned long paddr_end, E820_TYPE_RAM) && !e820__mapped_any(paddr & P4D_MASK, paddr_next, E820_TYPE_RESERVED_KERN)) + WARN_ON_ONCE(p4d_present(*p4d)); set_p4d(p4d, __p4d(0)); continue; } @@ -674,6 +683,7 @@ phys_p4d_init(p4d_t *p4d_page, unsigned long paddr, unsigned long paddr_end, page_size_mask);
spin_lock(&init_mm.page_table_lock); + WARN_ON_ONCE(p4d_present(*p4d)); p4d_populate(&init_mm, p4d, pud); spin_unlock(&init_mm.page_table_lock); }