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

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 14/16] debug_vm_pgtable/hugetlb: Disable hugetlb test on ppc64
Date: Wed, 12 Aug 2020 18:46:35 +0530	[thread overview]
Message-ID: <efc8f8c8-c16d-0826-9401-a45005ecaf62@linux.ibm.com> (raw)
In-Reply-To: <d067c278-6391-72f1-75c2-ccbd52909233@arm.com>

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@linux.ibm.com>
>> ---
>>   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);
>>