Re: [PATCH] mm/khugepaged: use [pmd|pte]_addr for better reading

[Wei Yang <richard.weiyang@gmail.com>]

On Sat, Sep 20, 2025 at 10:21:56AM +0530, Dev Jain wrote:
>
>On 20/09/25 6:24 am, Wei Yang wrote:
>> When collapse a pmd, there are two address in use:
>> 
>>    * address points to the start of pmd
>>    * address points to each individual page
>> 
>> Current naming is not easy to distinguish these two and error prone.
>> 
>> Name the first one to pmd_addr and second one to pte_addr.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> ---
>>   mm/khugepaged.c | 43 ++++++++++++++++++++++---------------------
>>   1 file changed, 22 insertions(+), 21 deletions(-)
>> 
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 4c957ce788d1..6d03072c1a92 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -537,18 +537,19 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
>>   }
>>   static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>> -					unsigned long address,
>> +					unsigned long pmd_addr,
>>   					pte_t *pte,
>>   					struct collapse_control *cc,
>>   					struct list_head *compound_pagelist)
>>   {
>>   	struct page *page = NULL;
>>   	struct folio *folio = NULL;
>> +	unsigned long pte_addr = pmd_addr;
>>   	pte_t *_pte;
>>   	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
>>   	for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
>> -	     _pte++, address += PAGE_SIZE) {
>> +	     _pte++, pte_addr += PAGE_SIZE) {
>>   		pte_t pteval = ptep_get(_pte);
>>   		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
>>   			++none_or_zero;
>> @@ -570,7 +571,7 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>>   			result = SCAN_PTE_UFFD_WP;
>>   			goto out;
>>   		}
>> -		page = vm_normal_page(vma, address, pteval);
>> +		page = vm_normal_page(vma, pte_addr, pteval);
>>   		if (unlikely(!page) || unlikely(is_zone_device_page(page))) {
>>   			result = SCAN_PAGE_NULL;
>>   			goto out;
>> @@ -655,8 +656,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>>   		 */
>>   		if (cc->is_khugepaged &&
>>   		    (pte_young(pteval) || folio_test_young(folio) ||
>> -		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
>> -								     address)))
>> +		     folio_test_referenced(folio) ||
>> +		     mmu_notifier_test_young(vma->vm_mm, pte_addr)))
>>   			referenced++;
>>   	}
>> @@ -985,21 +986,21 @@ static int check_pmd_still_valid(struct mm_struct *mm,
>>    */
>>   static int __collapse_huge_page_swapin(struct mm_struct *mm,
>>   				       struct vm_area_struct *vma,
>> -				       unsigned long haddr, pmd_t *pmd,
>> +				       unsigned long pmd_addr, pmd_t *pmd,
>>   				       int referenced)
>
>Will this be a problem when mTHP collapse is in? You may have the starting
>address lying in the PTE table.
>
>Personally "haddr" is pretty clear to me - I read it as "huge-aligned addr".
>I will vote for naming the starting addresses everywhere as "haddr", and use
>addr as the loop iterator. This is a short name and haddr will imply that it
>is aligned to the huge order we are collapsing for.
>

So your suggestion is 

    pmd_addr -> haddr
    pte_addr -> address

right?

-- 
Wei Yang
Help you, Help me