Re: [PATCH 14/16] debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64

["Aneesh Kumar K.V" <aneesh.kumar@xxxxxxxxxxxxx>]

On 8/12/20 6:33 PM, Anshuman Khandual wrote:
> On 08/12/2020 12:03 PM, Aneesh Kumar K.V wrote:
> > The seems to be missing quite a lot of details w.r.t allocating
> > the correct pgtable_t page (huge_pte_alloc()), holding the right
> > lock (huge_pte_lock()) etc. The vma used is also not a hugetlb VMA.
> >
> > ppc64 do have runtime checks within CONFIG_DEBUG_VM for most of these.
> > Hence disable the test on ppc64.
>
> This test is free from any platform specific #ifdefs which should
> never be broken. If hugetlb_advanced_tests() does not work or is
> not detailed enough for ppc64, then it would be great if you could
> suggest some improvements so that it works for all enabled platforms.

As mentioned the test is broken. For hugetlb, the pgtable_t pages should 
be allocated by huge_pte_alloc(). We need to hold huget_pte_lock() 
before  updating huge tlb pte. That takes hugepage size, which is mostly 
derived out of vma. Hence vma need to be a hugetlb vma. Some of the 
functions also depend on hstate. Also we should use set_huge_pte_at() 
when setting up hugetlb pte entries. I was tempted to remove that test 
completely marking it broken. But avoided that by marking it broken on 
only PPC64.



> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxx>
> > ---
> >   mm/debug_vm_pgtable.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> > index 529892b9be2f..3e112d0ba1b2 100644
> > --- a/mm/debug_vm_pgtable.c
> > +++ b/mm/debug_vm_pgtable.c
> > @@ -800,6 +800,7 @@ static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
> >   #endif /* CONFIG_ARCH_WANT_GENERAL_HUGETLB */
> >   }
> >   
> > +#ifndef CONFIG_PPC_BOOK3S_64
> >   static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> >   					  struct vm_area_struct *vma,
> >   					  pte_t *ptep, unsigned long pfn,
> > @@ -842,6 +843,7 @@ static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> >   	pte = huge_ptep_get(ptep);
> >   	WARN_ON(!(huge_pte_write(pte) && huge_pte_dirty(pte)));
> >   }
> > +#endif
> >   #else  /* !CONFIG_HUGETLB_PAGE */
> >   static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
> >   static void __init hugetlb_advanced_tests(struct mm_struct *mm,
> > @@ -1053,7 +1055,9 @@ static int __init debug_vm_pgtable(void)
> >   	pud_populate_tests(mm, pudp, saved_pmdp);
> >   	spin_unlock(ptl);
> >   
> > -	//hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> > +#ifndef CONFIG_PPC_BOOK3S_64
> > +	hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot);
> > +#endif
> >   
> >   	spin_lock(&mm->page_table_lock);
> >   	p4d_clear_tests(mm, p4dp);