Re: [PATCH] mm/khugepaged: fix the address passed to notifier on testing young

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

On Tue, Aug 26, 2025 at 10:53:45AM +0200, David Hildenbrand wrote:
>On 22.08.25 08:33, Wei Yang wrote:
>> Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
>> mmu_notifier_test_young(), but we should pass the address need to test.
>
>... "but we are passing the wrong address".
>
>> In xxx_scan_pmd(), the actual iteration address is "_address" not
>> "address". We seem to misuse the variable on the very beginning.
>> 
>> Change it to the right one.
>> 
>> Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Cc: Liam R. Howlett <Liam.Howlett@oracle.com>
>> Cc: Nico Pache <npache@redhat.com>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Cc: Dev Jain <dev.jain@arm.com>
>> Cc: Barry Song <baohua@kernel.org>
>> CC: <stable@vger.kernel.org>
>> 
>> ---
>> The original commit 8ee53820edfd is at 2011.
>> Then the code is moved to khugepaged.c in commit b46e756f5e470 ("thp:
>> extract khugepaged from mm/huge_memory.c") in 2022.
>> ---
>>   mm/khugepaged.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 24e18a7f8a93..b000942250d1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1418,7 +1418,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
>>   		if (cc->is_khugepaged &&
>>   		    (pte_young(pteval) || folio_test_young(folio) ||
>>   		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
>> -								     address)))
>> +								     _address)))
>
>Please just put that into a single line, that's a perfectly reasonable case
>to exceed 80 chars.
>
>
>Acked-by: David Hildenbrand <david@redhat.com>

Thanks.

@Andrew

Would you mind adjust the changelog and put it into one line?

>
>>   			referenced++;
>>   	}
>>   	if (!writable) {
>
>Maybe, just maybe, it's because of *horrible* variable naming.
>
>Can someone please send a cleanup to rename address -> pmd_addr and
>_address -> pte_addr or sth like that?
>
>pretty much any naming is better than this.
>
>-- 
>Cheers
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me