Re: [PATCH] mm/khugepaged: use map_anon_folio_pmd() in collapse_huge_page()

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

On Sun, Oct 05, 2025 at 02:48:25AM +0100, Matthew Wilcox wrote:
>On Sat, Oct 04, 2025 at 09:25:23AM +0000, Wei Yang wrote:
>> +void map_anon_folio_pmd(struct folio *folio, pmd_t *pmd,
>> +		struct vm_area_struct *vma, unsigned long haddr, bool in_pf);
>
>It's generally a bad idea to have a 'bool' flag like this because it's
>not immediately obvious in the callers what 'true' means.
>
>> -static void map_anon_folio_pmd(struct folio *folio, pmd_t *pmd,
>> -		struct vm_area_struct *vma, unsigned long haddr)
>> +void map_anon_folio_pmd(struct folio *folio, pmd_t *pmd,
>> +		struct vm_area_struct *vma, unsigned long haddr, bool in_pf)
>>  {
>>  	pmd_t entry;
>>  
>> @@ -1228,11 +1228,13 @@ static void map_anon_folio_pmd(struct folio *folio, pmd_t *pmd,
>>  	folio_add_lru_vma(folio, vma);
>>  	set_pmd_at(vma->vm_mm, haddr, pmd, entry);
>>  	update_mmu_cache_pmd(vma, haddr, pmd);
>> -	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>> -	count_vm_event(THP_FAULT_ALLOC);
>> -	count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_ALLOC);
>> -	count_memcg_event_mm(vma->vm_mm, THP_FAULT_ALLOC);
>>  	deferred_split_folio(folio, false);
>> +	if (in_pf) {
>> +		add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>> +		count_vm_event(THP_FAULT_ALLOC);
>> +		count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_ALLOC);
>> +		count_memcg_event_mm(vma->vm_mm, THP_FAULT_ALLOC);
>> +	}
>>  }
>
>And it's really not necessary here.  There's probably better naming
>available, but a good pattern to follow here would be:
>
>void map_anon_folio_pmd_nopf(struct folio *folio, pmd_t *pmd,
>		struct vm_area_struct *vma, unsigned long haddr)
>{
>...
>}
>
>static void map_anon_folio_pmd_pf(struct folio *folio, pmd_t *pmd,
>		struct vm_area_struct *vma, unsigned long haddr)
>{
>	map_anon_folio_pmd_nopf(folio, pmd, vma, haddr);
>	add_mm_counter(vma->vm_mm, MM_ANONPAGES, HPAGE_PMD_NR);
>	count_vm_event(THP_FAULT_ALLOC);
>	count_mthp_stat(HPAGE_PMD_ORDER, MTHP_STAT_ANON_FAULT_ALLOC);
>	count_memcg_event_mm(vma->vm_mm, THP_FAULT_ALLOC);
>}

Thanks for the comment.

Will adjust it.

-- 
Wei Yang
Help you, Help me